Skip to content

Commit 616b613

Browse files
shulaodashulaoda
committed
fix(linter/switch-case-braces): align the logic with unicorn (#11405)
In the case of `switch(foo){ case 1: {}; break; }`, only a "Missing braces in case clause" diagnostic should be reported. However, the current behavior incorrectly reports below: ```bash ⚠ eslint-plugin-unicorn(switch-case-braces): Unexpected braces in empty case clause. ╭─[switch_case_braces.tsx:1:22] 1 │ switch(foo){ case 1: {}; break; } · ── ╰──── help: Remove braces in empty case clause. ⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause. ╭─[switch_case_braces.tsx:1:22] 1 │ switch(foo){ case 1: {}; break; } · ────────── ╰──── help: Add Braces for case clause. ```
1 parent d0cc331 commit 616b613

File tree

2 files changed

+98
-151
lines changed

2 files changed

+98
-151
lines changed

‎crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs‎

Lines changed: 79 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,22 @@ use oxc_span::{GetSpan, Span};
55

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

8-
#[derive(Clone, Copy)]
9-
enum Diagnostic {
10-
EmptyClause,
11-
MissingBraces,
12-
UnnecessaryBraces,
8+
fn switch_case_braces_diagnostic_empty_clause(span: Span) -> OxcDiagnostic {
9+
OxcDiagnostic::warn("Unexpected braces in empty case clause.")
10+
.with_help("Remove braces in empty case clause.")
11+
.with_label(span)
1312
}
1413

15-
fn switch_case_braces_diagnostic(span: Span, diagnostic_type: Diagnostic) -> OxcDiagnostic {
16-
(match diagnostic_type {
17-
Diagnostic::EmptyClause => OxcDiagnostic::warn("Unexpected braces in empty case clause.")
18-
.with_help("Remove braces in empty case clause."),
19-
Diagnostic::MissingBraces => OxcDiagnostic::warn("Missing braces in case clause.")
20-
.with_help("Add Braces for case clause."),
21-
Diagnostic::UnnecessaryBraces => OxcDiagnostic::warn("Unnecessary braces in case clause.")
22-
.with_help("Remove Braces for case clause."),
23-
})
24-
.with_label(span)
14+
fn switch_case_braces_diagnostic_missing_braces(span: Span) -> OxcDiagnostic {
15+
OxcDiagnostic::warn("Missing braces in case clause.")
16+
.with_help("Add Braces for case clause.")
17+
.with_label(span)
18+
}
19+
20+
fn switch_case_braces_diagnostic_unnecessary_braces(span: Span) -> OxcDiagnostic {
21+
OxcDiagnostic::warn("Unnecessary braces in case clause.")
22+
.with_help("Remove Braces for case clause.")
23+
.with_label(span)
2524
}
2625

2726
#[derive(Debug, Default, Clone)]
@@ -76,9 +75,8 @@ declare_oxc_lint!(
7675

7776
impl Rule for SwitchCaseBraces {
7877
fn from_configuration(value: serde_json::Value) -> Self {
79-
let always = value.get(0).is_none_or(|v| v.as_str() != Some("avoid"));
80-
81-
Self { always_braces: always }
78+
let always_braces = value.get(0).is_none_or(|v| v.as_str() != Some("avoid"));
79+
Self { always_braces }
8280
}
8381

8482
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
@@ -91,130 +89,70 @@ impl Rule for SwitchCaseBraces {
9189
}
9290

9391
for case in &switch.cases {
94-
for case_consequent in &case.consequent {
95-
match case_consequent {
96-
Statement::BlockStatement(case_block) => {
97-
if case_block.body.is_empty() {
98-
ctx.diagnostic_with_fix(
99-
switch_case_braces_diagnostic(
100-
case_block.span,
101-
Diagnostic::EmptyClause,
102-
),
103-
|fixer| fixer.delete_range(case_block.span),
104-
);
105-
}
106-
107-
if !self.always_braces
108-
&& !case_block.body.iter().any(|stmt| {
109-
matches!(
110-
stmt,
111-
Statement::VariableDeclaration(_)
112-
| Statement::FunctionDeclaration(_)
113-
)
114-
})
115-
{
116-
ctx.diagnostic_with_fix(
117-
switch_case_braces_diagnostic(
118-
case_block.span(),
119-
Diagnostic::UnnecessaryBraces,
120-
),
121-
|fixer| {
122-
fixer.replace(
123-
case_block.span,
124-
fixer.source_range(Span::new(
125-
case_block.span.start + 1,
126-
case_block.span.end - 1,
127-
)),
128-
)
129-
},
130-
);
131-
}
92+
if case.consequent.is_empty() {
93+
continue;
94+
}
95+
let missing_braces = match &case.consequent[0] {
96+
Statement::BlockStatement(block_stmt) if case.consequent.len() == 1 => {
97+
if block_stmt.body.is_empty() {
98+
ctx.diagnostic_with_fix(
99+
switch_case_braces_diagnostic_empty_clause(block_stmt.span),
100+
|fixer| fixer.delete_range(block_stmt.span),
101+
);
102+
continue;
132103
}
133-
Statement::EmptyStatement(_) => {}
134-
_ => {
135-
if !self.always_braces
136-
&& !&case.consequent.iter().any(|stmt| {
137-
matches!(
138-
stmt,
139-
Statement::VariableDeclaration(_)
140-
| Statement::FunctionDeclaration(_)
141-
)
142-
})
143-
{
144-
return;
145-
}
146-
147-
let Some(first_statement) = &case.consequent.first() else {
148-
return;
149-
};
150-
let Some(last_statement) = &case.consequent.last() else {
151-
return;
152-
};
153-
154-
let case_body_span =
155-
Span::new(first_statement.span().start, last_statement.span().end);
156-
104+
if !self.always_braces
105+
&& !block_stmt.body.iter().any(|stmt| {
106+
matches!(
107+
stmt,
108+
Statement::VariableDeclaration(_)
109+
| Statement::FunctionDeclaration(_)
110+
)
111+
})
112+
{
157113
ctx.diagnostic_with_fix(
158-
switch_case_braces_diagnostic(
159-
case_body_span,
160-
Diagnostic::MissingBraces,
161-
),
114+
switch_case_braces_diagnostic_unnecessary_braces(block_stmt.span()),
162115
|fixer| {
163-
let modified_code = {
164-
let mut formatter = fixer.codegen();
165-
166-
if let Some(case_test) = &case.test {
167-
formatter.print_str("case ");
168-
formatter.print_expression(case_test);
169-
} else {
170-
formatter.print_str("default");
171-
}
172-
173-
formatter.print_ascii_byte(b':');
174-
formatter.print_ascii_byte(b' ');
175-
formatter.print_ascii_byte(b'{');
176-
177-
let source_text = ctx.source_text();
178-
179-
for x in &case.consequent {
180-
if matches!(
181-
x,
182-
Statement::ExpressionStatement(_)
183-
| Statement::BreakStatement(_)
184-
) {
185-
// indent the statement in the case consequent, if needed
186-
if let Some(indent_str) =
187-
get_preceding_indent_str(source_text, x.span())
188-
{
189-
formatter.print_ascii_byte(b'\n');
190-
formatter.print_str(indent_str);
191-
}
192-
}
193-
194-
formatter.print_str(x.span().source_text(source_text));
195-
}
196-
197-
// indent the closing case bracket, if needed
198-
if let Some(case_indent_str) =
199-
get_preceding_indent_str(source_text, case.span())
200-
{
201-
formatter.print_ascii_byte(b'\n');
202-
formatter.print_str(case_indent_str);
203-
}
204-
205-
formatter.print_ascii_byte(b'}');
206-
207-
formatter.into_source_text()
208-
};
209-
210-
fixer.replace(case.span, modified_code)
116+
fixer.replace(
117+
block_stmt.span,
118+
fixer.source_range(block_stmt.span.shrink(1)),
119+
)
211120
},
212121
);
213-
214-
// After first incorrect consequent we have to break to not repeat the work
215-
break;
122+
continue;
216123
}
124+
false
217125
}
126+
_ => true,
127+
};
128+
129+
if self.always_braces && missing_braces {
130+
let colon = u32::try_from(ctx.source_range(case.span).find(':').unwrap()).unwrap();
131+
let span = Span::new(case.span.start, case.span.start + colon + 1);
132+
ctx.diagnostic_with_fix(
133+
switch_case_braces_diagnostic_missing_braces(span),
134+
|fixer| {
135+
let fixer = fixer.for_multifix();
136+
let mut fix = fixer.new_fix_with_capacity(2);
137+
138+
fix.push(fixer.insert_text_after_range(span, " {"));
139+
140+
// Indent the closing case bracket, if needed
141+
let code = match get_preceding_indent_str(fixer.source_text(), case.span) {
142+
Some(indent) => {
143+
let mut code = String::with_capacity(2 + indent.len());
144+
code.push('\n');
145+
code.push_str(indent);
146+
code.push('}');
147+
code
148+
}
149+
None => String::from('}'),
150+
};
151+
152+
fix.push(fixer.insert_text_after_range(case.span, code));
153+
fix.with_message("Add Braces for case clause.")
154+
},
155+
);
218156
}
219157
}
220158
}
@@ -243,6 +181,7 @@ fn test() {
243181
"switch(foo) { case 1: { doSomething(); } break; /* <-- This should be between braces */ }",
244182
"switch(foo) { default: label: {} }",
245183
"switch(something) { case 1: case 2: { console.log('something'); break; } case 3: console.log('something else'); }",
184+
"switch(foo){ case 1: {}; break; }",
246185
];
247186

248187
let fix = vec![
@@ -253,15 +192,15 @@ fn test() {
253192
),
254193
(
255194
"switch(something) { case 1: {} case 2: console.log('something'); break;}",
256-
"switch(something) { case 1: case 2: {console.log('something');break;}}",
195+
"switch(something) { case 1: case 2: { console.log('something'); break;}}",
257196
None,
258197
),
259198
(
260199
"switch(foo) { default: doSomething(); }",
261-
"switch(foo) { default: {doSomething();} }",
200+
"switch(foo) { default: { doSomething();} }",
262201
None,
263202
),
264-
("switch(s){case'':/]/}", "switch(s){case '': {/]/}}", None),
203+
("switch(s){case'':/]/}", "switch(s){case'': {/]/}}", None),
265204
(
266205
"switch(foo) { default: {doSomething();} }",
267206
"switch(foo) { default: doSomething(); }",
@@ -275,7 +214,7 @@ fn test() {
275214
let gamma = 0
276215
277216
switch (alpha) {
278-
case 1:
217+
case 1:
279218
beta = 'one'
280219
gamma = 1
281220
break
@@ -296,6 +235,7 @@ fn test() {
296235
",
297236
None,
298237
),
238+
("switch(foo){ case 1: {}; break; }", "switch(foo){ case 1: { {}; break;} }", None),
299239
];
300240

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

‎crates/oxc_linter/src/snapshots/unicorn_switch_case_braces.snap‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
source: crates/oxc_linter/src/tester.rs
33
---
44
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
5-
╭─[switch_case_braces.tsx:1:18]
5+
╭─[switch_case_braces.tsx:1:11]
66
1 │ switch(s){case'':/]/}
7-
· ───
7+
· ───────
88
╰────
99
help: Add Braces for case clause.
1010

@@ -16,9 +16,9 @@ source: crates/oxc_linter/src/tester.rs
1616
help: Remove braces in empty case clause.
1717

1818
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
19-
╭─[switch_case_braces.tsx:1:37]
19+
╭─[switch_case_braces.tsx:1:29]
2020
1 │ switch(something) { case 1: case 2: console.log('something'); break;}
21-
· ────────────────────────────────
21+
· ───────
2222
╰────
2323
help: Add Braces for case clause.
2424

@@ -51,29 +51,36 @@ source: crates/oxc_linter/src/tester.rs
5151
help: Remove braces in empty case clause.
5252

5353
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
54-
╭─[switch_case_braces.tsx:1:24]
54+
╭─[switch_case_braces.tsx:1:15]
5555
1 │ switch(foo) { default: doSomething(); }
56-
· ──────────────
56+
· ────────
5757
╰────
5858
help: Add Braces for case clause.
5959

6060
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
61-
╭─[switch_case_braces.tsx:1:23]
61+
╭─[switch_case_braces.tsx:1:15]
6262
1 │ switch(foo) { case 1: { doSomething(); } break; /* <-- This should be between braces */ }
63-
· ─────────────────────────
63+
· ───────
6464
╰────
6565
help: Add Braces for case clause.
6666

6767
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
68-
╭─[switch_case_braces.tsx:1:24]
68+
╭─[switch_case_braces.tsx:1:15]
6969
1 │ switch(foo) { default: label: {} }
70-
· ─────────
70+
· ────────
7171
╰────
7272
help: Add Braces for case clause.
7373

7474
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
75-
╭─[switch_case_braces.tsx:1:82]
75+
╭─[switch_case_braces.tsx:1:74]
7676
1 │ switch(something) { case 1: case 2: { console.log('something'); break; } case 3: console.log('something else'); }
77-
· ──────────────────────────────
77+
· ───────
78+
╰────
79+
help: Add Braces for case clause.
80+
81+
⚠ eslint-plugin-unicorn(switch-case-braces): Missing braces in case clause.
82+
╭─[switch_case_braces.tsx:1:14]
83+
1 │ switch(foo){ case 1: {}; break; }
84+
· ───────
7885
╰────
7986
help: Add Braces for case clause.

0 commit comments

Comments
 (0)