From dd5b0ac2b7e4a3af77f95fd3dbdeabde205cf913 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 19 Aug 2024 00:29:33 +0800 Subject: [PATCH 1/6] feat(linter/eslint-plugin-vitest): implement no-conditional-tests --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/vitest/no_conditional_tests.rs | 130 ++++++++++++++++++ .../src/snapshots/no_conditional_tests.snap | 32 +++++ 3 files changed, 164 insertions(+) create mode 100644 crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs create mode 100644 crates/oxc_linter/src/snapshots/no_conditional_tests.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index ccad7740e77d7..ebed8e21138bc 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -452,6 +452,7 @@ mod promise { } mod vitest { + pub mod no_conditional_tests; pub mod no_import_node_test; pub mod prefer_to_be_falsy; pub mod prefer_to_be_truthy; @@ -866,4 +867,5 @@ oxc_macros::declare_all_lint_rules! { vitest::no_import_node_test, vitest::prefer_to_be_falsy, vitest::prefer_to_be_truthy, + vitest::no_conditional_tests, } diff --git a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs new file mode 100644 index 0000000000000..4ca0c3c9afcba --- /dev/null +++ b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs @@ -0,0 +1,130 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{ + context::LintContext, + rule::Rule, + utils::{collect_possible_jest_call_node, JestFnKind, JestGeneralFnKind, PossibleJestNode}, +}; + +fn no_conditional_tests(span0: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Disallow conditional tests".to_string()) + .with_help("Avoid using if conditions in a test.") + .with_label(span0) +} + +#[derive(Debug, Default, Clone)] +pub struct NoConditionalTests; + +declare_oxc_lint!( + /// ### What it does + /// The rule disallows the use of conditional statements within test cases to ensure that tests are deterministic and clearly readable. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// describe('my tests', () => { + /// if (true) { + /// it('is awesome', () => { + /// doTheThing() + /// }) + /// } + /// }) + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// describe('my tests', () => { + /// it('is awesome', () => { + /// doTheThing() + /// }) + /// }) + /// ``` + NoConditionalTests, + correctness, +); + +impl Rule for NoConditionalTests { + fn run_once(&self, ctx: &LintContext) { + for node in &collect_possible_jest_call_node(ctx) { + run(node, ctx); + } + } +} + +fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) { + let node = possible_jest_node.node; + if let AstKind::CallExpression(call_expr) = node.kind() { + let Some(callee_name) = call_expr.callee_name() else { return }; + + if matches!( + JestFnKind::from(callee_name), + JestFnKind::General(JestGeneralFnKind::Describe | JestGeneralFnKind::Test) + ) { + let has_if_statement = ctx + .nodes() + .iter_parents(node.id()) + .any(|node| matches!(node.kind(), AstKind::IfStatement(_))); + + if has_if_statement { + ctx.diagnostic(no_conditional_tests(call_expr.span)); + } + } + } +} +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r#"test("shows error", () => {});"#, + r#"it("foo", function () {})"#, + "it('foo', () => {}); function myTest() { if ('bar') {} }", + r#"function myFunc(str: string) { + return str; + } + describe("myTest", () => { + it("convert shortened equal filter", () => { + expect( + myFunc("5") + ).toEqual("5"); + }); + });"#, + r#"describe("shows error", () => { + if (1 === 2) { + myFunc(); + } + expect(true).toBe(false); + });"#, + ]; + + let fail = vec![ + r#"describe("shows error", () => { + if(true) { + test("shows error", () => { + expect(true).toBe(true); + }) + } + })"#, + r#" + describe("shows error", () => { + if(true) { + it("shows error", () => { + expect(true).toBe(true); + }) + } + })"#, + r#"describe("errors", () => { + if (Math.random() > 0) { + test("test2", () => { + expect(true).toBeTruthy(); + }); + } + });"#, + ]; + + Tester::new(NoConditionalTests::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_conditional_tests.snap b/crates/oxc_linter/src/snapshots/no_conditional_tests.snap new file mode 100644 index 0000000000000..d5e193f3c2ab3 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_conditional_tests.snap @@ -0,0 +1,32 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests + ╭─[no_conditional_tests.tsx:3:9] + 2 │ if(true) { + 3 │ ╭─▶ test("shows error", () => { + 4 │ │ expect(true).toBe(true); + 5 │ ╰─▶ }) + 6 │ } + ╰──── + help: Avoid using if conditions in a test. + + ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests + ╭─[no_conditional_tests.tsx:4:9] + 3 │ if(true) { + 4 │ ╭─▶ it("shows error", () => { + 5 │ │ expect(true).toBe(true); + 6 │ ╰─▶ }) + 7 │ } + ╰──── + help: Avoid using if conditions in a test. + + ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests + ╭─[no_conditional_tests.tsx:3:9] + 2 │ if (Math.random() > 0) { + 3 │ ╭─▶ test("test2", () => { + 4 │ │ expect(true).toBeTruthy(); + 5 │ ╰─▶ }); + 6 │ } + ╰──── + help: Avoid using if conditions in a test. From bafe5e47d5329c56562fec95a6be465b33ec3faa Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 19 Aug 2024 00:46:21 +0800 Subject: [PATCH 2/6] chore: update snapshots --- .../src/rules/vitest/no_conditional_tests.rs | 11 +++--- .../src/snapshots/no_conditional_tests.snap | 36 +++++++++++-------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs index 4ca0c3c9afcba..61d172795b9df 100644 --- a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs +++ b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs @@ -64,17 +64,20 @@ fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) JestFnKind::from(callee_name), JestFnKind::General(JestGeneralFnKind::Describe | JestGeneralFnKind::Test) ) { - let has_if_statement = ctx + let if_statement_node = ctx .nodes() .iter_parents(node.id()) - .any(|node| matches!(node.kind(), AstKind::IfStatement(_))); + .find(|node| matches!(node.kind(), AstKind::IfStatement(_))); - if has_if_statement { - ctx.diagnostic(no_conditional_tests(call_expr.span)); + let Some(node) = if_statement_node else { return }; + + if let AstKind::IfStatement(if_statement) = node.kind() { + ctx.diagnostic(no_conditional_tests(if_statement.span)); } } } } + #[test] fn test() { use crate::tester::Tester; diff --git a/crates/oxc_linter/src/snapshots/no_conditional_tests.snap b/crates/oxc_linter/src/snapshots/no_conditional_tests.snap index d5e193f3c2ab3..c36e3bef62cee 100644 --- a/crates/oxc_linter/src/snapshots/no_conditional_tests.snap +++ b/crates/oxc_linter/src/snapshots/no_conditional_tests.snap @@ -2,31 +2,37 @@ source: crates/oxc_linter/src/tester.rs --- ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests - ╭─[no_conditional_tests.tsx:3:9] - 2 │ if(true) { - 3 │ ╭─▶ test("shows error", () => { + ╭─[no_conditional_tests.tsx:2:8] + 1 │ describe("shows error", () => { + 2 │ ╭─▶ if(true) { + 3 │ │ test("shows error", () => { 4 │ │ expect(true).toBe(true); - 5 │ ╰─▶ }) - 6 │ } + 5 │ │ }) + 6 │ ╰─▶ } + 7 │ }) ╰──── help: Avoid using if conditions in a test. ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests - ╭─[no_conditional_tests.tsx:4:9] - 3 │ if(true) { - 4 │ ╭─▶ it("shows error", () => { + ╭─[no_conditional_tests.tsx:3:8] + 2 │ describe("shows error", () => { + 3 │ ╭─▶ if(true) { + 4 │ │ it("shows error", () => { 5 │ │ expect(true).toBe(true); - 6 │ ╰─▶ }) - 7 │ } + 6 │ │ }) + 7 │ ╰─▶ } + 8 │ }) ╰──── help: Avoid using if conditions in a test. ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests - ╭─[no_conditional_tests.tsx:3:9] - 2 │ if (Math.random() > 0) { - 3 │ ╭─▶ test("test2", () => { + ╭─[no_conditional_tests.tsx:2:8] + 1 │ describe("errors", () => { + 2 │ ╭─▶ if (Math.random() > 0) { + 3 │ │ test("test2", () => { 4 │ │ expect(true).toBeTruthy(); - 5 │ ╰─▶ }); - 6 │ } + 5 │ │ }); + 6 │ ╰─▶ } + 7 │ }); ╰──── help: Avoid using if conditions in a test. From f864f375504398620ce42816006d54233271c709 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 19 Aug 2024 04:21:25 +0800 Subject: [PATCH 3/6] refactor: use is_type_of_jest_fn_call --- .../src/rules/vitest/no_conditional_tests.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs index 61d172795b9df..0b8a1f1830413 100644 --- a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs +++ b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs @@ -6,7 +6,10 @@ use oxc_span::Span; use crate::{ context::LintContext, rule::Rule, - utils::{collect_possible_jest_call_node, JestFnKind, JestGeneralFnKind, PossibleJestNode}, + utils::{ + collect_possible_jest_call_node, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, + PossibleJestNode, + }, }; fn no_conditional_tests(span0: Span) -> OxcDiagnostic { @@ -58,11 +61,14 @@ impl Rule for NoConditionalTests { fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) { let node = possible_jest_node.node; if let AstKind::CallExpression(call_expr) = node.kind() { - let Some(callee_name) = call_expr.callee_name() else { return }; - - if matches!( - JestFnKind::from(callee_name), - JestFnKind::General(JestGeneralFnKind::Describe | JestGeneralFnKind::Test) + if is_type_of_jest_fn_call( + call_expr, + possible_jest_node, + ctx, + &[ + JestFnKind::General(JestGeneralFnKind::Describe), + JestFnKind::General(JestGeneralFnKind::Test), + ], ) { let if_statement_node = ctx .nodes() From 22eed3a9f8b39303275102fd0c7459bfb2a3c793 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Sun, 18 Aug 2024 19:40:35 -0400 Subject: [PATCH 4/6] Update no_conditional_tests.rs --- crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs index 0b8a1f1830413..65ac6f4dd99c7 100644 --- a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs +++ b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs @@ -13,7 +13,7 @@ use crate::{ }; fn no_conditional_tests(span0: Span) -> OxcDiagnostic { - OxcDiagnostic::warn("Disallow conditional tests".to_string()) + OxcDiagnostic::warn("Avoid having conditionals in tests") .with_help("Avoid using if conditions in a test.") .with_label(span0) } From c0f3c1600bea4e505980efa02f402d3291dd4524 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Sun, 18 Aug 2024 19:40:41 -0400 Subject: [PATCH 5/6] Update no_conditional_tests.rs --- crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs index 65ac6f4dd99c7..a492bd7b171a7 100644 --- a/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs +++ b/crates/oxc_linter/src/rules/vitest/no_conditional_tests.rs @@ -14,7 +14,7 @@ use crate::{ fn no_conditional_tests(span0: Span) -> OxcDiagnostic { OxcDiagnostic::warn("Avoid having conditionals in tests") - .with_help("Avoid using if conditions in a test.") + .with_help("Remove the surrounding if statement.") .with_label(span0) } From 652d75b6002fafd4d816009137cbf18906e140e5 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 19 Aug 2024 08:37:08 +0800 Subject: [PATCH 6/6] chore: update snapshots --- .../src/snapshots/no_conditional_tests.snap | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/oxc_linter/src/snapshots/no_conditional_tests.snap b/crates/oxc_linter/src/snapshots/no_conditional_tests.snap index c36e3bef62cee..eda7c4a9fc8c8 100644 --- a/crates/oxc_linter/src/snapshots/no_conditional_tests.snap +++ b/crates/oxc_linter/src/snapshots/no_conditional_tests.snap @@ -1,7 +1,7 @@ --- source: crates/oxc_linter/src/tester.rs --- - ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests + ⚠ eslint-plugin-vitest(no-conditional-tests): Avoid having conditionals in tests ╭─[no_conditional_tests.tsx:2:8] 1 │ describe("shows error", () => { 2 │ ╭─▶ if(true) { @@ -11,9 +11,9 @@ source: crates/oxc_linter/src/tester.rs 6 │ ╰─▶ } 7 │ }) ╰──── - help: Avoid using if conditions in a test. + help: Remove the surrounding if statement. - ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests + ⚠ eslint-plugin-vitest(no-conditional-tests): Avoid having conditionals in tests ╭─[no_conditional_tests.tsx:3:8] 2 │ describe("shows error", () => { 3 │ ╭─▶ if(true) { @@ -23,9 +23,9 @@ source: crates/oxc_linter/src/tester.rs 7 │ ╰─▶ } 8 │ }) ╰──── - help: Avoid using if conditions in a test. + help: Remove the surrounding if statement. - ⚠ eslint-plugin-vitest(no-conditional-tests): Disallow conditional tests + ⚠ eslint-plugin-vitest(no-conditional-tests): Avoid having conditionals in tests ╭─[no_conditional_tests.tsx:2:8] 1 │ describe("errors", () => { 2 │ ╭─▶ if (Math.random() > 0) { @@ -35,4 +35,4 @@ source: crates/oxc_linter/src/tester.rs 6 │ ╰─▶ } 7 │ }); ╰──── - help: Avoid using if conditions in a test. + help: Remove the surrounding if statement.