From c509a21a1f6cddd8df79dd72de8e393ea5f555ff Mon Sep 17 00:00:00 2001 From: dalaoshu Date: Fri, 9 Aug 2024 13:17:45 +0800 Subject: [PATCH] feat(linter/eslint-plugin-vitest): implement prefer-to-be-falsy (#4770) Related to https://github.com/oxc-project/oxc/issues/4656 --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/vitest/prefer_to_be_falsy.rs | 97 +++++++++++++++++++ .../src/rules/vitest/prefer_to_be_truthy.rs | 65 +------------ .../src/snapshots/prefer_to_be_falsy.snap | 44 +++++++++ crates/oxc_linter/src/utils/vitest.rs | 66 ++++++++++++- 5 files changed, 209 insertions(+), 65 deletions(-) create mode 100644 crates/oxc_linter/src/rules/vitest/prefer_to_be_falsy.rs create mode 100644 crates/oxc_linter/src/snapshots/prefer_to_be_falsy.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 56e650eb547d8..76460f1d8c189 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -449,6 +449,7 @@ mod promise { mod vitest { pub mod no_import_node_test; + pub mod prefer_to_be_falsy; pub mod prefer_to_be_truthy; } @@ -855,5 +856,6 @@ oxc_macros::declare_all_lint_rules! { promise::no_new_statics, promise::param_names, vitest::no_import_node_test, + vitest::prefer_to_be_falsy, vitest::prefer_to_be_truthy, } diff --git a/crates/oxc_linter/src/rules/vitest/prefer_to_be_falsy.rs b/crates/oxc_linter/src/rules/vitest/prefer_to_be_falsy.rs new file mode 100644 index 0000000000000..edd060a8da797 --- /dev/null +++ b/crates/oxc_linter/src/rules/vitest/prefer_to_be_falsy.rs @@ -0,0 +1,97 @@ +use oxc_macros::declare_oxc_lint; + +use crate::{ + context::LintContext, + rule::Rule, + utils::{collect_possible_jest_call_node, prefer_to_be_simply_bool}, +}; + +#[derive(Debug, Default, Clone)] +pub struct PreferToBeFalsy; + +declare_oxc_lint!( + /// ### What it does + /// + /// This rule warns when `toBe(false)` is used with `expect` or `expectTypeOf`. With `--fix`, it will be replaced with `toBeFalsy()`. + /// + /// ### Examples + /// + /// ```javascript + /// // bad + /// expect(foo).toBe(false) + /// expectTypeOf(foo).toBe(false) + /// + /// // good + /// expect(foo).toBeFalsy() + /// expectTypeOf(foo).toBeFalsy() + /// ``` + PreferToBeFalsy, + style, + fix +); + +impl Rule for PreferToBeFalsy { + fn run_once(&self, ctx: &LintContext) { + for possible_vitest_node in &collect_possible_jest_call_node(ctx) { + prefer_to_be_simply_bool(possible_vitest_node, ctx, false); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "[].push(false)", + r#"expect("something");"#, + "expect(true).toBeTrue();", + "expect(false).toBeTrue();", + "expect(false).toBeFalsy();", + "expect(true).toBeFalsy();", + "expect(value).toEqual();", + "expect(value).not.toBeFalsy();", + "expect(value).not.toEqual();", + "expect(value).toBe(undefined);", + "expect(value).not.toBe(undefined);", + "expect(false).toBe(true)", + "expect(value).toBe();", + "expect(true).toMatchSnapshot();", + r#"expect("a string").toMatchSnapshot(false);"#, + r#"expect("a string").not.toMatchSnapshot();"#, + "expect(something).toEqual('a string');", + "expect(false).toBe", + "expectTypeOf(false).toBe", + ]; + + let fail = vec![ + "expect(true).toBe(false);", + "expect(wasSuccessful).toEqual(false);", + "expect(fs.existsSync('/path/to/file')).toStrictEqual(false);", + r#"expect("a string").not.toBe(false);"#, + r#"expect("a string").not.toEqual(false);"#, + r#"expectTypeOf("a string").not.toEqual(false);"#, + ]; + + let fix = vec![ + ("expect(true).toBe(false);", "expect(true).toBeFalsy();", None), + ("expect(wasSuccessful).toEqual(false);", "expect(wasSuccessful).toBeFalsy();", None), + ( + "expect(fs.existsSync('/path/to/file')).toStrictEqual(false);", + "expect(fs.existsSync('/path/to/file')).toBeFalsy();", + None, + ), + (r#"expect("a string").not.toBe(false);"#, r#"expect("a string").not.toBeFalsy();"#, None), + ( + r#"expect("a string").not.toEqual(false);"#, + r#"expect("a string").not.toBeFalsy();"#, + None, + ), + ( + r#"expectTypeOf("a string").not.toEqual(false);"#, + r#"expectTypeOf("a string").not.toBeFalsy();"#, + None, + ), + ]; + Tester::new(PreferToBeFalsy::NAME, pass, fail).expect_fix(fix).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/rules/vitest/prefer_to_be_truthy.rs b/crates/oxc_linter/src/rules/vitest/prefer_to_be_truthy.rs index c97dd29ef684b..9997e8a5d41c9 100644 --- a/crates/oxc_linter/src/rules/vitest/prefer_to_be_truthy.rs +++ b/crates/oxc_linter/src/rules/vitest/prefer_to_be_truthy.rs @@ -1,24 +1,11 @@ -use oxc_ast::{ - ast::{Argument, Expression}, - 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, is_equality_matcher, - parse_expect_and_typeof_vitest_fn_call, PossibleJestNode, - }, + utils::{collect_possible_jest_call_node, prefer_to_be_simply_bool}, }; -fn use_to_be_truthy(span0: Span) -> OxcDiagnostic { - OxcDiagnostic::warn("Use `toBeTruthy` instead.").with_label(span0) -} - #[derive(Debug, Default, Clone)] pub struct PreferToBeTruthy; @@ -46,55 +33,7 @@ declare_oxc_lint!( impl Rule for PreferToBeTruthy { fn run_once(&self, ctx: &LintContext) { for possible_vitest_node in &collect_possible_jest_call_node(ctx) { - Self::run(possible_vitest_node, ctx); - } - } -} - -impl PreferToBeTruthy { - fn run<'a>(possible_vitest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) { - let node = possible_vitest_node.node; - let AstKind::CallExpression(call_expr) = node.kind() else { - return; - }; - let Some(vitest_expect_fn_call) = - parse_expect_and_typeof_vitest_fn_call(call_expr, possible_vitest_node, ctx) - else { - return; - }; - let Some(matcher) = vitest_expect_fn_call.matcher() else { - return; - }; - - if !is_equality_matcher(matcher) || vitest_expect_fn_call.args.len() == 0 { - return; - } - - let Some(arg_expr) = vitest_expect_fn_call.args.first().and_then(Argument::as_expression) - else { - return; - }; - - if let Expression::BooleanLiteral(arg) = arg_expr.get_inner_expression() { - if arg.value { - let span = Span::new(matcher.span.start, call_expr.span.end); - - let is_cmp_mem_expr = match matcher.parent { - Some(Expression::ComputedMemberExpression(_)) => true, - Some( - Expression::StaticMemberExpression(_) - | Expression::PrivateFieldExpression(_), - ) => false, - _ => return, - }; - - ctx.diagnostic_with_fix(use_to_be_truthy(span), |fixer| { - let new_matcher = - if is_cmp_mem_expr { "[\"toBeTruthy\"]()" } else { "toBeTruthy()" }; - - fixer.replace(span, new_matcher) - }); - } + prefer_to_be_simply_bool(possible_vitest_node, ctx, true); } } } diff --git a/crates/oxc_linter/src/snapshots/prefer_to_be_falsy.snap b/crates/oxc_linter/src/snapshots/prefer_to_be_falsy.snap new file mode 100644 index 0000000000000..edfaf6e48cc8c --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_to_be_falsy.snap @@ -0,0 +1,44 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-vitest(prefer-to-be-falsy): Use `toBeFalsy` instead. + ╭─[prefer_to_be_falsy.tsx:1:14] + 1 │ expect(true).toBe(false); + · ─────────── + ╰──── + help: Replace `toBe(false)` with `toBeFalsy()`. + + ⚠ eslint-plugin-vitest(prefer-to-be-falsy): Use `toBeFalsy` instead. + ╭─[prefer_to_be_falsy.tsx:1:23] + 1 │ expect(wasSuccessful).toEqual(false); + · ────────────── + ╰──── + help: Replace `toEqual(false)` with `toBeFalsy()`. + + ⚠ eslint-plugin-vitest(prefer-to-be-falsy): Use `toBeFalsy` instead. + ╭─[prefer_to_be_falsy.tsx:1:40] + 1 │ expect(fs.existsSync('/path/to/file')).toStrictEqual(false); + · ──────────────────── + ╰──── + help: Replace `toStrictEqual(false)` with `toBeFalsy()`. + + ⚠ eslint-plugin-vitest(prefer-to-be-falsy): Use `toBeFalsy` instead. + ╭─[prefer_to_be_falsy.tsx:1:24] + 1 │ expect("a string").not.toBe(false); + · ─────────── + ╰──── + help: Replace `toBe(false)` with `toBeFalsy()`. + + ⚠ eslint-plugin-vitest(prefer-to-be-falsy): Use `toBeFalsy` instead. + ╭─[prefer_to_be_falsy.tsx:1:24] + 1 │ expect("a string").not.toEqual(false); + · ────────────── + ╰──── + help: Replace `toEqual(false)` with `toBeFalsy()`. + + ⚠ eslint-plugin-vitest(prefer-to-be-falsy): Use `toBeFalsy` instead. + ╭─[prefer_to_be_falsy.tsx:1:30] + 1 │ expectTypeOf("a string").not.toEqual(false); + · ────────────── + ╰──── + help: Replace `toEqual(false)` with `toBeFalsy()`. diff --git a/crates/oxc_linter/src/utils/vitest.rs b/crates/oxc_linter/src/utils/vitest.rs index 9ef6c4fd56695..a27255767ff53 100644 --- a/crates/oxc_linter/src/utils/vitest.rs +++ b/crates/oxc_linter/src/utils/vitest.rs @@ -1,7 +1,15 @@ use crate::LintContext; -use oxc_ast::ast::CallExpression; +use oxc_ast::{ + ast::{Argument, CallExpression, Expression}, + AstKind, +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_span::Span; -use super::{parse_jest_fn_call, ParsedExpectFnCall, ParsedJestFnCallNew, PossibleJestNode}; +use super::{ + is_equality_matcher, parse_jest_fn_call, ParsedExpectFnCall, ParsedJestFnCallNew, + PossibleJestNode, +}; pub fn parse_expect_and_typeof_vitest_fn_call<'a>( call_expr: &'a CallExpression<'a>, @@ -16,3 +24,57 @@ pub fn parse_expect_and_typeof_vitest_fn_call<'a>( ParsedJestFnCallNew::GeneralJest(_) => None, } } + +pub fn prefer_to_be_simply_bool<'a>( + possible_vitest_node: &PossibleJestNode<'a, '_>, + ctx: &LintContext<'a>, + value: bool, +) { + let node = possible_vitest_node.node; + let AstKind::CallExpression(call_expr) = node.kind() else { + return; + }; + let Some(vitest_expect_fn_call) = + parse_expect_and_typeof_vitest_fn_call(call_expr, possible_vitest_node, ctx) + else { + return; + }; + let Some(matcher) = vitest_expect_fn_call.matcher() else { + return; + }; + if !is_equality_matcher(matcher) || vitest_expect_fn_call.args.len() == 0 { + return; + } + let Some(arg_expr) = vitest_expect_fn_call.args.first().and_then(Argument::as_expression) + else { + return; + }; + + if let Expression::BooleanLiteral(arg) = arg_expr.get_inner_expression() { + if arg.value == value { + let span = Span::new(matcher.span.start, call_expr.span.end); + + let is_cmp_mem_expr = match matcher.parent { + Some(Expression::ComputedMemberExpression(_)) => true, + Some( + Expression::StaticMemberExpression(_) | Expression::PrivateFieldExpression(_), + ) => false, + _ => return, + }; + + let call_name = if value { "toBeTruthy" } else { "toBeFalsy" }; + + ctx.diagnostic_with_fix( + OxcDiagnostic::warn(format!("Use `{call_name}` instead.")).with_label(span), + |fixer| { + let new_matcher = if is_cmp_mem_expr { + format!("[\"{call_name}\"]()") + } else { + format!("{call_name}()") + }; + fixer.replace(span, new_matcher) + }, + ); + } + } +}