mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 12:19:15 +00:00
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:
parent
c63291a8ea
commit
4f30a170eb
2 changed files with 88 additions and 1 deletions
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue