From 8b0032d4af9262433cab76156fb7991a6b29a71c Mon Sep 17 00:00:00 2001 From: Cameron Date: Tue, 21 Nov 2023 02:02:32 +0000 Subject: [PATCH] feat(linter): eslint plugin unicorn: no useless switch case (#1463) --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/unicorn/no_empty_file.rs | 18 +- .../rules/unicorn/no_useless_switch_case.rs | 266 ++++++++++++++++++ .../src/snapshots/no_useless_switch_case.snap | 111 ++++++++ crates/oxc_linter/src/utils/unicorn.rs | 16 +- 5 files changed, 396 insertions(+), 17 deletions(-) create mode 100644 crates/oxc_linter/src/rules/unicorn/no_useless_switch_case.rs create mode 100644 crates/oxc_linter/src/snapshots/no_useless_switch_case.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 70640b618..5f9f90367 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -170,6 +170,7 @@ mod unicorn { pub mod no_unnecessary_await; pub mod no_useless_fallback_in_spread; pub mod no_useless_promise_resolve_reject; + pub mod no_useless_switch_case; pub mod number_literal_case; pub mod prefer_add_event_listener; pub mod prefer_array_flat_map; @@ -331,6 +332,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::no_unnecessary_await, unicorn::no_useless_fallback_in_spread, unicorn::no_useless_promise_resolve_reject, + unicorn::no_useless_switch_case, unicorn::number_literal_case, unicorn::prefer_add_event_listener, unicorn::prefer_array_flat_map, diff --git a/crates/oxc_linter/src/rules/unicorn/no_empty_file.rs b/crates/oxc_linter/src/rules/unicorn/no_empty_file.rs index 1fb9a1cbd..a0c9f5bef 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_empty_file.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_empty_file.rs @@ -1,4 +1,4 @@ -use oxc_ast::{ast::Statement, AstKind}; +use oxc_ast::AstKind; use oxc_diagnostics::{ miette::{self, Diagnostic}, thiserror::Error, @@ -6,7 +6,7 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use crate::{context::LintContext, rule::Rule}; +use crate::{context::LintContext, rule::Rule, utils::is_empty_stmt}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-unicorn(no-empty-file): Empty files are not allowed.")] @@ -53,20 +53,6 @@ impl Rule for NoEmptyFile { } } -fn is_empty_stmt(stmt: &Statement) -> bool { - match stmt { - Statement::BlockStatement(block_stmt) => { - if block_stmt.body.is_empty() || block_stmt.body.iter().all(|node| is_empty_stmt(node)) - { - return true; - } - false - } - Statement::EmptyStatement(_) => true, - _ => false, - } -} - fn has_triple_slash_directive(ctx: &LintContext<'_>) -> bool { for (start, comment) in ctx.semantic().trivias().comments() { if !comment.is_single_line() { diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_switch_case.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_switch_case.rs new file mode 100644 index 000000000..1f17ea762 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_switch_case.rs @@ -0,0 +1,266 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, utils::is_empty_stmt, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement.")] +#[diagnostic( + severity(warning), + help("Consider removing this case or removing the `default` case.") +)] +struct NoUselessSwitchCaseDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoUselessSwitchCase; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows useless default cases in switch statements. + /// + /// ### Why is this bad? + /// + /// An empty case before the last default case is useless. + /// + /// ### Example + /// ```javascript + /// // bad + /// switch (foo) { + /// case 1: + /// default: + /// handleDefaultCase(); + /// break; + /// } + /// // good: + /// switch (foo) { + /// case 1: + /// case 2: + /// handleCase1And2(); + /// break; + /// } + /// ``` + NoUselessSwitchCase, + pedantic +); + +impl Rule for NoUselessSwitchCase { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::SwitchStatement(switch_statement) = node.kind() else { + return; + }; + + let cases = &switch_statement.cases; + + let default_cases = cases.iter().filter(|v| v.test.is_none()).collect::>(); + + if default_cases.len() != 1 { + return; + } + + let default_case = default_cases[0]; + + // Check if the `default` case is the last case + if default_case as *const _ != cases.last().unwrap() as *const _ { + return; + } + + let mut useless_cases = vec![]; + + for case in cases.iter().rev().skip(1) { + if case.consequent.iter().all(|v| is_empty_stmt(v)) { + useless_cases.push(case); + } else { + break; + } + } + + if useless_cases.is_empty() { + return; + } + + for case in useless_cases { + ctx.diagnostic(NoUselessSwitchCaseDiagnostic(case.span)); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r" + switch (foo) { + case a: + case b: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + handleCaseA(); + break; + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + handleCaseA(); + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + break; + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + handleCaseA(); + // Fallthrough + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + default: + handleDefaultCase(); + break; + case b: + handleCaseB(); + break; + } + ", + r" + switch (1) { + // This is not useless + case 1: + default: + console.log('1') + case 1: + console.log('2') + } + ", + r" + switch (1) { + default: + handleDefaultCase1(); + break; + case 1: + default: + handleDefaultCase2(); + break; + } + ", + ]; + + let fail = vec![ + r" + switch (foo) { + case a: + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: { + } + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: { + ;; + { + ;; + { + ;; + } + } + } + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + case (( b )) : + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + case b: + handleCaseAB(); + break; + case d: + case d: + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + case b: + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + // eslint-disable-next-line + case a: + case b: + default: + handleDefaultCase(); + break; + } + ", + r" + switch (foo) { + case a: + // eslint-disable-next-line + case b: + default: + handleDefaultCase(); + break; + } + ", + ]; + + Tester::new_without_config(NoUselessSwitchCase::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_useless_switch_case.snap b/crates/oxc_linter/src/snapshots/no_useless_switch_case.snap new file mode 100644 index 000000000..fed841bb1 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_useless_switch_case.snap @@ -0,0 +1,111 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_useless_switch_case +--- + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:2:1] + 2 │ switch (foo) { + 3 │ case a: + · ─────── + 4 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:2:1] + 2 │ switch (foo) { + 3 │ ╭─▶ case a: { + 4 │ ╰─▶ } + 5 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:2:1] + 2 │ switch (foo) { + 3 │ ╭─▶ case a: { + 4 │ │ ;; + 5 │ │ { + 6 │ │ ;; + 7 │ │ { + 8 │ │ ;; + 9 │ │ } + 10 │ │ } + 11 │ ╰─▶ } + 12 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:3:1] + 3 │ case a: + 4 │ case (( b )) : + · ────────────────────── + 5 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:2:1] + 2 │ switch (foo) { + 3 │ case a: + · ─────── + 4 │ case (( b )) : + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:7:1] + 7 │ case d: + 8 │ case d: + · ─────── + 9 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:6:1] + 6 │ break; + 7 │ case d: + · ─────── + 8 │ case d: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:3:1] + 3 │ case a: + 4 │ case b: + · ─────── + 5 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:2:1] + 2 │ switch (foo) { + 3 │ case a: + · ─────── + 4 │ case b: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:4:1] + 4 │ case a: + 5 │ case b: + · ─────── + 6 │ default: + ╰──── + help: Consider removing this case or removing the `default` case. + + ⚠ eslint-plugin-unicorn(no-useless-switch-case): Useless case in switch statement. + ╭─[no_useless_switch_case.tsx:2:1] + 2 │ switch (foo) { + 3 │ case a: + · ─────── + 4 │ // eslint-disable-next-line + ╰──── + help: Consider removing this case or removing the `default` case. + + diff --git a/crates/oxc_linter/src/utils/unicorn.rs b/crates/oxc_linter/src/utils/unicorn.rs index 4f22005c1..ea8269bc6 100644 --- a/crates/oxc_linter/src/utils/unicorn.rs +++ b/crates/oxc_linter/src/utils/unicorn.rs @@ -1,4 +1,4 @@ -use oxc_ast::ast::Expression; +use oxc_ast::ast::{Expression, Statement}; pub fn is_node_value_not_dom_node(expr: &Expression) -> bool { matches!( @@ -12,3 +12,17 @@ pub fn is_node_value_not_dom_node(expr: &Expression) -> bool { | Expression::StringLiteral(_) ) } + +pub fn is_empty_stmt(stmt: &Statement) -> bool { + match stmt { + Statement::BlockStatement(block_stmt) => { + if block_stmt.body.is_empty() || block_stmt.body.iter().all(|node| is_empty_stmt(node)) + { + return true; + } + false + } + Statement::EmptyStatement(_) => true, + _ => false, + } +}