Skip to content

Commit

Permalink
feat(linter/eslint-plugin-vitest): implement prefer-to-be-truthy (#4755)
Browse files Browse the repository at this point in the history
Related to #4656
  • Loading branch information
shulaoda authored Aug 8, 2024
1 parent b20e335 commit 41f861f
Show file tree
Hide file tree
Showing 13 changed files with 264 additions and 19 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ mod promise {

mod vitest {
pub mod no_import_node_test;
pub mod prefer_to_be_truthy;
}

oxc_macros::declare_all_lint_rules! {
Expand Down Expand Up @@ -854,4 +855,5 @@ oxc_macros::declare_all_lint_rules! {
promise::no_new_statics,
promise::param_names,
vitest::no_import_node_test,
vitest::prefer_to_be_truthy,
}
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/consistent_test_it.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl ConsistentTestIt {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};
let Some(ParsedJestFnCallNew::GeneralJestFnCall(jest_fn_call)) =
let Some(ParsedJestFnCallNew::GeneralJest(jest_fn_call)) =
parse_jest_fn_call(call_expr, possible_jest_node, ctx)
else {
return;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/no_disabled_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>)
let ParsedGeneralJestFnCall { kind, members, name, .. } = jest_fn_call;
// `test('foo')`
let kind = match kind {
JestFnKind::Expect | JestFnKind::Unknown => return,
JestFnKind::Expect | JestFnKind::ExpectTypeOf | JestFnKind::Unknown => return,
JestFnKind::General(kind) => kind,
};
if matches!(kind, JestGeneralFnKind::Test)
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/no_duplicate_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl NoDuplicateHooks {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};
let Some(ParsedJestFnCallNew::GeneralJestFnCall(jest_fn_call)) =
let Some(ParsedJestFnCallNew::GeneralJest(jest_fn_call)) =
parse_jest_fn_call(call_expr, possible_jest_node, ctx)
else {
return;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/prefer_hooks_in_order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl PreferHooksInOrder {
call_expr: &'a CallExpression<'_>,
ctx: &LintContext<'a>,
) {
let Some(ParsedJestFnCallNew::GeneralJestFnCall(jest_fn_call)) =
let Some(ParsedJestFnCallNew::GeneralJest(jest_fn_call)) =
parse_jest_fn_call(call_expr, possible_jest_node, ctx)
else {
*previous_hook_index = -1;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/prefer_lowercase_title.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl PreferLowercaseTitle {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};
let Some(ParsedJestFnCallNew::GeneralJestFnCall(jest_fn_call)) =
let Some(ParsedJestFnCallNew::GeneralJest(jest_fn_call)) =
parse_jest_fn_call(call_expr, possible_jest_node, ctx)
else {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl RequireTopLevelDescribe {
return;
};

let Some(ParsedJestFnCallNew::GeneralJestFnCall(ParsedGeneralJestFnCall { kind, .. })) =
let Some(ParsedJestFnCallNew::GeneralJest(ParsedGeneralJestFnCall { kind, .. })) =
parse_jest_fn_call(call_expr, possible_jest_node, ctx)
else {
return;
Expand Down
160 changes: 160 additions & 0 deletions crates/oxc_linter/src/rules/vitest/prefer_to_be_truthy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
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,
},
};

fn use_to_be_truthy(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Use `toBeTruthy` instead.").with_label(span0)
}

#[derive(Debug, Default, Clone)]
pub struct PreferToBeTruthy;

declare_oxc_lint!(
/// ### What it does
///
/// This rule warns when `toBe(true)` is used with `expect` or `expectTypeOf`. With `--fix`, it will be replaced with `toBeTruthy()`.
///
/// ### Examples
///
/// ```javascript
/// // bad
/// expect(foo).toBe(true)
/// expectTypeOf(foo).toBe(true)
///
/// // good
/// expect(foo).toBeTruthy()
/// expectTypeOf(foo).toBeTruthy()
/// ```
PreferToBeTruthy,
style,
fix
);

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)
});
}
}
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
"[].push(true)",
r#"expect("something");"#,
"expect(true).toBeTrue();",
"expect(false).toBeTrue();",
"expect(fal,se).toBeFalse();",
"expect(true).toBeFalse();",
"expect(value).toEqual();",
"expect(value).not.toBeTrue();",
"expect(value).not.toEqual();",
"expect(value).toBe(undefined);",
"expect(value).not.toBe(undefined);",
"expect(true).toBe(false)",
"expect(value).toBe();",
"expect(true).toMatchSnapshot();",
r#"expect("a string").toMatchSnapshot(true);"#,
r#"expect("a string").not.toMatchSnapshot();"#,
"expect(something).toEqual('a string');",
"expect(true).toBe",
"expectTypeOf(true).toBe()",
];

