From 62b252cfc6353fb1c16ce865895aba594904c81d Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 26 Aug 2024 00:53:40 +0800 Subject: [PATCH 1/3] feat(linter/eslint-plugin-vitest): implement prefer-each --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/vitest/prefer_each.rs | 257 ++++++++++++++++++ .../oxc_linter/src/snapshots/prefer_each.snap | 218 +++++++++++++++ 3 files changed, 477 insertions(+) create mode 100644 crates/oxc_linter/src/rules/vitest/prefer_each.rs create mode 100644 crates/oxc_linter/src/snapshots/prefer_each.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 95075ac8e1ae7..aecdf477c82a8 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -458,6 +458,7 @@ mod promise { mod vitest { pub mod no_conditional_tests; pub mod no_import_node_test; + pub mod prefer_each; pub mod prefer_to_be_falsy; pub mod prefer_to_be_truthy; pub mod require_local_test_context_for_concurrent_snapshots; @@ -874,6 +875,7 @@ oxc_macros::declare_all_lint_rules! { promise::no_return_in_finally, promise::prefer_await_to_then, vitest::no_import_node_test, + vitest::prefer_each, vitest::prefer_to_be_falsy, vitest::prefer_to_be_truthy, vitest::no_conditional_tests, diff --git a/crates/oxc_linter/src/rules/vitest/prefer_each.rs b/crates/oxc_linter/src/rules/vitest/prefer_each.rs new file mode 100644 index 0000000000000..e0662c50c213b --- /dev/null +++ b/crates/oxc_linter/src/rules/vitest/prefer_each.rs @@ -0,0 +1,257 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::{AstNode, AstNodeId}; +use oxc_span::{GetSpan, Span}; + +use crate::{ + context::LintContext, + rule::Rule, + utils::{parse_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode}, +}; + +fn use_prefer_each(span0: Span, fn_name: &str) -> OxcDiagnostic { + OxcDiagnostic::warn("Enforce using `each` rather than manual loops") + .with_help(format!("Prefer using `{fn_name}.each` rather than a manual loop.")) + .with_label(span0) +} + +#[inline] +fn is_in_test(ctx: &LintContext<'_>, id: AstNodeId) -> bool { + ctx.nodes().iter_parents(id).any(|node| { + if let AstKind::CallExpression(ancestor_call_expr) = node.kind() { + if let Some(ancestor_member_expr) = ancestor_call_expr.callee.as_member_expression() { + if let Some(id) = ancestor_member_expr.object().get_identifier_reference() { + return matches!( + JestFnKind::from(id.name.as_str()), + JestFnKind::General(JestGeneralFnKind::Test) + ); + } + return false; + } + } + false + }) +} + +#[derive(Debug, Default, Clone)] +pub struct PreferEach; + +declare_oxc_lint!( + /// ### What it does + /// This rule enforces using `each` rather than manual loops. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// for (const item of items) { + /// describe(item, () => { + /// expect(item).toBe('foo') + /// }) + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// describe.each(items)('item', (item) => { + /// expect(item).toBe('foo') + /// }) + /// ``` + PreferEach, + style, +); + +impl Rule for PreferEach { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let kind = node.kind(); + + if let AstKind::CallExpression(call_expr) = kind { + let Some(vitest_fn_call) = + parse_jest_fn_call(call_expr, &PossibleJestNode { node, original: None }, ctx) + else { + return; + }; + + if matches!( + vitest_fn_call.kind(), + JestFnKind::General( + JestGeneralFnKind::Describe | JestGeneralFnKind::Hook | JestGeneralFnKind::Test + ) + ) { + for parent_node in ctx.nodes().iter_parents(node.id()).skip(1) { + match parent_node.kind() { + AstKind::CallExpression(_) => { + return; + } + AstKind::ForStatement(_) + | AstKind::ForInStatement(_) + | AstKind::ForOfStatement(_) => { + if !is_in_test(ctx, parent_node.id()) { + let fn_name = if matches!( + vitest_fn_call.kind(), + JestFnKind::General(JestGeneralFnKind::Test) + ) { + "it" + } else { + "describe" + }; + + ctx.diagnostic(use_prefer_each(parent_node.span(), fn_name)); + } + + break; + } + _ => (), + } + } + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r#"it("is true", () => { expect(true).toBe(false) });"#, + r#"it.each(getNumbers())("only returns numbers that are greater than seven", number => { + expect(number).toBeGreaterThan(7); + });"#, + r#"it("returns numbers that are greater than five", function () { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(5); + } + });"#, + r#"it("returns things that are less than ten", function () { + for (const thing in things) { + expect(thing).toBeLessThan(10); + } + });"#, + r#"it("only returns numbers that are greater than seven", function () { + const numbers = getNumbers(); + + for (let i = 0; i < numbers.length; i++) { + expect(numbers[i]).toBeGreaterThan(7); + } + });"#, + ]; + + let fail = vec![ + " for (const [input, expected] of data) { + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }", + " for (const [input, expected] of data) { + describe(`when the input is ${input}`, () => { + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }); + }", + "for (const [input, expected] of data) { + describe(`when the input is ${input}`, () => { + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }); + } + + for (const [input, expected] of data) { + it.skip(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }", + "for (const [input, expected] of data) { + it.skip(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }", + "it('is true', () => { + expect(true).toBe(false); + }); + + for (const [input, expected] of data) { + it.skip(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }", + " for (const [input, expected] of data) { + it.skip(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + } + + it('is true', () => { + expect(true).toBe(false); + });", + " it('is true', () => { + expect(true).toBe(false); + }); + + for (const [input, expected] of data) { + it.skip(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + } + + it('is true', () => { + expect(true).toBe(false); + });", + "for (const [input, expected] of data) { + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }", + "for (const [input, expected] of data) { + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + } + + for (const [input, expected] of data) { + it(`results in ${expected}`, () => { + expect(fn(input)).toBe(expected) + }); + }", + "for (const [input, expected] of data) { + beforeEach(() => setupSomething(input)); + + test(`results in ${expected}`, () => { + expect(doSomething()).toBe(expected) + }); + }", + r#" + for (const [input, expected] of data) { + it("only returns numbers that are greater than seven", function () { + const numbers = getNumbers(input); + + for (let i = 0; i < numbers.length; i++) { + expect(numbers[i]).toBeGreaterThan(7); + } + }); + } + "#, + r#" + for (const [input, expected] of data) { + beforeEach(() => setupSomething(input)); + + it("only returns numbers that are greater than seven", function () { + const numbers = getNumbers(); + + for (let i = 0; i < numbers.length; i++) { + expect(numbers[i]).toBeGreaterThan(7); + } + }); + } + "#, + ]; + + Tester::new(PreferEach::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/prefer_each.snap b/crates/oxc_linter/src/snapshots/prefer_each.snap new file mode 100644 index 0000000000000..cc70efe4dc642 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_each.snap @@ -0,0 +1,218 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:3] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ it(`results in ${expected}`, () => { + 3 │ │ expect(fn(input)).toBe(expected) + 4 │ │ }); + 5 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:2] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ describe(`when the input is ${input}`, () => { + 3 │ │ it(`results in ${expected}`, () => { + 4 │ │ expect(fn(input)).toBe(expected) + 5 │ │ }); + 6 │ │ }); + 7 │ ╰─▶ } + ╰──── + help: Prefer using `describe.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ describe(`when the input is ${input}`, () => { + 3 │ │ it(`results in ${expected}`, () => { + 4 │ │ expect(fn(input)).toBe(expected) + 5 │ │ }); + 6 │ │ }); + 7 │ ╰─▶ } + 8 │ + ╰──── + help: Prefer using `describe.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:9:11] + 8 │ + 9 │ ╭─▶ for (const [input, expected] of data) { + 10 │ │ it.skip(`results in ${expected}`, () => { + 11 │ │ expect(fn(input)).toBe(expected) + 12 │ │ }); + 13 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ it.skip(`results in ${expected}`, () => { + 3 │ │ expect(fn(input)).toBe(expected) + 4 │ │ }); + 5 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:5:11] + 4 │ + 5 │ ╭─▶ for (const [input, expected] of data) { + 6 │ │ it.skip(`results in ${expected}`, () => { + 7 │ │ expect(fn(input)).toBe(expected) + 8 │ │ }); + 9 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:2] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ it.skip(`results in ${expected}`, () => { + 3 │ │ expect(fn(input)).toBe(expected) + 4 │ │ }); + 5 │ ╰─▶ } + 6 │ + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:5:11] + 4 │ + 5 │ ╭─▶ for (const [input, expected] of data) { + 6 │ │ it.skip(`results in ${expected}`, () => { + 7 │ │ expect(fn(input)).toBe(expected) + 8 │ │ }); + 9 │ ╰─▶ } + 10 │ + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ it(`results in ${expected}`, () => { + 3 │ │ expect(fn(input)).toBe(expected) + 4 │ │ }); + 5 │ │ + 6 │ │ it(`results in ${expected}`, () => { + 7 │ │ expect(fn(input)).toBe(expected) + 8 │ │ }); + 9 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ it(`results in ${expected}`, () => { + 3 │ │ expect(fn(input)).toBe(expected) + 4 │ │ }); + 5 │ │ + 6 │ │ it(`results in ${expected}`, () => { + 7 │ │ expect(fn(input)).toBe(expected) + 8 │ │ }); + 9 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ it(`results in ${expected}`, () => { + 3 │ │ expect(fn(input)).toBe(expected) + 4 │ │ }); + 5 │ ╰─▶ } + 6 │ + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:7:11] + 6 │ + 7 │ ╭─▶ for (const [input, expected] of data) { + 8 │ │ it(`results in ${expected}`, () => { + 9 │ │ expect(fn(input)).toBe(expected) + 10 │ │ }); + 11 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ beforeEach(() => setupSomething(input)); + 3 │ │ + 4 │ │ test(`results in ${expected}`, () => { + 5 │ │ expect(doSomething()).toBe(expected) + 6 │ │ }); + 7 │ ╰─▶ } + ╰──── + help: Prefer using `describe.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:1:1] + 1 │ ╭─▶ for (const [input, expected] of data) { + 2 │ │ beforeEach(() => setupSomething(input)); + 3 │ │ + 4 │ │ test(`results in ${expected}`, () => { + 5 │ │ expect(doSomething()).toBe(expected) + 6 │ │ }); + 7 │ ╰─▶ } + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:2:11] + 1 │ + 2 │ ╭─▶ for (const [input, expected] of data) { + 3 │ │ it("only returns numbers that are greater than seven", function () { + 4 │ │ const numbers = getNumbers(input); + 5 │ │ + 6 │ │ for (let i = 0; i < numbers.length; i++) { + 7 │ │ expect(numbers[i]).toBeGreaterThan(7); + 8 │ │ } + 9 │ │ }); + 10 │ ╰─▶ } + 11 │ + ╰──── + help: Prefer using `it.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:2:11] + 1 │ + 2 │ ╭─▶ for (const [input, expected] of data) { + 3 │ │ beforeEach(() => setupSomething(input)); + 4 │ │ + 5 │ │ it("only returns numbers that are greater than seven", function () { + 6 │ │ const numbers = getNumbers(); + 7 │ │ + 8 │ │ for (let i = 0; i < numbers.length; i++) { + 9 │ │ expect(numbers[i]).toBeGreaterThan(7); + 10 │ │ } + 11 │ │ }); + 12 │ ╰─▶ } + 13 │ + ╰──── + help: Prefer using `describe.each` rather than a manual loop. + + ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops + ╭─[prefer_each.tsx:2:11] + 1 │ + 2 │ ╭─▶ for (const [input, expected] of data) { + 3 │ │ beforeEach(() => setupSomething(input)); + 4 │ │ + 5 │ │ it("only returns numbers that are greater than seven", function () { + 6 │ │ const numbers = getNumbers(); + 7 │ │ + 8 │ │ for (let i = 0; i < numbers.length; i++) { + 9 │ │ expect(numbers[i]).toBeGreaterThan(7); + 10 │ │ } + 11 │ │ }); + 12 │ ╰─▶ } + 13 │ + ╰──── + help: Prefer using `it.each` rather than a manual loop. From 7988c3c4487ed2a2b296205766eb142bde2a67a8 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 26 Aug 2024 09:46:53 +0800 Subject: [PATCH 2/3] chore: follow the suggestions --- .../src/rules/vitest/prefer_each.rs | 95 ++++--- .../oxc_linter/src/snapshots/prefer_each.snap | 243 +++++++----------- 2 files changed, 137 insertions(+), 201 deletions(-) diff --git a/crates/oxc_linter/src/rules/vitest/prefer_each.rs b/crates/oxc_linter/src/rules/vitest/prefer_each.rs index e0662c50c213b..e8bb6479acfb3 100644 --- a/crates/oxc_linter/src/rules/vitest/prefer_each.rs +++ b/crates/oxc_linter/src/rules/vitest/prefer_each.rs @@ -10,27 +10,24 @@ use crate::{ utils::{parse_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode}, }; -fn use_prefer_each(span0: Span, fn_name: &str) -> OxcDiagnostic { +fn use_prefer_each(span: Span, fn_name: &str) -> OxcDiagnostic { OxcDiagnostic::warn("Enforce using `each` rather than manual loops") .with_help(format!("Prefer using `{fn_name}.each` rather than a manual loop.")) - .with_label(span0) + .with_label(span) } #[inline] fn is_in_test(ctx: &LintContext<'_>, id: AstNodeId) -> bool { ctx.nodes().iter_parents(id).any(|node| { - if let AstKind::CallExpression(ancestor_call_expr) = node.kind() { - if let Some(ancestor_member_expr) = ancestor_call_expr.callee.as_member_expression() { - if let Some(id) = ancestor_member_expr.object().get_identifier_reference() { - return matches!( - JestFnKind::from(id.name.as_str()), - JestFnKind::General(JestGeneralFnKind::Test) - ); - } - return false; - } - } - false + let AstKind::CallExpression(ancestor_call_expr) = node.kind() else { return false }; + let Some(ancestor_member_expr) = ancestor_call_expr.callee.as_member_expression() else { + return false; + }; + let Some(id) = ancestor_member_expr.object().get_identifier_reference() else { + return false; + }; + + matches!(JestFnKind::from(id.name.as_str()), JestFnKind::General(JestGeneralFnKind::Test)) }) } @@ -66,45 +63,47 @@ impl Rule for PreferEach { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let kind = node.kind(); - if let AstKind::CallExpression(call_expr) = kind { - let Some(vitest_fn_call) = - parse_jest_fn_call(call_expr, &PossibleJestNode { node, original: None }, ctx) - else { - return; - }; + let AstKind::CallExpression(call_expr) = kind else { return }; - if matches!( - vitest_fn_call.kind(), - JestFnKind::General( - JestGeneralFnKind::Describe | JestGeneralFnKind::Hook | JestGeneralFnKind::Test - ) - ) { - for parent_node in ctx.nodes().iter_parents(node.id()).skip(1) { - match parent_node.kind() { - AstKind::CallExpression(_) => { - return; - } - AstKind::ForStatement(_) - | AstKind::ForInStatement(_) - | AstKind::ForOfStatement(_) => { - if !is_in_test(ctx, parent_node.id()) { - let fn_name = if matches!( - vitest_fn_call.kind(), - JestFnKind::General(JestGeneralFnKind::Test) - ) { - "it" - } else { - "describe" - }; + let Some(vitest_fn_call) = + parse_jest_fn_call(call_expr, &PossibleJestNode { node, original: None }, ctx) + else { + return; + }; - ctx.diagnostic(use_prefer_each(parent_node.span(), fn_name)); - } + if !matches!( + vitest_fn_call.kind(), + JestFnKind::General( + JestGeneralFnKind::Describe | JestGeneralFnKind::Hook | JestGeneralFnKind::Test + ) + ) { + return; + } + + for parent_node in ctx.nodes().iter_parents(node.id()).skip(1) { + match parent_node.kind() { + AstKind::CallExpression(_) => { + return; + } + AstKind::ForStatement(_) + | AstKind::ForInStatement(_) + | AstKind::ForOfStatement(_) => { + if !is_in_test(ctx, parent_node.id()) { + let fn_name = if matches!( + vitest_fn_call.kind(), + JestFnKind::General(JestGeneralFnKind::Test) + ) { + "it" + } else { + "describe" + }; - break; - } - _ => (), + ctx.diagnostic(use_prefer_each(call_expr.callee.span(), fn_name)); } + + break; } + _ => {} } } } diff --git a/crates/oxc_linter/src/snapshots/prefer_each.snap b/crates/oxc_linter/src/snapshots/prefer_each.snap index cc70efe4dc642..4c05fbae29151 100644 --- a/crates/oxc_linter/src/snapshots/prefer_each.snap +++ b/crates/oxc_linter/src/snapshots/prefer_each.snap @@ -2,217 +2,154 @@ source: crates/oxc_linter/src/tester.rs --- ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:3] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ it(`results in ${expected}`, () => { - 3 │ │ expect(fn(input)).toBe(expected) - 4 │ │ }); - 5 │ ╰─▶ } + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ it(`results in ${expected}`, () => { + · ── + 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:2] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ describe(`when the input is ${input}`, () => { - 3 │ │ it(`results in ${expected}`, () => { - 4 │ │ expect(fn(input)).toBe(expected) - 5 │ │ }); - 6 │ │ }); - 7 │ ╰─▶ } + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ describe(`when the input is ${input}`, () => { + · ──────── + 3 │ it(`results in ${expected}`, () => { ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ describe(`when the input is ${input}`, () => { - 3 │ │ it(`results in ${expected}`, () => { - 4 │ │ expect(fn(input)).toBe(expected) - 5 │ │ }); - 6 │ │ }); - 7 │ ╰─▶ } - 8 │ + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ describe(`when the input is ${input}`, () => { + · ──────── + 3 │ it(`results in ${expected}`, () => { ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:9:11] - 8 │ - 9 │ ╭─▶ for (const [input, expected] of data) { - 10 │ │ it.skip(`results in ${expected}`, () => { - 11 │ │ expect(fn(input)).toBe(expected) - 12 │ │ }); - 13 │ ╰─▶ } + ╭─[prefer_each.tsx:10:10] + 9 │ for (const [input, expected] of data) { + 10 │ it.skip(`results in ${expected}`, () => { + · ─────── + 11 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ it.skip(`results in ${expected}`, () => { - 3 │ │ expect(fn(input)).toBe(expected) - 4 │ │ }); - 5 │ ╰─▶ } + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ it.skip(`results in ${expected}`, () => { + · ─────── + 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:5:11] - 4 │ - 5 │ ╭─▶ for (const [input, expected] of data) { - 6 │ │ it.skip(`results in ${expected}`, () => { - 7 │ │ expect(fn(input)).toBe(expected) - 8 │ │ }); - 9 │ ╰─▶ } + ╭─[prefer_each.tsx:6:10] + 5 │ for (const [input, expected] of data) { + 6 │ it.skip(`results in ${expected}`, () => { + · ─────── + 7 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:2] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ it.skip(`results in ${expected}`, () => { - 3 │ │ expect(fn(input)).toBe(expected) - 4 │ │ }); - 5 │ ╰─▶ } - 6 │ + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ it.skip(`results in ${expected}`, () => { + · ─────── + 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:5:11] - 4 │ - 5 │ ╭─▶ for (const [input, expected] of data) { - 6 │ │ it.skip(`results in ${expected}`, () => { - 7 │ │ expect(fn(input)).toBe(expected) - 8 │ │ }); - 9 │ ╰─▶ } - 10 │ - ╰──── + ╭─[prefer_each.tsx:6:10] + 5 │ for (const [input, expected] of data) { + 6 │ it.skip(`results in ${expected}`, () => { + · ─────── + 7 │ expect(fn(input)).toBe(expected) + ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ it(`results in ${expected}`, () => { - 3 │ │ expect(fn(input)).toBe(expected) - 4 │ │ }); - 5 │ │ - 6 │ │ it(`results in ${expected}`, () => { - 7 │ │ expect(fn(input)).toBe(expected) - 8 │ │ }); - 9 │ ╰─▶ } + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ it(`results in ${expected}`, () => { + · ── + 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ it(`results in ${expected}`, () => { - 3 │ │ expect(fn(input)).toBe(expected) - 4 │ │ }); - 5 │ │ - 6 │ │ it(`results in ${expected}`, () => { - 7 │ │ expect(fn(input)).toBe(expected) - 8 │ │ }); - 9 │ ╰─▶ } + ╭─[prefer_each.tsx:6:10] + 5 │ + 6 │ it(`results in ${expected}`, () => { + · ── + 7 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ it(`results in ${expected}`, () => { - 3 │ │ expect(fn(input)).toBe(expected) - 4 │ │ }); - 5 │ ╰─▶ } - 6 │ + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ it(`results in ${expected}`, () => { + · ── + 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:7:11] - 6 │ - 7 │ ╭─▶ for (const [input, expected] of data) { - 8 │ │ it(`results in ${expected}`, () => { - 9 │ │ expect(fn(input)).toBe(expected) - 10 │ │ }); - 11 │ ╰─▶ } - ╰──── + ╭─[prefer_each.tsx:8:10] + 7 │ for (const [input, expected] of data) { + 8 │ it(`results in ${expected}`, () => { + · ── + 9 │ expect(fn(input)).toBe(expected) + ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ beforeEach(() => setupSomething(input)); - 3 │ │ - 4 │ │ test(`results in ${expected}`, () => { - 5 │ │ expect(doSomething()).toBe(expected) - 6 │ │ }); - 7 │ ╰─▶ } + ╭─[prefer_each.tsx:2:10] + 1 │ for (const [input, expected] of data) { + 2 │ beforeEach(() => setupSomething(input)); + · ────────── + 3 │ ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:1:1] - 1 │ ╭─▶ for (const [input, expected] of data) { - 2 │ │ beforeEach(() => setupSomething(input)); - 3 │ │ - 4 │ │ test(`results in ${expected}`, () => { - 5 │ │ expect(doSomething()).toBe(expected) - 6 │ │ }); - 7 │ ╰─▶ } + ╭─[prefer_each.tsx:4:10] + 3 │ + 4 │ test(`results in ${expected}`, () => { + · ──── + 5 │ expect(doSomething()).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:11] - 1 │ - 2 │ ╭─▶ for (const [input, expected] of data) { - 3 │ │ it("only returns numbers that are greater than seven", function () { - 4 │ │ const numbers = getNumbers(input); - 5 │ │ - 6 │ │ for (let i = 0; i < numbers.length; i++) { - 7 │ │ expect(numbers[i]).toBeGreaterThan(7); - 8 │ │ } - 9 │ │ }); - 10 │ ╰─▶ } - 11 │ - ╰──── + ╭─[prefer_each.tsx:3:10] + 2 │ for (const [input, expected] of data) { + 3 │ it("only returns numbers that are greater than seven", function () { + · ── + 4 │ const numbers = getNumbers(input); + ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:11] - 1 │ - 2 │ ╭─▶ for (const [input, expected] of data) { - 3 │ │ beforeEach(() => setupSomething(input)); - 4 │ │ - 5 │ │ it("only returns numbers that are greater than seven", function () { - 6 │ │ const numbers = getNumbers(); - 7 │ │ - 8 │ │ for (let i = 0; i < numbers.length; i++) { - 9 │ │ expect(numbers[i]).toBeGreaterThan(7); - 10 │ │ } - 11 │ │ }); - 12 │ ╰─▶ } - 13 │ - ╰──── + ╭─[prefer_each.tsx:3:10] + 2 │ for (const [input, expected] of data) { + 3 │ beforeEach(() => setupSomething(input)); + · ────────── + 4 │ + ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:11] - 1 │ - 2 │ ╭─▶ for (const [input, expected] of data) { - 3 │ │ beforeEach(() => setupSomething(input)); - 4 │ │ - 5 │ │ it("only returns numbers that are greater than seven", function () { - 6 │ │ const numbers = getNumbers(); - 7 │ │ - 8 │ │ for (let i = 0; i < numbers.length; i++) { - 9 │ │ expect(numbers[i]).toBeGreaterThan(7); - 10 │ │ } - 11 │ │ }); - 12 │ ╰─▶ } - 13 │ - ╰──── + ╭─[prefer_each.tsx:5:10] + 4 │ + 5 │ it("only returns numbers that are greater than seven", function () { + · ── + 6 │ const numbers = getNumbers(); + ╰──── help: Prefer using `it.each` rather than a manual loop. From 053676e967525f495c4b80e8b08699e62b749fc2 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Mon, 26 Aug 2024 10:06:09 +0800 Subject: [PATCH 3/3] chore: avoid unnecessary repetition of warnings --- .../src/rules/vitest/prefer_each.rs | 51 +++++++-- .../oxc_linter/src/snapshots/prefer_each.snap | 103 ++++++------------ 2 files changed, 73 insertions(+), 81 deletions(-) diff --git a/crates/oxc_linter/src/rules/vitest/prefer_each.rs b/crates/oxc_linter/src/rules/vitest/prefer_each.rs index e8bb6479acfb3..014884d43d4f1 100644 --- a/crates/oxc_linter/src/rules/vitest/prefer_each.rs +++ b/crates/oxc_linter/src/rules/vitest/prefer_each.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use oxc_ast::AstKind; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -60,7 +62,16 @@ declare_oxc_lint!( ); impl Rule for PreferEach { - fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + fn run_once(&self, ctx: &LintContext<'_>) { + let mut skip = HashSet::::new(); + ctx.nodes().iter().for_each(|node| { + Self::run(node, ctx, &mut skip); + }); + } +} + +impl PreferEach { + fn run<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>, skip: &mut HashSet) { let kind = node.kind(); let AstKind::CallExpression(call_expr) = kind else { return }; @@ -88,19 +99,35 @@ impl Rule for PreferEach { AstKind::ForStatement(_) | AstKind::ForInStatement(_) | AstKind::ForOfStatement(_) => { - if !is_in_test(ctx, parent_node.id()) { - let fn_name = if matches!( - vitest_fn_call.kind(), - JestFnKind::General(JestGeneralFnKind::Test) - ) { - "it" - } else { - "describe" - }; - - ctx.diagnostic(use_prefer_each(call_expr.callee.span(), fn_name)); + if skip.contains(&parent_node.id()) || is_in_test(ctx, parent_node.id()) { + return; } + let fn_name = if matches!( + vitest_fn_call.kind(), + JestFnKind::General(JestGeneralFnKind::Test) + ) { + "it" + } else { + "describe" + }; + + let span = match parent_node.kind() { + AstKind::ForStatement(statement) => { + Span::new(statement.span.start, statement.body.span().start) + } + AstKind::ForInStatement(statement) => { + Span::new(statement.span.start, statement.body.span().start) + } + AstKind::ForOfStatement(statement) => { + Span::new(statement.span.start, statement.body.span().start) + } + _ => unreachable!(), + }; + + skip.insert(parent_node.id()); + ctx.diagnostic(use_prefer_each(span, fn_name)); + break; } _ => {} diff --git a/crates/oxc_linter/src/snapshots/prefer_each.snap b/crates/oxc_linter/src/snapshots/prefer_each.snap index 4c05fbae29151..930894635a5ec 100644 --- a/crates/oxc_linter/src/snapshots/prefer_each.snap +++ b/crates/oxc_linter/src/snapshots/prefer_each.snap @@ -2,154 +2,119 @@ source: crates/oxc_linter/src/tester.rs --- ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:3] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ it(`results in ${expected}`, () => { - · ── - 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:2] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ describe(`when the input is ${input}`, () => { - · ──────── - 3 │ it(`results in ${expected}`, () => { ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:1] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ describe(`when the input is ${input}`, () => { - · ──────── - 3 │ it(`results in ${expected}`, () => { ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:10:10] + ╭─[prefer_each.tsx:9:11] + 8 │ 9 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 10 │ it.skip(`results in ${expected}`, () => { - · ─────── - 11 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:1] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ it.skip(`results in ${expected}`, () => { - · ─────── - 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:6:10] + ╭─[prefer_each.tsx:5:11] + 4 │ 5 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 6 │ it.skip(`results in ${expected}`, () => { - · ─────── - 7 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:2] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ it.skip(`results in ${expected}`, () => { - · ─────── - 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:6:10] + ╭─[prefer_each.tsx:5:11] + 4 │ 5 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 6 │ it.skip(`results in ${expected}`, () => { - · ─────── - 7 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:1] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ it(`results in ${expected}`, () => { - · ── - 3 │ expect(fn(input)).toBe(expected) - ╰──── - help: Prefer using `it.each` rather than a manual loop. - - ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:6:10] - 5 │ - 6 │ it(`results in ${expected}`, () => { - · ── - 7 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:1] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ it(`results in ${expected}`, () => { - · ── - 3 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:8:10] + ╭─[prefer_each.tsx:7:11] + 6 │ 7 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 8 │ it(`results in ${expected}`, () => { - · ── - 9 │ expect(fn(input)).toBe(expected) ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:2:10] + ╭─[prefer_each.tsx:1:1] 1 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 2 │ beforeEach(() => setupSomething(input)); - · ────────── - 3 │ ╰──── help: Prefer using `describe.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:4:10] - 3 │ - 4 │ test(`results in ${expected}`, () => { - · ──── - 5 │ expect(doSomething()).toBe(expected) - ╰──── - help: Prefer using `it.each` rather than a manual loop. - - ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:3:10] + ╭─[prefer_each.tsx:2:11] + 1 │ 2 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 3 │ it("only returns numbers that are greater than seven", function () { - · ── - 4 │ const numbers = getNumbers(input); ╰──── help: Prefer using `it.each` rather than a manual loop. ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:3:10] + ╭─[prefer_each.tsx:2:11] + 1 │ 2 │ for (const [input, expected] of data) { + · ────────────────────────────────────── 3 │ beforeEach(() => setupSomething(input)); - · ────────── - 4 │ ╰──── help: Prefer using `describe.each` rather than a manual loop. - - ⚠ eslint-plugin-vitest(prefer-each): Enforce using `each` rather than manual loops - ╭─[prefer_each.tsx:5:10] - 4 │ - 5 │ it("only returns numbers that are greater than seven", function () { - · ── - 6 │ const numbers = getNumbers(); - ╰──── - help: Prefer using `it.each` rather than a manual loop.