Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(linter): unicorn/switch-case-braces mangles code when applying fix #8758

Merged
35 changes: 35 additions & 0 deletions crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,41 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool {
_ => false,
}
}
/// Get the preceding indentation string before the start of a Span in a given source_text string slice. Useful for maintaining the format of source code when applying a linting fix.
///
/// Slice into source_text until the start of given Span.
/// Then, get the preceding spaces from the last line of the source_text.
/// If there are any non-whitespace characters preceding the Span in the last line of source_text, return None.
///
/// Examples:
///
/// 1. Given the following source_text (with 2 preceding spaces):
///
/// ```ts
/// break
/// ```
///
/// and the Span encapsulating the break statement,
///
/// this function will return " " (2 preceding spaces).
///
/// 2. Given the following source_text:
///
/// ```ts
/// const foo = 'bar'; break;
/// ```
///
/// and the Span encapsulating the break statement,
///
/// this function will return None because there is non-whitespace before the statement,
/// meaning the line of source_text containing the Span is not indented on a new line.
pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> {
let span_start = span.start as usize;
let preceding_source_text = &source_text[..span_start];

// only return last line if is whitespace
preceding_source_text.lines().last().filter(|&line| line.trim().is_empty())
}

pub fn could_be_error(ctx: &LintContext, expr: &Expression) -> bool {
match expr.get_inner_expression() {
Expand Down
54 changes: 53 additions & 1 deletion crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};

use crate::{context::LintContext, rule::Rule, AstNode};
use crate::{ast_util::get_preceding_indent_str, context::LintContext, rule::Rule, AstNode};

#[derive(Clone, Copy)]
enum Diagnostic {
Expand Down Expand Up @@ -158,10 +158,33 @@ impl Rule for SwitchCaseBraces {
formatter.print_ascii_byte(b'{');

let source_text = ctx.source_text();

for x in &case.consequent {
if matches!(
x,
Statement::ExpressionStatement(_)
| Statement::BreakStatement(_)
) {
// indent the statement in the case consequent, if needed
if let Some(indent_str) =
get_preceding_indent_str(source_text, x.span())
{
formatter.print_ascii_byte(b'\n');
formatter.print_str(indent_str);
}
}

formatter.print_str(x.span().source_text(source_text));
}

// indent the closing case bracket, if needed
if let Some(case_indent_str) =
get_preceding_indent_str(source_text, case.span())
{
formatter.print_ascii_byte(b'\n');
formatter.print_str(case_indent_str);
}

formatter.print_ascii_byte(b'}');

formatter.into_source_text()
Expand Down Expand Up @@ -227,6 +250,35 @@ fn test() {
"switch(foo) { default: doSomething(); }",
Some(serde_json::json!(["avoid"])),
),
// Issue: https://github.com/oxc-project/oxc/issues/8491
(
"
const alpha = 7
let beta = ''
let gamma = 0

switch (alpha) {
case 1:
beta = 'one'
gamma = 1
break
}
",
"
const alpha = 7
let beta = ''
let gamma = 0

switch (alpha) {
case 1: {
beta = 'one'
gamma = 1
break
}
}
",
None,
),
];

Tester::new(SwitchCaseBraces::NAME, SwitchCaseBraces::PLUGIN, pass, fail)
Expand Down
Loading