feat(linter/vitest): implement prefer-to-be-object (#5321)

Related to #4656

crates/oxc_linter/src/rules.rs

@@ -465,6 +465,7 @@ mod vitest {
     pub mod no_import_node_test;
     pub mod prefer_each;
     pub mod prefer_to_be_falsy;
+    pub mod prefer_to_be_object;
     pub mod prefer_to_be_truthy;
     pub mod require_local_test_context_for_concurrent_snapshots;
 }
@@ -887,6 +888,7 @@ oxc_macros::declare_all_lint_rules! {
     vitest::no_import_node_test,
     vitest::prefer_each,
     vitest::prefer_to_be_falsy,
+    vitest::prefer_to_be_object,
     vitest::prefer_to_be_truthy,
     vitest::no_conditional_tests,
     vitest::require_local_test_context_for_concurrent_snapshots,

crates/oxc_linter/src/rules/vitest/prefer_to_be_object.rs

@@ -0,0 +1,233 @@
+use oxc_ast::{
+    ast::{Argument, BinaryOperator, Expression},
+    AstKind,
+};
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::{GetSpan, Span};
+
+use crate::{
+    context::LintContext,
+    rule::Rule,
+    utils::{
+        collect_possible_jest_call_node, parse_jest_fn_call, KnownMemberExpressionProperty,
+        ParsedExpectFnCall, ParsedJestFnCallNew, PossibleJestNode,
+    },
+};
+
+fn prefer_to_be_object(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn("Prefer `toBeObject()` for object assertions")
+        .with_help("Consider using `toBeObject()` to test if a value is an object.")
+        .with_label(span)
+}
+
+fn is_parsed_instance_of_matcher_call(
+    parsed_expect_call: &ParsedExpectFnCall,
+    matcher: &KnownMemberExpressionProperty,
+) -> bool {
+    parsed_expect_call.args.len() == 1
+        && matches!(
+            parsed_expect_call.args.first(),
+            Some(Argument::Identifier(id)) if matcher.name().as_deref() == Some("toBeInstanceOf") && id.name == "Object"
+        )
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct PreferToBeObject;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// This rule enforces using `toBeObject()` to check if a value is of type `Object`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Using other methods such as `toBeInstanceOf(Object)` or `instanceof Object` can be less clear and potentially misleading. Enforcing the use of `toBeObject()` provides more explicit and readable code, making your intentions clear and improving the overall maintainability and readability of your tests.
+    ///
+    /// ### Examples
+    ///
+    /// Examples of **incorrect** code for this rule:
+    /// ```js
+    /// expectTypeOf({}).toBeInstanceOf(Object);
+    /// expectTypeOf({} instanceof Object).toBeTruthy();
+    /// ```
+    ///
+    /// Examples of **correct** code for this rule:
+    /// ```js
+    /// expectTypeOf({}).toBeObject();
+    /// expectTypeOf({}).toBeObject();
+    /// ```
+    PreferToBeObject,
+    style,
+    fix
+);
+
+impl Rule for PreferToBeObject {
+    fn run_once(&self, ctx: &LintContext) {
+        for possible_vitest_node in &collect_possible_jest_call_node(ctx) {
+            Self::run(possible_vitest_node, ctx);
+        }
+    }
+}
+
+impl PreferToBeObject {
+    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(ParsedJestFnCallNew::ExpectTypeOf(parsed_expect_call)) =
+            parse_jest_fn_call(call_expr, possible_vitest_node, ctx)
+        else {
+            return;
+        };
+
+        let Some(matcher) = parsed_expect_call.matcher() else {
+            return;
+        };
+
+        if is_parsed_instance_of_matcher_call(&parsed_expect_call, matcher) {
+            ctx.diagnostic_with_fix(prefer_to_be_object(matcher.span), |fixer| {
+                fixer.replace(Span::new(matcher.span.start, call_expr.span.end), "toBeObject()")
+            });
+            return;
+        }
+
+        if matches!(matcher.name().as_deref(), Some("toBeTruthy" | "toBeFalsy")) {
+            let Some(Expression::CallExpression(parent_call_expr)) = parsed_expect_call.head.parent
+            else {
+                return;
+            };
+
+            let Some(arg) = parent_call_expr.arguments.first() else {
+                return;
+            };
+
+            let expr = match &arg {
+                Argument::ParenthesizedExpression(paren_expr) => &paren_expr.expression,
+                _ => arg.to_expression(),
+            };
+
+            let Expression::BinaryExpression(binary_expr) = expr else {
+                return;
+            };
+
+            if binary_expr.operator != BinaryOperator::Instanceof {
+                return;
+            }
+
+            let Expression::Identifier(id) = &binary_expr.right else {
+                return;
+            };
+
+            if id.name == "Object" {
+                ctx.diagnostic_with_fix(prefer_to_be_object(matcher.span), |fixer| {
+                    let mut formatter = fixer.codegen();
+                    formatter.print_str(fixer.source_range(Span::new(
+                        call_expr.span.start,
+                        binary_expr.left.span().end,
+                    )));
+                    formatter.print_str(
+                        fixer.source_range(Span::new(
+                            binary_expr.span.end,
+                            parent_call_expr.span.end,
+                        )),
+                    );
+
+                    let not_modifier = parsed_expect_call
+                        .modifiers()
+                        .iter()
+                        .any(|node| node.name().as_deref() == Some("not"));
+
+                    if (matcher.name().as_deref() == Some("toBeFalsy")) != not_modifier {
+                        formatter.print_str(".not");
+                    }
+
+                    formatter.print_str(".toBeObject()");
+
+                    fixer.replace(call_expr.span, formatter)
+                });
+            }
+        }
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        "expectTypeOf.hasAssertions",
+        "expectTypeOf.hasAssertions()",
+        "expectTypeOf",
+        "expectTypeOf().not",
+        "expectTypeOf().toBe",
+        "expectTypeOf().toBe(true)",
+        "expectTypeOf({}).toBe(true)",
+        "expectTypeOf({}).toBeObject()",
+        "expectTypeOf({}).not.toBeObject()",
+        "expectTypeOf([] instanceof Array).not.toBeObject()",
+        "expectTypeOf({}).not.toBeInstanceOf(Array)",
+    ];
+
+    let fail = vec![
+        // "expectTypeOf(({} instanceof Object)).toBeTruthy();",
+        "expectTypeOf({} instanceof Object).toBeTruthy();",
+        "expectTypeOf({} instanceof Object).not.toBeTruthy();",
+        "expectTypeOf({} instanceof Object).toBeFalsy();",
+        "expectTypeOf({} instanceof Object).not.toBeFalsy();",
+        "expectTypeOf({}).toBeInstanceOf(Object);",
+        "expectTypeOf({}).not.toBeInstanceOf(Object);",
+        "expectTypeOf(requestValues()).resolves.toBeInstanceOf(Object);",
+        "expectTypeOf(queryApi()).resolves.not.toBeInstanceOf(Object);",
+    ];
+
+    let fix = vec![
+        (
+            "expectTypeOf(({} instanceof Object)).toBeTruthy();",
+            "expectTypeOf(({})).toBeObject();",
+            None,
+        ),
+        (
+            "expectTypeOf({} instanceof Object).toBeTruthy();",
+            "expectTypeOf({}).toBeObject();",
+            None,
+        ),
+        (
+            "expectTypeOf({} instanceof Object).not.toBeTruthy();",
+            "expectTypeOf({}).not.toBeObject();",
+            None,
+        ),
+        (
+            "expectTypeOf({} instanceof Object).toBeFalsy();",
+            "expectTypeOf({}).not.toBeObject();",
+            None,
+        ),
+        (
+            "expectTypeOf({} instanceof Object).not.toBeFalsy();",
+            "expectTypeOf({}).toBeObject();",
+            None,
+        ),
+        ("expectTypeOf({}).toBeInstanceOf(Object);", "expectTypeOf({}).toBeObject();", None),
+        (
+            "expectTypeOf({}).not.toBeInstanceOf(Object);",
+            "expectTypeOf({}).not.toBeObject();",
+            None,
+        ),
+        (
+            "expectTypeOf(requestValues()).resolves.toBeInstanceOf(Object);",
+            "expectTypeOf(requestValues()).resolves.toBeObject();",
+            None,
+        ),
+        (
+            "expectTypeOf(queryApi()).resolves.not.toBeInstanceOf(Object);",
+            "expectTypeOf(queryApi()).resolves.not.toBeObject();",
+            None,
+        ),
+    ];
+    Tester::new(PreferToBeObject::NAME, pass, fail)
+        .with_vitest_plugin(true)
+        .expect_fix(fix)
+        .test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/prefer_to_be_object.snap

@@ -0,0 +1,58 @@
+---
+source: crates/oxc_linter/src/tester.rs
+---
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:36]
+ 1 │ expectTypeOf({} instanceof Object).toBeTruthy();
+   ·                                    ──────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:40]
+ 1 │ expectTypeOf({} instanceof Object).not.toBeTruthy();
+   ·                                        ──────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:36]
+ 1 │ expectTypeOf({} instanceof Object).toBeFalsy();
+   ·                                    ─────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:40]
+ 1 │ expectTypeOf({} instanceof Object).not.toBeFalsy();
+   ·                                        ─────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:18]
+ 1 │ expectTypeOf({}).toBeInstanceOf(Object);
+   ·                  ──────────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:22]
+ 1 │ expectTypeOf({}).not.toBeInstanceOf(Object);
+   ·                      ──────────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:40]
+ 1 │ expectTypeOf(requestValues()).resolves.toBeInstanceOf(Object);
+   ·                                        ──────────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.
+
+  ⚠ eslint-plugin-vitest(prefer-to-be-object): Prefer `toBeObject()` for object assertions
+   ╭─[prefer_to_be_object.tsx:1:39]
+ 1 │ expectTypeOf(queryApi()).resolves.not.toBeInstanceOf(Object);
+   ·                                       ──────────────
+   ╰────
+  help: Consider using `toBeObject()` to test if a value is an object.

crates/oxc_linter/src/utils/jest/parse_jest_fn.rs

@@ -146,8 +146,10 @@ fn parse_jest_expect_fn_call<'a>(
         }
     }
 
+    let kind = if is_type_of { JestFnKind::ExpectTypeOf } else { JestFnKind::Expect };
+
     let parsed_expect_fn = ParsedExpectFnCall {
-        kind: JestFnKind::Expect,
+        kind,
         head,
         members,
         name: Cow::Borrowed(name),
@@ -330,6 +332,7 @@ fn resolve_first_ident<'a>(expr: &'a Expression<'a>) -> Option<&'a IdentifierRef
     }
 }
 
+#[derive(Debug)]
 pub enum ParsedJestFnCall<'a> {
     GeneralJest(ParsedGeneralJestFnCall<'a>),
     Expect(ParsedExpectFnCall<'a>),