diff --git a/apps/oxlint/src/snapshots/_--vitest-plugin -c fixtures__eslintrc_vitest_replace__eslintrc.json fixtures__eslintrc_vitest_replace__foo.test.js@oxlint.snap b/apps/oxlint/src/snapshots/_--vitest-plugin -c fixtures__eslintrc_vitest_replace__eslintrc.json fixtures__eslintrc_vitest_replace__foo.test.js@oxlint.snap index c2f166b1e391f..ece6aeeb94874 100644 --- a/apps/oxlint/src/snapshots/_--vitest-plugin -c fixtures__eslintrc_vitest_replace__eslintrc.json fixtures__eslintrc_vitest_replace__foo.test.js@oxlint.snap +++ b/apps/oxlint/src/snapshots/_--vitest-plugin -c fixtures__eslintrc_vitest_replace__eslintrc.json fixtures__eslintrc_vitest_replace__foo.test.js@oxlint.snap @@ -23,7 +23,7 @@ working directory: help: Remove the appending `.skip` Found 1 warning and 1 error. -Finished in ms on 1 file with 104 rules using 1 threads. +Finished in ms on 1 file with 105 rules using 1 threads. ---------- CLI result: LintFoundErrors ---------- diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index 25b1d26851c04..e3a112c4d2436 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -4007,6 +4007,11 @@ impl RuleRunner for crate::rules::unicorn::throw_new_error::ThrowNewError { const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; } +impl RuleRunner for crate::rules::vitest::consistent_each_for::ConsistentEachFor { + const NODE_TYPES: Option<&AstTypesBitset> = None; + const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::RunOnJestNode; +} + impl RuleRunner for crate::rules::vitest::consistent_test_filename::ConsistentTestFilename { const NODE_TYPES: Option<&AstTypesBitset> = None; const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::RunOnce; diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 5541cc08adba6..b77241a368b4d 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -667,6 +667,7 @@ pub(crate) mod promise { } pub(crate) mod vitest { + pub mod consistent_each_for; pub mod consistent_test_filename; pub mod consistent_vitest_vi; pub mod hoisted_apis_on_top; @@ -1337,6 +1338,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::switch_case_braces, unicorn::text_encoding_identifier_case, unicorn::throw_new_error, + vitest::consistent_each_for, vitest::consistent_test_filename, vitest::consistent_vitest_vi, vitest::hoisted_apis_on_top, diff --git a/crates/oxc_linter/src/rules/vitest/consistent_each_for.rs b/crates/oxc_linter/src/rules/vitest/consistent_each_for.rs new file mode 100644 index 0000000000000..8d45f615b3197 --- /dev/null +++ b/crates/oxc_linter/src/rules/vitest/consistent_each_for.rs @@ -0,0 +1,417 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{CompactStr, Span}; +use rustc_hash::FxHashMap; +use schemars::JsonSchema; +use serde::Deserialize; + +use crate::{ + context::LintContext, + rule::{DefaultRuleConfig, Rule}, + utils::{JestFnKind, JestGeneralFnKind, PossibleJestNode, parse_general_jest_fn_call}, +}; + +fn consistent_each_for_diagnostic( + span: Span, + fn_kind: &str, + method_used: &str, + method_name: &CompactStr, +) -> OxcDiagnostic { + let message = + format!("`{fn_kind}` can not be used with `.{method_used}` to create parameterized test."); + let help = format!( + "To create parameterized test with `{fn_kind}` function you should use `{method_name}`" + ); + + OxcDiagnostic::warn(message).with_help(help).with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct ConsistentEachFor(Box); + +impl std::ops::Deref for ConsistentEachFor { + type Target = ConsistentEachForConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[derive(Debug, Clone, PartialEq, Eq, JsonSchema, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum MemberNames { + For, + Each, +} + +impl MemberNames { + fn not_allowed_method(&self, method: &str) -> bool { + match self { + MemberNames::For => "each" == method, + MemberNames::Each => "for" == method, + } + } + + fn allowed_method_from_disallowed_method(&self) -> CompactStr { + match self { + MemberNames::For => ".for".into(), + MemberNames::Each => ".each".into(), + } + } +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +enum MatchKind { + Describe, + It, + Test, + Suite, +} + +impl MatchKind { + fn from(name: &str) -> Option { + match name { + "describe" => Some(Self::Describe), + "it" => Some(Self::It), + "test" => Some(Self::Test), + "suite" => Some(Self::Suite), + _ => None, + } + } +} + +#[derive(Debug, Default, Clone)] +pub struct ConsistentEachForConfig { + methods: FxHashMap, +} + +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "camelCase", default)] +struct ConsistentEachForJson { + describe: Option, + suite: Option, + test: Option, + it: Option, +} + +impl ConsistentEachForJson { + fn into_consistent_each_for_config(self) -> ConsistentEachForConfig { + let mut members = FxHashMap::default(); + + if let Some(describe) = self.describe { + members.insert(MatchKind::Describe, describe); + } + + if let Some(it) = self.it { + members.insert(MatchKind::It, it); + } + + if let Some(suite) = self.suite { + members.insert(MatchKind::Suite, suite); + } + + if let Some(test) = self.test { + members.insert(MatchKind::Test, test); + } + + ConsistentEachForConfig { methods: members } + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// This rule ensure consistency on which method used to create parameterized test. + /// This configuration affects to different test function types (`test`, `it`, `describe`, `suite`). + /// + /// ### Why is this bad? + /// + /// Not having a consistent way to create parametrized tests, we rely on the developer to remember that + /// `.for` spread the values as different arguments and `.each` pass the array as an unique argument. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// // { test: 'for' } + /// test.each([[1, 1, 2]])('test', (a, b, expected) => { + /// expect(a + b).toBe(expected) + /// }) + /// + /// // { describe: 'for' } + /// describe.each([[1], [2]])('suite %s', (n) => { + /// test('test', () => {}) + /// }) + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// // { test: 'for' } + /// test.for([[1, 1, 2]])('test', ([a, b, expected]) => { + /// expect(a + b).toBe(expected) + /// }) + /// + /// // { describe: 'for' } + /// describe.for([[1], [2]])('suite %s', ([n]) => { + /// test('test', () => {}) + /// }) + /// ``` + ConsistentEachFor, + vitest, + correctness, + config = ConsistentEachForJson +); + +impl Rule for ConsistentEachFor { + fn from_configuration(value: serde_json::Value) -> Result { + Ok(Self(Box::new( + serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner() + .into_consistent_each_for_config(), + ))) + } + + fn run_on_jest_node<'a, 'c>( + &self, + jest_node: &PossibleJestNode<'a, 'c>, + ctx: &'c LintContext<'a>, + ) { + self.run(jest_node, ctx); + } +} + +impl ConsistentEachFor { + fn run<'a>(&self, possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) { + let node = possible_jest_node.node; + + let AstKind::CallExpression(call_expr) = node.kind() else { + return; + }; + + let Some(jest_fn_call) = parse_general_jest_fn_call(call_expr, possible_jest_node, ctx) + else { + return; + }; + + if !matches!( + jest_fn_call.kind, + JestFnKind::General(JestGeneralFnKind::Describe | JestGeneralFnKind::Test) + ) { + return; + } + + let Some(fn_kind) = MatchKind::from(jest_fn_call.name.as_ref()) else { + return; + }; + + let Some(member_to_check) = self.methods.get(&fn_kind) else { + return; + }; + + let Some(last_method) = jest_fn_call.members.last() else { + return; + }; + + let Some(method_name) = last_method.name() else { + return; + }; + + if member_to_check.not_allowed_method(method_name.as_ref()) { + ctx.diagnostic(consistent_each_for_diagnostic( + last_method.span, + jest_fn_call.name.as_ref(), + method_name.as_ref(), + &member_to_check.allowed_method_from_disallowed_method(), + )); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + /* + * Currently the responsible to set what frameworks are active or not is not `with_vitest_plugin` or oxlint config. + * The code that set what test framewors are active is ContextHost::sniff_for_frameworks, and the current detection lead to a + * a false negative. To detect if the current source code belongs to vitest is based if a `vitest` import exist, if not, assumes + * we are on a possible jest test. On top of that, the method `frameworks::is_jestlike_file` most of the times is going to be true, at least in + * our current situation. So this lead that the ContextHost can have jest and vitest active **at same time**. + * + * This detection isn't compatible on how `parse_general_jest_fn_call` handle if a node is valid or not. To make it simple: + * + * - Jest file: ctx.frameworks().is_jest() is true && ctx.frameworks().is_vitest() is false + * - Vitest file: ctx.frameworks().is_jest() is true && ctx.frameworks().is_vitest is true + * + * And if you are dealing with non compatible modifiers or methods, that only exists in vitest, it will fail as in jest doesn't exist. + * + * In case of dealing with syntax that only exists in vitest, add an import of `vitest` to force the ContextHost to detect we are dealing with vitest. + * This probably will allow reuse allow of the methods that rely on this false negative detection. + */ + macro_rules! vitest_context { + ($test: literal) => { + concat!("import * as vi from 'vitest'\n\n", $test) + }; + } + + let pass = vec![ + (vitest_context!("test.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })"), None), + ( + vitest_context!("test.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })"), + None, + ), + ( + vitest_context!("describe.each([1, 2, 3])('suite', (n) => { test('test', () => {}) })"), + None, + ), + ( + vitest_context!("test.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!( + "test.skip.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!( + "test.only.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!( + "test.concurrent.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!("test.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "test": "for" }])), + ), + ( + vitest_context!( + "test.skip.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "for" }])), + ), + ( + vitest_context!( + "test.only.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "for" }])), + ), + ( + vitest_context!("it.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "it": "each" }])), + ), + ( + vitest_context!("it.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "it": "for" }])), + ), + ( + vitest_context!("describe.each([1, 2, 3])('suite', (n) => { test('test', () => {}) })"), + Some(serde_json::json!([{ "describe": "each" }])), + ), + ( + vitest_context!( + "describe.skip.each([1, 2, 3])('suite', (n) => { test('test', () => {}) })" + ), + Some(serde_json::json!([{ "describe": "each" }])), + ), + ( + vitest_context!( + "describe.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) })" + ), + Some(serde_json::json!([{ "describe": "for" }])), + ), + ( + vitest_context!("suite.each([1, 2, 3])('suite', (n) => { test('test', () => {}) })"), + Some(serde_json::json!([{ "suite": "each" }])), + ), + ( + vitest_context!("suite.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) })"), + Some(serde_json::json!([{ "suite": "for" }])), + ), + ( + vitest_context!( + " + test.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() }) + describe.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) }) + " + ), + Some(serde_json::json!([{ "test": "each", "describe": "for" }])), + ), + ]; + + let fail = vec![ + ( + vitest_context!("test.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!( + "test.skip.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!( + "test.only.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "each" }])), + ), + ( + vitest_context!("test.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "test": "for" }])), + ), + ( + vitest_context!( + "test.skip.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })" + ), + Some(serde_json::json!([{ "test": "for" }])), + ), + ( + vitest_context!("it.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "it": "each" }])), + ), + ( + vitest_context!("it.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() })"), + Some(serde_json::json!([{ "it": "for" }])), + ), + ( + vitest_context!( + "describe.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) })" + ), + Some(serde_json::json!([{ "describe": "each" }])), + ), + ( + vitest_context!("describe.each([1, 2, 3])('suite', (n) => { test('test', () => {}) })"), + Some(serde_json::json!([{ "describe": "for" }])), + ), + ( + vitest_context!("suite.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) })"), + Some(serde_json::json!([{ "suite": "each" }])), + ), + ( + vitest_context!("suite.each([1, 2, 3])('suite', (n) => { test('test', () => {}) })"), + Some(serde_json::json!([{ "suite": "for" }])), + ), + ( + vitest_context!( + " + test.for([1, 2])('test1', ([n]) => {}) + test.for([3, 4])('test2', ([n]) => {}) + " + ), + Some(serde_json::json!([{ "test": "each" }])), + ), + ]; + + Tester::new(ConsistentEachFor::NAME, ConsistentEachFor::PLUGIN, pass, fail) + .with_vitest_plugin(true) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/vitest_consistent_each_for.snap b/crates/oxc_linter/src/snapshots/vitest_consistent_each_for.snap new file mode 100644 index 0000000000000..a7b1034ebf640 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/vitest_consistent_each_for.snap @@ -0,0 +1,108 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:3:6] + 2 │ + 3 │ test.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() }) + · ─── + ╰──── + help: To create parameterized test with `test` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:3:11] + 2 │ + 3 │ test.skip.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() }) + · ─── + ╰──── + help: To create parameterized test with `test` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:3:11] + 2 │ + 3 │ test.only.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() }) + · ─── + ╰──── + help: To create parameterized test with `test` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.each` to create parameterized test. + ╭─[consistent_each_for.tsx:3:6] + 2 │ + 3 │ test.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() }) + · ──── + ╰──── + help: To create parameterized test with `test` function you should use `.for` + + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.each` to create parameterized test. + ╭─[consistent_each_for.tsx:3:11] + 2 │ + 3 │ test.skip.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() }) + · ──── + ╰──── + help: To create parameterized test with `test` function you should use `.for` + + ⚠ eslint-plugin-vitest(consistent-each-for): `it` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:3:4] + 2 │ + 3 │ it.for([1, 2, 3])('test', ([n]) => { expect(n).toBeDefined() }) + · ─── + ╰──── + help: To create parameterized test with `it` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `it` can not be used with `.each` to create parameterized test. + ╭─[consistent_each_for.tsx:3:4] + 2 │ + 3 │ it.each([1, 2, 3])('test', (n) => { expect(n).toBeDefined() }) + · ──── + ╰──── + help: To create parameterized test with `it` function you should use `.for` + + ⚠ eslint-plugin-vitest(consistent-each-for): `describe` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:3:10] + 2 │ + 3 │ describe.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) }) + · ─── + ╰──── + help: To create parameterized test with `describe` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `describe` can not be used with `.each` to create parameterized test. + ╭─[consistent_each_for.tsx:3:10] + 2 │ + 3 │ describe.each([1, 2, 3])('suite', (n) => { test('test', () => {}) }) + · ──── + ╰──── + help: To create parameterized test with `describe` function you should use `.for` + + ⚠ eslint-plugin-vitest(consistent-each-for): `suite` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:3:7] + 2 │ + 3 │ suite.for([1, 2, 3])('suite', ([n]) => { test('test', () => {}) }) + · ─── + ╰──── + help: To create parameterized test with `suite` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `suite` can not be used with `.each` to create parameterized test. + ╭─[consistent_each_for.tsx:3:7] + 2 │ + 3 │ suite.each([1, 2, 3])('suite', (n) => { test('test', () => {}) }) + · ──── + ╰──── + help: To create parameterized test with `suite` function you should use `.for` + + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:4:17] + 3 │ + 4 │ test.for([1, 2])('test1', ([n]) => {}) + · ─── + 5 │ test.for([3, 4])('test2', ([n]) => {}) + ╰──── + help: To create parameterized test with `test` function you should use `.each` + + ⚠ eslint-plugin-vitest(consistent-each-for): `test` can not be used with `.for` to create parameterized test. + ╭─[consistent_each_for.tsx:5:17] + 4 │ test.for([1, 2])('test1', ([n]) => {}) + 5 │ test.for([3, 4])('test2', ([n]) => {}) + · ─── + 6 │ + ╰──── + help: To create parameterized test with `test` function you should use `.each` diff --git a/crates/oxc_linter/src/utils/jest.rs b/crates/oxc_linter/src/utils/jest.rs index af2f7e603414f..ea58fd44e0e79 100644 --- a/crates/oxc_linter/src/utils/jest.rs +++ b/crates/oxc_linter/src/utils/jest.rs @@ -20,7 +20,7 @@ pub use crate::utils::jest::parse_jest_fn::{ mod parse_jest_fn; -const JEST_METHOD_NAMES: [&str; 18] = [ +const JEST_METHOD_NAMES: [&str; 19] = [ "afterAll", "afterEach", "beforeAll", @@ -34,6 +34,7 @@ const JEST_METHOD_NAMES: [&str; 18] = [ "it", "jest", "pending", + "suite", "test", "vi", "xdescribe", @@ -57,7 +58,9 @@ impl JestFnKind { "vi" | "vitest" => Self::General(JestGeneralFnKind::Vitest), "bench" => Self::General(JestGeneralFnKind::Bench), "jest" => Self::General(JestGeneralFnKind::Jest), - "describe" | "fdescribe" | "xdescribe" => Self::General(JestGeneralFnKind::Describe), + "describe" | "fdescribe" | "xdescribe" | "suite" => { + Self::General(JestGeneralFnKind::Describe) + } "fit" | "it" | "test" | "xit" | "xtest" => Self::General(JestGeneralFnKind::Test), "beforeAll" | "beforeEach" | "afterAll" | "afterEach" => { Self::General(JestGeneralFnKind::Hook)