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

Addresses #8491.

Essentially, the fix was ensuring that we track the indentation of a
`StatementExpression` or `BreakStatement` when we apply a fix for this
lint rule, and to make sure we add these statements on a new line with
the proper indentation. I also made sure we're indenting the closing
case brackets correctly.

To do this, I added a `get_preceding_indent_str` fn to the `ast_utils`
that handles getting the preceding indentation of a `Span` in a
`source_text` &str. It returns an Option in case no indentation is
found, or if the statement is not the only one on a given line.

This new fn was useful when addressing this particular bug, but I figure
it might be useful for other fixer use cases, too. I added some
documentation for this fn for clarity.
This commit is contained in:
Tyler Earls 2025-01-30 22:32:11 -06:00 committed by GitHub
parent c63291a8ea
commit 4f30a170eb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 88 additions and 1 deletions

View file

@ -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() {

View file

@ -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 {
@ -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()
@ -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)