dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-24 12:21:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

feat(linter/eslint): improve no-duplicate-case rule (#4942)

Browse source 
- ignore parethesis when comparing case test expressions
- make labels in diagnostics more friendly
- add more examples to documentation
This commit is contained in:
DonIsaac 2024-08-17 01:21:49 +00:00
parent e99836d7d8
commit e1582a5dd3
2 changed files with 149 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
crates/oxc_linter/src/rules/eslint/no_duplicate_case.rs
View file

@ -6,10 +6,10 @@ use rustc_hash::FxHashMap;
use crate::{ast_util::calculate_hash, context::LintContext, rule::Rule, AstNode}; use crate::{ast_util::calculate_hash, context::LintContext, rule::Rule, AstNode};
fn no_duplicate_case_diagnostic(span0: Span, span1: Span) -> OxcDiagnostic { fn no_duplicate_case_diagnostic(first: Span, second: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow duplicate case labels") OxcDiagnostic::warn("Duplicate case label")
.with_help("Remove the duplicated case") .with_help("Remove the duplicated case")
.with_labels([span0, span1]) .with_labels([first.label("This label here"), second.label("is duplicated here")])
} }
#[derive(Debug, Default, Clone)] #[derive(Debug, Default, Clone)]
@ -26,18 +26,54 @@ declare_oxc_lint!(
/// it is likely that a programmer copied a case clause but forgot to change the test expression. /// it is likely that a programmer copied a case clause but forgot to change the test expression.
/// ///
/// ### Example /// ### Example
/// ```javascript ///
/// var a = 1; /// Examples of **incorrect** code for this rule:
/// ```js
/// var a = 1, one = 1;
/// switch (a) { /// switch (a) {
/// case 1: /// case 1:
/// break; /// break;
/// case 1:
/// break;
/// case 2: /// case 2:
/// break; /// break;
/// case 1: // duplicate test expression
/// break;
/// default: /// default:
/// break; /// break;
/// } /// }
///
/// switch (a) {
/// case one:
/// break;
/// case 2:
/// break;
/// case one: // duplicate test expression
/// break;
/// default:
/// break;
/// }
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// var a = 1,
/// one = 1
/// switch (a) {
/// case 1:
/// break
/// case 2:
/// break
/// default:
/// break
/// }
///
/// switch (a) {
/// case '1':
/// break
/// case '2':
/// break
/// default:
/// break
/// }
/// ``` /// ```
NoDuplicateCase, NoDuplicateCase,
correctness correctness
@ -45,12 +81,15 @@ declare_oxc_lint!(
impl Rule for NoDuplicateCase { impl Rule for NoDuplicateCase {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::SwitchStatement(ss) = node.kind() { let AstKind::SwitchStatement(ss) = node.kind() else {
return;
};
let mut map = FxHashMap::default(); let mut map = FxHashMap::default();
map.reserve(ss.cases.len()); map.reserve(ss.cases.len());
for case in &ss.cases { for case in &ss.cases {
if let Some(test) = case.test.as_ref() { if let Some(test) = case.test.as_ref() {
let hash = calculate_hash(test); let hash = calculate_hash(test.get_inner_expression());
if let Some(prev_span) = map.insert(hash, test.span()) { if let Some(prev_span) = map.insert(hash, test.span()) {
ctx.diagnostic(no_duplicate_case_diagnostic(prev_span, test.span())); ctx.diagnostic(no_duplicate_case_diagnostic(prev_span, test.span()));
@ -59,7 +98,6 @@ impl Rule for NoDuplicateCase {
} }
} }
} }
}
#[test] #[test]
fn test() { fn test() {
@ -103,6 +141,10 @@ fn test() {
"var a = 1; switch (a) {case 1: break; case 1: break; case 2: break; default: break;}", "var a = 1; switch (a) {case 1: break; case 1: break; case 2: break; default: break;}",
None, None,
), ),
(
"var a = 1; switch (a) {case 1: break; case (1): break; case 2: break; default: break;}",
None,
),
( (
"var a = '1'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;}", "var a = '1'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;}",
None, None,

135
crates/oxc_linter/src/snapshots/no_duplicate_case.snap
View file

@ -1,146 +1,193 @@
--- ---
source: crates/oxc_linter/src/tester.rs source: crates/oxc_linter/src/tester.rs
--- ---
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:29] ╭─[no_duplicate_case.tsx:1:29]
1 │ var a = 1; switch (a) {case 1: break; case 1: break; case 2: break; default: break;} 1 │ var a = 1; switch (a) {case 1: break; case 1: break; case 2: break; default: break;}
· ─ ─ · ┬ ┬
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:29]
1 │ var a = 1; switch (a) {case 1: break; case (1): break; case 2: break; default: break;}
· ┬ ─┬─
· │ ╰── is duplicated here
· ╰── This label here
╰────
help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:31] ╭─[no_duplicate_case.tsx:1:31]
1 │ var a = '1'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;} 1 │ var a = '1'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;}
· ─── ─── · ─┬─ ─┬─
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:38] ╭─[no_duplicate_case.tsx:1:38]
1 │ var a = 1, one = 1; switch (a) {case one: break; case one: break; case 2: break; default: break;} 1 │ var a = 1, one = 1; switch (a) {case one: break; case one: break; case 2: break; default: break;}
· ─── ─── · ─┬─ ─┬─
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:54] ╭─[no_duplicate_case.tsx:1:54]
1 │ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p.p.p1: break; default: break;} 1 │ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p.p.p1: break; default: break;}
· ────── ────── · ───┬── ───┬──
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:84] ╭─[no_duplicate_case.tsx:1:84]
1 │ var a = 1, f = function(b) { return b ? { p1: 1 } : { p1: 2 }; }; switch (a) {case f(true).p1: break; case f(true).p1: break; default: break;} 1 │ var a = 1, f = function(b) { return b ? { p1: 1 } : { p1: 2 }; }; switch (a) {case f(true).p1: break; case f(true).p1: break; default: break;}
· ────────── ────────── · ─────┬──── ─────┬────
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:67] ╭─[no_duplicate_case.tsx:1:67]
1 │ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a + 1).p1: break; case f(a + 1).p1: break; default: break;} 1 │ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a + 1).p1: break; case f(a + 1).p1: break; default: break;}
· ─────────── ─────────── · ─────┬───── ─────┬─────
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:67] ╭─[no_duplicate_case.tsx:1:67]
1 │ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a === 1 ? 2 : 3).p1: break; case f(a === 1 ? 2 : 3).p1: break; default: break;} 1 │ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a === 1 ? 2 : 3).p1: break; case f(a === 1 ? 2 : 3).p1: break; default: break;}
· ───────────────────── ───────────────────── · ──────────┬────────── ──────────┬──────────
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:67] ╭─[no_duplicate_case.tsx:1:67]
1 │ var a = 1, f1 = function() { return { p1: 1 } }; switch (a) {case f1().p1: break; case f1().p1: break; default: break;} 1 │ var a = 1, f1 = function() { return { p1: 1 } }; switch (a) {case f1().p1: break; case f1().p1: break; default: break;}
· ─────── ─────── · ───┬─── ───┬───
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:43] ╭─[no_duplicate_case.tsx:1:43]
1 │ var a = [1, 2]; switch(a.toString()){case ([1, 2]).toString():break; case ([1, 2]).toString():break; default:break;} 1 │ var a = [1, 2]; switch(a.toString()){case ([1, 2]).toString():break; case ([1, 2]).toString():break; default:break;}
· ─────────────────── ─────────────────── · ─────────┬───────── ─────────┬─────────
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:19] ╭─[no_duplicate_case.tsx:1:19]
1 │ switch (a) { case a: case a: } 1 │ switch (a) { case a: case a: }
· ─ ─ · ┬ ┬
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:19] ╭─[no_duplicate_case.tsx:1:19]
1 │ switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; } 1 │ switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; }
· ─ ─ · ┬ ┬
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:49] ╭─[no_duplicate_case.tsx:1:49]
1 │ switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; } 1 │ switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; }
· ─ ─ · ┬ ┬
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:54] ╭─[no_duplicate_case.tsx:1:54]
1 │ ╭─▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment 1 │ ╭─▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment
· │ ────── · │ ───┬──
2 │ ╰─▶ .p1: break; default: break;} · │ ╰── This label here
2 │ ├─▶ .p1: break; default: break;}
· ╰──── is duplicated here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:54] ╭─[no_duplicate_case.tsx:1:54]
1 │ ╭─▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p 1 │ ╭─▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p
2 │ │ /* comment */ 2 │ │ /* comment */
3 │ ╰─▶ .p1: break; case p.p.p1: break; default: break;} 3 │ ├─▶ .p1: break; case p.p.p1: break; default: break;}
· ╰─── ────── · ╰─── ───┬──
· ╰─── ╰── is duplicated here
· ╰──── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:54] ╭─[no_duplicate_case.tsx:1:54]
1 │ ╭──▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p 1 │ ╭──▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p
2 │ │ /* comment */ 2 │ │ /* comment */
3 │ ╰──▶ .p1: break; case p. p // comment 3 │ ├──▶ .p1: break; case p. p // comment
4 │ ╰──▶ .p1: break; default: break;} · ╰───── This label here
4 │ ├──▶ .p1: break; default: break;}
· ╰───── is duplicated here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:54] ╭─[no_duplicate_case.tsx:1:54]
1 │ ╭─▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment 1 │ ╭─▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment
· │ ────── · │ ───┬──
2 │ ╰─▶ .p1: break; case p .p · │ ╰── This label here
2 │ ├─▶ .p1: break; case p .p
· ╰──── is duplicated here
3 │ /* comment */ 3 │ /* comment */
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:74] ╭─[no_duplicate_case.tsx:1:74]
1 │ ╭──▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment 1 │ ╭──▶ var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment
2 │ ╰──▶ .p1: break; case p .p 2 │ ├──▶ .p1: break; case p .p
· ╰───── This label here
3 │ │ /* comment */ 3 │ │ /* comment */
4 │ ╰──▶ .p1: break; default: break;} 4 │ ├──▶ .p1: break; default: break;}
· ╰───── is duplicated here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:67] ╭─[no_duplicate_case.tsx:1:67]
1 │ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a + 1).p1: break; case f(a+1).p1: break; default: break;} 1 │ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a + 1).p1: break; case f(a+1).p1: break; default: break;}
· ─────────── ───────── · ─────┬───── ────┬────
· │ ╰── is duplicated here
· ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Disallow duplicate case labels ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:67] ╭─[no_duplicate_case.tsx:1:67]
1 │ ╭──▶ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f( 1 │ ╭──▶ var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(
2 │ │ a + 1 // comment 2 │ │ a + 1 // comment
3 │ ╰──▶ ).p1: break; case f(a+1) 3 │ ├──▶ ).p1: break; case f(a+1)
4 │ ╰──▶ .p1: break; default: break;} · ╰───── This label here
4 │ ├──▶ .p1: break; default: break;}
· ╰───── is duplicated here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case