Skip to content

Commit

Permalink
refactor(linter): use ContentHash for no_duplicate_case; remove `…
Browse files Browse the repository at this point in the history 
…calculate_hash` (#5648)

closes #5600
  • Loading branch information
@Boshen
Boshen committed Sep 9, 2024
1 parent bfe9186 commit a37c064
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 137 deletions.
10 changes: 1 addition & 9 deletions crates/oxc_linter/src/ast_util.rs
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
use core::hash::Hasher;

use oxc_ast::{ast::BindingIdentifier, AstKind};
use oxc_semantic::{AstNode, AstNodeId, SymbolId};
use oxc_span::{hash::ContentHash, GetSpan, Span};
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator};
use rustc_hash::FxHasher;

pub fn calculate_hash<T: ContentHash>(t: &T) -> u64 {
let mut hasher = FxHasher::default();
t.content_hash(&mut hasher);
hasher.finish()
}
#[allow(clippy::wildcard_imports)]
use oxc_ast::ast::*;

Expand Down
157 changes: 41 additions & 116 deletions crates/oxc_linter/src/rules/eslint/no_duplicate_case.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use oxc_ast::AstKind;
use oxc_ast::ast::Expression;
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use rustc_hash::FxHashMap;
use oxc_span::{cmp::ContentEq, GetSpan, Span};

use crate::{ast_util::calculate_hash, context::LintContext, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_duplicate_case_diagnostic(first: Span, second: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Duplicate case label")
Expand Down Expand Up @@ -81,19 +80,14 @@ declare_oxc_lint!(

impl Rule for NoDuplicateCase {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::SwitchStatement(ss) = node.kind() else {
return;
};

let mut map = FxHashMap::default();
map.reserve(ss.cases.len());
for case in &ss.cases {
if let Some(test) = case.test.as_ref() {
let hash = calculate_hash(test.get_inner_expression());

if let Some(prev_span) = map.insert(hash, test.span()) {
ctx.diagnostic(no_duplicate_case_diagnostic(prev_span, test.span()));
}
let Some(ss) = node.kind().as_switch_statement() else { return };
let mut previous_tests: Vec<&Expression<'_>> = vec![];
for test in ss.cases.iter().filter_map(|c| c.test.as_ref()) {
let test = test.without_parentheses();
if let Some(prev) = previous_tests.iter().find(|t| t.content_eq(test)) {
ctx.diagnostic(no_duplicate_case_diagnostic(prev.span(), test.span()));
} else {
previous_tests.push(test);
}
}
}
Expand All @@ -104,108 +98,39 @@ fn test() {
use crate::tester::Tester;

let pass = vec![
("var a = 1; switch (a) {case 1: break; case 2: break; default: break;}", None),
("var a = 1; switch (a) {case 1: break; case '1': break; default: break;}", None),
("var a = 1; switch (a) {case 1: break; case true: break; default: break;}", None),
("var a = 1; switch (a) {default: break;}", None),
(
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p.p.p2: break; default: break;}",
None,
),
(
"var a = 1, f = function(b) { return b ? { p1: 1 } : { p1: 2 }; }; switch (a) {case f(true).p1: break; case f(true, false).p1: break; default: break;}",
None,
),
(
"var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a + 1).p1: break; case f(a + 2).p1: break; default: break;}",
None,
),
(
"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;}",
None,
),
(
"var a = 1, f1 = function() { return { p1: 1 } }, f2 = function() { return { p1: 2 } }; switch (a) {case f1().p1: break; case f2().p1: break; default: break;}",
None,
),
(
"var a = [1,2]; switch(a.toString()){case ([1,2]).toString():break; case ([1]).toString():break; default:break;}",
None,
),
("switch(a) { case a: break; } switch(a) { case a: break; }", None),
("switch(a) { case toString: break; }", None),
"var a = 1; switch (a) {case 1: break; case 2: break; default: break;}",
"var a = 1; switch (a) {case 1: break; case '1': break; default: break;}",
"var a = 1; switch (a) {case 1: break; case true: break; default: break;}",
"var a = 1; switch (a) {default: break;}",
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p.p.p2: break; default: break;}",
"var a = 1, f = function(b) { return b ? { p1: 1 } : { p1: 2 }; }; switch (a) {case f(true).p1: break; case f(true, false).p1: break; default: break;}",
"var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(a + 1).p1: break; case f(a + 2).p1: break; default: break;}",
"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;}",
"var a = 1, f1 = function() { return { p1: 1 } }, f2 = function() { return { p1: 2 } }; switch (a) {case f1().p1: break; case f2().p1: break; default: break;}",
"var a = [1,2]; switch(a.toString()){case ([1,2]).toString():break; case ([1]).toString():break; default:break;}",
"switch(a) { case a: break; } switch(a) { case a: break; }",
"switch(a) { case toString: break; }",
];

let fail = vec![
(
"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;}",
None,
),
(
"var a = '1'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;}",
None,
),
(
"var a = 1, one = 1; switch (a) {case one: break; case one: break; case 2: break; default: break;}",
None,
),
(
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p.p.p1: break; default: break;}",
None,
),
(
"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;}",
None,
),
(
"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;}",
None,
),
(
"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;}",
None,
),
(
"var a = 1, f1 = function() { return { p1: 1 } }; switch (a) {case f1().p1: break; case f1().p1: break; default: break;}",
None,
),
(
"var a = [1, 2]; switch(a.toString()){case ([1, 2]).toString():break; case ([1, 2]).toString():break; default:break;}",
None,
),
("switch (a) { case a: case a: }", None),
(
"switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; }",
None,
),
(
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment\n .p1: break; default: break;}",
None,
),
(
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p\n/* comment */\n.p1: break; case p.p.p1: break; default: break;}",
None,
),
(
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p\n/* comment */\n.p1: break; case p. p // comment\n .p1: break; default: break;}",
None,
),
(
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment\n .p1: break; case p .p\n/* comment */\n.p1: break; default: break;}",
None,
),
(
"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;}",
None,
),
(
"var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(\na + 1 // comment\n).p1: break; case f(a+1)\n.p1: 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;}",
"var a = '1'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;}",
"var a = 1, one = 1; switch (a) {case one: break; case one: break; case 2: break; default: break;}",
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p.p.p1: break; default: break;}",
"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;}",
"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;}",
"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;}",
"var a = 1, f1 = function() { return { p1: 1 } }; switch (a) {case f1().p1: break; case f1().p1: break; default: break;}",
"var a = [1, 2]; switch(a.toString()){case ([1, 2]).toString():break; case ([1, 2]).toString():break; default:break;}",
"switch (a) { case a: case a: }",
"switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; }",
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment\n .p1: break; default: break;}",
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p\n/* comment */\n.p1: break; case p.p.p1: break; default: break;}",
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p .p\n/* comment */\n.p1: break; case p. p // comment\n .p1: break; default: break;}",
"var a = 1, p = {p: {p1: 1, p2: 1}}; switch (a) {case p.p.p1: break; case p. p // comment\n .p1: break; case p .p\n/* comment */\n.p1: break; default: break;}",
"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;}",
"var a = 1, f = function(s) { return { p1: s } }; switch (a) {case f(\na + 1 // comment\n).p1: break; case f(a+1)\n.p1: break; default: break;}",
];

