Skip to content

Commit 4f30a17

Browse files
authoredJan 31, 2025··
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.
1 parent c63291a commit 4f30a17

File tree

2 files changed

+88
-1
lines changed

2 files changed

+88
-1
lines changed
 

Diff for: ‎crates/oxc_linter/src/ast_util.rs

+35
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,41 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool {
486486
_ => false,
487487
}
488488
}
489+
/// 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.
490+
///
491+
/// Slice into source_text until the start of given Span.
492+
/// Then, get the preceding spaces from the last line of the source_text.
493+
/// If there are any non-whitespace characters preceding the Span in the last line of source_text, return None.
494+
///
495+
/// Examples:
496+
///
497+
/// 1. Given the following source_text (with 2 preceding spaces):
498+
///
499+
/// ```ts
500+
/// break
501+
/// ```
502+
///
503+
/// and the Span encapsulating the break statement,
504+
///
505+
/// this function will return " " (2 preceding spaces).
506+
///
507+
/// 2. Given the following source_text:
508+
///
509+
/// ```ts
510+
/// const foo = 'bar'; break;
511+
/// ```
512+
///
513+
/// and the Span encapsulating the break statement,
514+
///
515+
/// this function will return None because there is non-whitespace before the statement,
516+
/// meaning the line of source_text containing the Span is not indented on a new line.
517+
pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> {
518+
let span_start = span.start as usize;
519+
let preceding_source_text = &source_text[..span_start];
520+
521+
// only return last line if is whitespace
522+
preceding_source_text.lines().last().filter(|&line| line.trim().is_empty())
523+
}
489524

490525
pub fn could_be_error(ctx: &LintContext, expr: &Expression) -> bool {
491526
match expr.get_inner_expression() {

Diff for: ‎crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs

+53-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use oxc_diagnostics::OxcDiagnostic;
33
use oxc_macros::declare_oxc_lint;
44
use oxc_span::{GetSpan, Span};
55

6-
use crate::{context::LintContext, rule::Rule, AstNode};
6+
use crate::{ast_util::get_preceding_indent_str, context::LintContext, rule::Rule, AstNode};
77

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

160160
let source_text = ctx.source_text();
161+
161162
for x in &case.consequent {
163+
if matches!(
164+
x,
165+
Statement::ExpressionStatement(_)
166+
| Statement::BreakStatement(_)
167+
) {
168+
// indent the statement in the case consequent, if needed
169+
if let Some(indent_str) =
170+
get_preceding_indent_str(source_text, x.span())
171+
{
172+
formatter.print_ascii_byte(b'\n');
173+
formatter.print_str(indent_str);
174+
}
175+
}
176+
162177
formatter.print_str(x.span().source_text(source_text));
163178
}
164179

180+
// indent the closing case bracket, if needed
181+
if let Some(case_indent_str) =
182+
get_preceding_indent_str(source_text, case.span())
183+
{
184+
formatter.print_ascii_byte(b'\n');
185+
formatter.print_str(case_indent_str);
186+
}
187+
165188
formatter.print_ascii_byte(b'}');
166189

167190
formatter.into_source_text()
@@ -227,6 +250,35 @@ fn test() {
227250
"switch(foo) { default: doSomething(); }",
228251
Some(serde_json::json!(["avoid"])),
229252
),
253+
// Issue: https://github.com/oxc-project/oxc/issues/8491
254+
(
255+
"
256+
const alpha = 7
257+
let beta = ''
258+
let gamma = 0
259+
260+
switch (alpha) {
261+
case 1:
262+
beta = 'one'
263+
gamma = 1
264+
break
265+
}
266+
",
267+
"
268+
const alpha = 7
269+
let beta = ''
270+
let gamma = 0
271+
272+
switch (alpha) {
273+
case 1: {
274+
beta = 'one'
275+
gamma = 1
276+
break
277+
}
278+
}
279+
",
280+
None,
281+
),
230282
];
231283

232284
Tester::new(SwitchCaseBraces::NAME, SwitchCaseBraces::PLUGIN, pass, fail)

0 commit comments

Comments
 (0)
Please sign in to comment.