let fail = vec![
"expect(false).toBe(true);",
"expectTypeOf(false).toBe(true);",
"expect(wasSuccessful).toEqual(true);",
"expect(fs.existsSync('/path/to/file')).toStrictEqual(true);",
r#"expect("a string").not.toBe(true);"#,
r#"expect("a string").not.toEqual(true);"#,
r#"expectTypeOf("a string").not.toStrictEqual(true);"#,
];

let fix = vec![
("expect(false).toBe(true);", "expect(false).toBeTruthy();", None),
("expectTypeOf(false).toBe(true);", "expectTypeOf(false).toBeTruthy();", None),
("expect(wasSuccessful).toEqual(true);", "expect(wasSuccessful).toBeTruthy();", None),
(
"expect(fs.existsSync('/path/to/file')).toStrictEqual(true);",
"expect(fs.existsSync('/path/to/file')).toBeTruthy();",
None,
),
(r#"expect("a string").not.toBe(true);"#, r#"expect("a string").not.toBeTruthy();"#, None),
(
r#"expect("a string").not.toEqual(true);"#,
r#"expect("a string").not.toBeTruthy();"#,
None,
),
(
r#"expectTypeOf("a string").not.toStrictEqual(true);"#,
r#"expectTypeOf("a string").not.toBeTruthy();"#,
None,
),
];
Tester::new(PreferToBeTruthy::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
51 changes: 51 additions & 0 deletions crates/oxc_linter/src/snapshots/prefer_to_be_truthy.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
source: crates/oxc_linter/src/tester.rs
---
eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:15]
1expect(false).toBe(true);
· ──────────
╰────
help: Replace `toBe(true)` with `toBeTruthy()`.

eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:21]
1expectTypeOf(false).toBe(true);
· ──────────
╰────
help: Replace `toBe(true)` with `toBeTruthy()`.

eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:23]
1expect(wasSuccessful).toEqual(true);
· ─────────────
╰────
help: Replace `toEqual(true)` with `toBeTruthy()`.

eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:40]
1expect(fs.existsSync('/path/to/file')).toStrictEqual(true);
· ───────────────────
╰────
help: Replace `toStrictEqual(true)` with `toBeTruthy()`.

eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:24]
1expect("a string").not.toBe(true);
· ──────────
╰────
help: Replace `toBe(true)` with `toBeTruthy()`.

eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:24]
1expect("a string").not.toEqual(true);
· ─────────────
╰────
help: Replace `toEqual(true)` with `toBeTruthy()`.

eslint-plugin-vitest(prefer-to-be-truthy): Use `toBeTruthy` instead.
╭─[prefer_to_be_truthy.tsx:1:30]
1expectTypeOf("a string").not.toStrictEqual(true);
· ───────────────────
╰────
help: Replace `toStrictEqual(true)` with `toBeTruthy()`.
7 changes: 5 additions & 2 deletions crates/oxc_linter/src/utils/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub const JEST_METHOD_NAMES: phf::Set<&'static str> = phf_set![
"beforeEach",
"describe",
"expect",
"expectTypeOf",
"fdescribe",
"fit",
"it",
Expand All @@ -40,6 +41,7 @@ pub const JEST_METHOD_NAMES: phf::Set<&'static str> = phf_set![
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum JestFnKind {
Expect,
ExpectTypeOf,
General(JestGeneralFnKind),
Unknown,
}
Expand All @@ -48,6 +50,7 @@ impl JestFnKind {
pub fn from(name: &str) -> Self {
match name {
"expect" => Self::Expect,
"expectTypeOf" => Self::ExpectTypeOf,
"jest" => Self::General(JestGeneralFnKind::Jest),
"describe" | "fdescribe" | "xdescribe" => Self::General(JestGeneralFnKind::Describe),
"fit" | "it" | "test" | "xit" | "xtest" => Self::General(JestGeneralFnKind::Test),
Expand Down Expand Up @@ -113,7 +116,7 @@ pub fn parse_general_jest_fn_call<'a>(
) -> Option<ParsedGeneralJestFnCall<'a>> {
let jest_fn_call = parse_jest_fn_call(call_expr, possible_jest_node, ctx)?;

if let ParsedJestFnCallNew::GeneralJestFnCall(jest_fn_call) = jest_fn_call {
if let ParsedJestFnCallNew::GeneralJest(jest_fn_call) = jest_fn_call {
return Some(jest_fn_call);
}
None
Expand All @@ -126,7 +129,7 @@ pub fn parse_expect_jest_fn_call<'a>(
) -> Option<ParsedExpectFnCall<'a>> {
let jest_fn_call = parse_jest_fn_call(call_expr, possible_jest_node, ctx)?;

if let ParsedJestFnCallNew::ExpectFnCall(jest_fn_call) = jest_fn_call {
if let ParsedJestFnCallNew::Expect(jest_fn_call) = jest_fn_call {
return Some(jest_fn_call);
}
None
Expand Down
Loading

0 comments on commit 41f861f

Please sign in to comment.