Tester::new(NoDuplicateCase::NAME, pass, fail).test_and_snapshot();
Expand Down
25 changes: 13 additions & 12 deletions crates/oxc_linter/src/snapshots/no_duplicate_case.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ source: crates/oxc_linter/src/tester.rs
⚠ 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
╰────
Expand Down Expand Up @@ -110,11 +110,11 @@ source: crates/oxc_linter/src/tester.rs
help: Remove the duplicated case

⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:49]
╭─[no_duplicate_case.tsx:1:19]
1 │ switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; }
· ┬ ┬
· │ ╰── is duplicated here
· ╰── This label here
· ┬ ┬
· │ ╰── is duplicated here
· ╰── This label here
╰────
help: Remove the duplicated case

Expand Down Expand Up @@ -162,13 +162,14 @@ source: crates/oxc_linter/src/tester.rs
help: Remove the duplicated case

⚠ eslint(no-duplicate-case): Duplicate case label
╭─[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
2 │ ├──▶ .p1: break; case p .p
· ╰───── This label here
3 │ │ /* comment */
4 │ ├──▶ .p1: break; default: break;}
· ╰───── is duplicated here
╭─[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
· ───┬──
· ╰── This label here
2 │ ╭─▶ .p1: break; case p .p
3 │ │ /* comment */
4 │ ├─▶ .p1: break; default: break;}
· ╰──── is duplicated here
╰────
help: Remove the duplicated case

Expand Down

0 comments on commit a37c064

Please sign in to comment.