From 4adce6fb45f7cff430955384c8da1dc1bfef9b74 Mon Sep 17 00:00:00 2001 From: cin Date: Sun, 21 Jan 2024 16:06:00 +0800 Subject: [PATCH] feat/linter: (eslint-plugin-jest): no-restricted-matchers (#2090) Rule Detail: [link](https://github.com/jest-community/eslint-plugin-jest/blob/main/src/rules/no-restricted-matchers.ts) --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/jest/no_restricted_matchers.rs | 252 ++++++++++++++++++ .../src/snapshots/no_restricted_matchers.snap | 99 +++++++ 3 files changed, 353 insertions(+) create mode 100644 crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs create mode 100644 crates/oxc_linter/src/snapshots/no_restricted_matchers.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index f2e18d48e..c04deae5b 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -132,6 +132,7 @@ mod jest { pub mod no_interpolation_in_snapshots; pub mod no_jasmine_globals; pub mod no_mocks_import; + pub mod no_restricted_matchers; pub mod no_standalone_expect; pub mod no_test_prefixes; pub mod no_test_return_statement; @@ -404,6 +405,7 @@ oxc_macros::declare_all_lint_rules! { jest::no_interpolation_in_snapshots, jest::no_jasmine_globals, jest::no_mocks_import, + jest::no_restricted_matchers, jest::no_standalone_expect, jest::no_test_prefixes, jest::no_test_return_statement, diff --git a/crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs b/crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs new file mode 100644 index 000000000..5387aab02 --- /dev/null +++ b/crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs @@ -0,0 +1,252 @@ +use crate::{ + context::LintContext, + rule::Rule, + utils::{ + collect_possible_jest_call_node, is_type_of_jest_fn_call, parse_expect_jest_fn_call, + JestFnKind, KnownMemberExpressionProperty, PossibleJestNode, + }, +}; + +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use phf::phf_set; +use rustc_hash::{FxHashMap, FxHasher}; +use std::{collections::HashMap, hash::BuildHasherDefault, path::Path}; + +#[derive(Debug, Error, Diagnostic)] +enum NoRestrictedMatchersDiagnostic { + #[error("eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers")] + #[diagnostic(severity(warning), help("Use of `{0:?}` is disallowed`"))] + RestrictedChain(String, #[label] Span), + #[error("eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers")] + #[diagnostic(severity(warning), help("{0:?}"))] + RestrictedChainWithMessage(String, #[label] Span), +} + +#[derive(Debug, Default, Clone)] +pub struct NoRestrictedMatchers(Box); + +#[derive(Debug, Default, Clone)] +pub struct NoRestrictedMatchersConfig { + restricted_matchers: FxHashMap, +} + +impl std::ops::Deref for NoRestrictedMatchers { + type Target = NoRestrictedMatchersConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Ban specific matchers & modifiers from being used, and can suggest alternatives. + /// + /// ### Example + /// ```javascript + /// + /// it('is false', () => { + /// if this has a modifier (i.e. `not.toBeFalsy`), it would be considered fine + /// expect(a).toBeFalsy(); + /// }); + /// + /// it('resolves', async () => { + /// // all uses of this modifier are disallowed, regardless of matcher + /// await expect(myPromise()).resolves.toBe(true); + /// }); + /// + /// describe('when an error happens', () => { + /// it('does not upload the file', async () => { + /// // all uses of this matcher are disallowed + /// expect(uploadFileMock).not.toHaveBeenCalledWith('file.name'); + /// }); + /// }); + /// + NoRestrictedMatchers, + style, +); + +const MODIFIER_NAME: phf::Set<&'static str> = phf_set!["not", "rejects", "resolves"]; + +impl Rule for NoRestrictedMatchers { + fn from_configuration(value: serde_json::Value) -> Self { + let restricted_matchers = &value + .get(0) + .and_then(serde_json::Value::as_object) + .and_then(Self::compile_restricted_matchers) + .unwrap_or_default(); + + Self(Box::new(NoRestrictedMatchersConfig { + restricted_matchers: restricted_matchers.clone(), + })) + } + + fn run_once(&self, ctx: &LintContext<'_>) { + for possible_jest_node in &collect_possible_jest_call_node(ctx) { + self.run(possible_jest_node, ctx); + } + } +} + +impl NoRestrictedMatchers { + 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; + }; + + if !is_type_of_jest_fn_call(call_expr, possible_jest_node, ctx, &[JestFnKind::Expect]) { + return; + } + + let Some(jest_fn_call) = parse_expect_jest_fn_call(call_expr, possible_jest_node, ctx) + else { + return; + }; + + let members = &jest_fn_call.members; + + if members.is_empty() { + return; + } + + let chain_call = members + .iter() + .filter_map(KnownMemberExpressionProperty::name) + .collect::>() + .join("."); + + let span = Span { + start: members.first().unwrap().span.start, + end: members.last().unwrap().span.end, + }; + + for (restriction, message) in &self.restricted_matchers { + if Self::check_restriction(chain_call.as_str(), restriction.as_str()) { + if message.is_empty() { + ctx.diagnostic(NoRestrictedMatchersDiagnostic::RestrictedChain( + chain_call.clone(), + span, + )); + } else { + ctx.diagnostic(NoRestrictedMatchersDiagnostic::RestrictedChainWithMessage( + message.to_string(), + span, + )); + } + } + } + } + + fn check_restriction(chain_call: &str, restriction: &str) -> bool { + if MODIFIER_NAME.contains(restriction) + || Path::new(restriction) + .extension() + .map_or(false, |ext| ext.eq_ignore_ascii_case("not")) + { + return chain_call.starts_with(restriction); + } + + chain_call == restriction + } + + #[allow(clippy::unnecessary_wraps)] + pub fn compile_restricted_matchers( + matchers: &serde_json::Map, + ) -> Option>> { + Some( + matchers + .iter() + .map(|(key, value)| { + (String::from(key), String::from(value.as_str().unwrap_or_default())) + }) + .collect(), + ) + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("expect(a).toHaveBeenCalled()", None), + ("expect(a).not.toHaveBeenCalled()", None), + ("expect(a).toHaveBeenCalledTimes()", None), + ("expect(a).toHaveBeenCalledWith()", None), + ("expect(a).toHaveBeenLastCalledWith()", None), + ("expect(a).toHaveBeenNthCalledWith()", None), + ("expect(a).toHaveReturned()", None), + ("expect(a).toHaveReturnedTimes()", None), + ("expect(a).toHaveReturnedWith()", None), + ("expect(a).toHaveLastReturnedWith()", None), + ("expect(a).toHaveNthReturnedWith()", None), + ("expect(a).toThrow()", None), + ("expect(a).rejects;", None), + ("expect(a);", None), + ("expect(a).resolves", Some(serde_json::json!([{ "not": null }]))), + ("expect(a).toBe(b)", Some(serde_json::json!([{ "not.toBe": null }]))), + ("expect(a).toBeUndefined(b)", Some(serde_json::json!([{ "toBe": null }]))), + ("expect(a)[\"toBe\"](b)", Some(serde_json::json!([{ "not.toBe": null }]))), + ("expect(a).resolves.not.toBe(b)", Some(serde_json::json!([{ "not": null }]))), + ("expect(a).resolves.not.toBe(b)", Some(serde_json::json!([{ "not.toBe": null }]))), + ( + "expect(uploadFileMock).resolves.toHaveBeenCalledWith('file.name')", + Some( + serde_json::json!([{ "not.toHaveBeenCalledWith": "Use not.toHaveBeenCalled instead" }]), + ), + ), + ( + "expect(uploadFileMock).resolves.not.toHaveBeenCalledWith('file.name')", + Some( + serde_json::json!([{ "not.toHaveBeenCalledWith": "Use not.toHaveBeenCalled instead" }]), + ), + ), + ]; + + let fail = vec![ + ("expect(a).toBe(b)", Some(serde_json::json!([{ "toBe": null }]))), + ("expect(a)[\"toBe\"](b)", Some(serde_json::json!([{ "toBe": null }]))), + ("expect(a).not[x]()", Some(serde_json::json!([{ "not": null }]))), + ("expect(a).not.toBe(b)", Some(serde_json::json!([{ "not": null }]))), + ("expect(a).resolves.toBe(b)", Some(serde_json::json!([{ "resolves": null }]))), + ("expect(a).resolves.not.toBe(b)", Some(serde_json::json!([{ "resolves": null }]))), + ("expect(a).resolves.not.toBe(b)", Some(serde_json::json!([{ "resolves.not": null }]))), + ("expect(a).not.toBe(b)", Some(serde_json::json!([{ "not.toBe": null }]))), + ( + "expect(a).resolves.not.toBe(b)", + Some(serde_json::json!([{ "resolves.not.toBe": null }])), + ), + ( + "expect(a).toBe(b)", + Some(serde_json::json!([{ "toBe": "Prefer `toStrictEqual` instead" }])), + ), + ( + " + test('some test', async () => { + await expect(Promise.resolve(1)).resolves.toBe(1); + }); + ", + Some(serde_json::json!([{ "resolves": "Use `expect(await promise)` instead." }])), + ), + ( + "expect(Promise.resolve({})).rejects.toBeFalsy()", + Some(serde_json::json!([{ "rejects.toBeFalsy": null }])), + ), + ( + "expect(uploadFileMock).not.toHaveBeenCalledWith('file.name')", + Some(serde_json::json!([ + { "not.toHaveBeenCalledWith": "Use not.toHaveBeenCalled instead" }, + ])), + ), + ]; + + Tester::new(NoRestrictedMatchers::NAME, pass, fail).with_jest_plugin(true).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_restricted_matchers.snap b/crates/oxc_linter/src/snapshots/no_restricted_matchers.snap new file mode 100644 index 000000000..3f30bb222 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_restricted_matchers.snap @@ -0,0 +1,99 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 143 +expression: no_restricted_matchers +--- + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).toBe(b) + · ──── + ╰──── + help: Use of `"toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a)["toBe"](b) + · ────── + ╰──── + help: Use of `"toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).not[x]() + · ─── + ╰──── + help: Use of `"not"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).not.toBe(b) + · ──────── + ╰──── + help: Use of `"not.toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).resolves.toBe(b) + · ───────────── + ╰──── + help: Use of `"resolves.toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).resolves.not.toBe(b) + · ───────────────── + ╰──── + help: Use of `"resolves.not.toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).resolves.not.toBe(b) + · ───────────────── + ╰──── + help: Use of `"resolves.not.toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).not.toBe(b) + · ──────── + ╰──── + help: Use of `"not.toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).resolves.not.toBe(b) + · ───────────────── + ╰──── + help: Use of `"resolves.not.toBe"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(a).toBe(b) + · ──── + ╰──── + help: "Prefer `toStrictEqual` instead" + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:2:1] + 2 │ test('some test', async () => { + 3 │ await expect(Promise.resolve(1)).resolves.toBe(1); + · ───────────── + 4 │ }); + ╰──── + help: "Use `expect(await promise)` instead." + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(Promise.resolve({})).rejects.toBeFalsy() + · ───────────────── + ╰──── + help: Use of `"rejects.toBeFalsy"` is disallowed` + + ⚠ eslint-plugin-jest(no-restricted-matchers): Disallow specific matchers & modifiers + ╭─[no_restricted_matchers.tsx:1:1] + 1 │ expect(uploadFileMock).not.toHaveBeenCalledWith('file.name') + · ──────────────────────── + ╰──── + help: "Use not.toHaveBeenCalled instead" + +