From 004ffa082c78cb9b9467df4857b5ae4a941e7c11 Mon Sep 17 00:00:00 2001 From: dalaoshu Date: Mon, 26 Aug 2024 11:00:13 +0800 Subject: [PATCH] feat(linter/vitest): implement `prefer-each` (#5203) Related to #4656 --------- Co-authored-by: Don Isaac --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/vitest/prefer_each.rs | 283 ++++++++++++++++++ .../oxc_linter/src/snapshots/prefer_each.snap | 120 ++++++++ 3 files changed, 405 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..014884d43d4f1 --- /dev/null +++ b/crates/oxc_linter/src/rules/vitest/prefer_each.rs @@ -0,0 +1,283 @@ +use std::collections::HashSet; + +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(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(span) +} + +#[inline] +fn is_in_test(ctx: &LintContext<'_>, id: AstNodeId) -> bool { + ctx.nodes().iter_parents(id).any(|node| { + 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)) + }) +} + +#[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_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 }; + + 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 + ) + ) { + 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 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; + } + _ => {} + } + } + } +} + +#[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..930894635a5ec --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_each.snap @@ -0,0 +1,120 @@ +--- +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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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}`, () => { + ╰──── + 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)); + ╰──── + 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 │ it("only returns numbers that are greater than seven", function () { + ╰──── + 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)); + ╰──── + help: Prefer using `describe.each` rather than a manual loop.