refactor(linter): use ContentHash for no_duplicate_case; remove calculate_hash (#5648)

closes #5600
This commit is contained in:
Boshen 2024-09-09 14:01:29 +00:00
parent bfe9186611
commit a37c064565
3 changed files with 55 additions and 137 deletions

View file

@ -1,16 +1,8 @@
use core::hash::Hasher;
use oxc_ast::{ast::BindingIdentifier, AstKind}; use oxc_ast::{ast::BindingIdentifier, AstKind};
use oxc_semantic::{AstNode, AstNodeId, SymbolId}; 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 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)] #[allow(clippy::wildcard_imports)]
use oxc_ast::ast::*; use oxc_ast::ast::*;

View file

@ -1,10 +1,9 @@
use oxc_ast::AstKind; use oxc_ast::ast::Expression;
use oxc_diagnostics::OxcDiagnostic; use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint; use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span}; use oxc_span::{cmp::ContentEq, GetSpan, Span};
use rustc_hash::FxHashMap;
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 { fn no_duplicate_case_diagnostic(first: Span, second: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Duplicate case label") OxcDiagnostic::warn("Duplicate case label")
@ -81,19 +80,14 @@ 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>) {
let AstKind::SwitchStatement(ss) = node.kind() else { let Some(ss) = node.kind().as_switch_statement() else { return };
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();
let mut map = FxHashMap::default(); if let Some(prev) = previous_tests.iter().find(|t| t.content_eq(test)) {
map.reserve(ss.cases.len()); ctx.diagnostic(no_duplicate_case_diagnostic(prev.span(), test.span()));
for case in &ss.cases { } else {
if let Some(test) = case.test.as_ref() { previous_tests.push(test);
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()));
}
} }
} }
} }
@ -104,108 +98,39 @@ fn test() {
use crate::tester::Tester; use crate::tester::Tester;
let pass = vec![ 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 2: break; default: break;}",
("var a = 1; switch (a) {case 1: break; case '1': break; default: break;}", None), "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;}", None), "var a = 1; switch (a) {case 1: break; case true: break; default: break;}",
("var a = 1; switch (a) {default: break;}", None), "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, 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;}",
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;}",
), "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, 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,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; }",
), "switch(a) { case toString: 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;}",
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),
]; ];
let fail = vec![ let fail = vec![
( "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;}",
None, "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; switch (a) {case 1: break; case (1): break; case 2: 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;}",
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;}",
), "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'; switch (a) {case '1': break; case '1': break; case '2': break; default: break;}", "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: }",
), "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, 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\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;}",
), "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, 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(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, 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,
),
]; ];
Tester::new(NoDuplicateCase::NAME, pass, fail).test_and_snapshot(); Tester::new(NoDuplicateCase::NAME, pass, fail).test_and_snapshot();

View file

@ -13,7 +13,7 @@ source: crates/oxc_linter/src/tester.rs
⚠ eslint(no-duplicate-case): Duplicate case label ⚠ 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 · │ ╰── is duplicated here
· ╰── This label here · ╰── This label here
╰──── ╰────
@ -110,11 +110,11 @@ source: crates/oxc_linter/src/tester.rs
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Duplicate case label ⚠ 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; } 1 │ switch (a) { case a: break; case b: break; case a: break; case c: break; case a: break; }
· ·
· ╰── is duplicated here · ╰── is duplicated here
· ╰── This label here · ╰── This label here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case
@ -162,13 +162,14 @@ source: crates/oxc_linter/src/tester.rs
help: Remove the duplicated case help: Remove the duplicated case
⚠ eslint(no-duplicate-case): Duplicate case label ⚠ eslint(no-duplicate-case): Duplicate case label
╭─[no_duplicate_case.tsx:1:74] ╭─[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 · ╰── This label here
3 │ │ /* comment */ 2 │ ╭─▶ .p1: break; case p .p
4 │ ├──▶ .p1: break; default: break;} 3 │ │ /* comment */
· ╰───── is duplicated here 4 │ ├─▶ .p1: break; default: break;}
· ╰──── is duplicated here
╰──── ╰────
help: Remove the duplicated case help: Remove the duplicated case