From a37c064565396c074f2626db21ec5a77b808e4e9 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Mon, 9 Sep 2024 14:01:29 +0000 Subject: [PATCH] refactor(linter): use `ContentHash` for `no_duplicate_case`; remove `calculate_hash` (#5648) closes #5600 --- crates/oxc_linter/src/ast_util.rs | 10 +- .../src/rules/eslint/no_duplicate_case.rs | 157 +++++------------- .../src/snapshots/no_duplicate_case.snap | 25 +-- 3 files changed, 55 insertions(+), 137 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 127fcc26d4596..10b555169c9c1 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -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: &T) -> u64 { - let mut hasher = FxHasher::default(); - t.content_hash(&mut hasher); - hasher.finish() -} #[allow(clippy::wildcard_imports)] use oxc_ast::ast::*; diff --git a/crates/oxc_linter/src/rules/eslint/no_duplicate_case.rs b/crates/oxc_linter/src/rules/eslint/no_duplicate_case.rs index b9ea97940300c..4c21ff4af3489 100644 --- a/crates/oxc_linter/src/rules/eslint/no_duplicate_case.rs +++ b/crates/oxc_linter/src/rules/eslint/no_duplicate_case.rs @@ -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") @@ -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); } } } @@ -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(); diff --git a/crates/oxc_linter/src/snapshots/no_duplicate_case.snap b/crates/oxc_linter/src/snapshots/no_duplicate_case.snap index 5e52ca9943dcc..91f154b7099cf 100644 --- a/crates/oxc_linter/src/snapshots/no_duplicate_case.snap +++ b/crates/oxc_linter/src/snapshots/no_duplicate_case.snap @@ -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 ╰──── @@ -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 @@ -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