feat(linter): add rule noSkippedTests (#1635)

CHANGELOG.md

@@ -34,6 +34,25 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
 
 ### Linter
 
+#### New features
+
+- Add the rule [noSkippedTests](https://biomejs.dev/linter/rules/no-skipped-tests), to disallow skipped tests:
+
+  ```js
+  describe.skip("test", () => {});
+  it.skip("test", () => {});
+  ```
+<<<<<<< HEAD
+=======
+
+- Add the rule [noFocusedTests](https://biomejs.dev/linter/rules/no-focused-tests), to disallow skipped tests:
+
+  ```js
+  describe.only("test", () => {});
+  it.only("test", () => {});
+  ```
+
+>>>>>>> fd3de977d1 (feat(linter): new rule noFocusedTests (#1641))
 ### Parser
 
 ## 1.5.3 (2024-01-22)

crates/biome_diagnostics_categories/src/categories.rs

@@ -105,11 +105,13 @@ define_categories! {
     "lint/nursery/noDuplicateJsonKeys": "https://biomejs.dev/linter/rules/no-duplicate-json-keys",
     "lint/nursery/noEmptyBlockStatements": "https://biomejs.dev/linter/rules/no-empty-block-statements",
     "lint/nursery/noEmptyTypeParameters": "https://biomejs.dev/linter/rules/no-empty-type-parameters",
+    "lint/nursery/noFocusedTests": "https://biomejs.dev/linter/rules/no-focused-tests",
     "lint/nursery/noGlobalAssign": "https://biomejs.dev/linter/rules/no-global-assign",
     "lint/nursery/noGlobalEval": "https://biomejs.dev/linter/rules/no-global-eval",
     "lint/nursery/noInvalidUseBeforeDeclaration": "https://biomejs.dev/linter/rules/no-invalid-use-before-declaration",
     "lint/nursery/noMisleadingCharacterClass": "https://biomejs.dev/linter/rules/no-misleading-character-class",
     "lint/nursery/noNodejsModules": "https://biomejs.dev/linter/rules/no-nodejs-modules",
+    "lint/nursery/noSkippedTests": "https://biomejs.dev/linter/rules/no-skipped-tests",
     "lint/nursery/noThenProperty": "https://biomejs.dev/linter/rules/no-then-property",
     "lint/nursery/noTypeOnlyImportAttributes": "https://biomejs.dev/linter/rules/no-type-only-import-attributes",
     "lint/nursery/noUnusedImports": "https://biomejs.dev/linter/rules/no-unused-imports",

crates/biome_js_analyze/src/analyzers/nursery/no_focused_tests.rs

@@ -0,0 +1,82 @@
+use biome_analyze::{
+    context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic, RuleSource, RuleSourceKind,
+};
+use biome_console::markup;
+use biome_js_syntax::{AnyJsExpression, JsCallExpression, JsSyntaxToken, TextRange};
+
+declare_rule! {
+    /// Disallow focused tests.
+    ///
+    /// Disabled test are useful when developing and debugging, because it forces the test suite to run only certain tests.
+    ///
+    /// However, in pull/merge request, you usually want to run all the test suite.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// describe.only("foo", () => {});
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// test.only("foo", () => {});
+    /// ```
+    pub(crate) NoFocusedTests {
+        version: "next",
+        name: "noFocusedTests",
+        recommended: true,
+        source: RuleSource::EslintJest("no-focused-tests"),
+        source_kind: RuleSourceKind::Inspired,
+    }
+}
+
+impl Rule for NoFocusedTests {
+    type Query = Ast<JsCallExpression>;
+    type State = TextRange;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let node = ctx.query();
+
+        if node.is_test_call_expression().ok()? {
+            let callee = node.callee().ok()?;
+            if callee.contains_a_test_pattern().ok()? {
+                let function_name = get_function_name(&callee)?;
+
+                if function_name.text_trimmed() == "only" {
+                    return Some(function_name.text_trimmed_range());
+                }
+            }
+        }
+
+        None
+    }
+
+    fn diagnostic(_: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> {
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                range,
+                markup! {
+                    "Don't focus the test."
+                },
+            )
+                .note("This is likely a change done during debugging or implementation phases, but it's unlikely what you want in production.")
+                .note("Remove it.")
+        )
+    }
+}
+
+fn get_function_name(callee: &AnyJsExpression) -> Option<JsSyntaxToken> {
+    match callee {
+        AnyJsExpression::JsStaticMemberExpression(node) => {
+            let member = node.member().ok()?;
+            let member = member.as_js_name()?;
+            member.value_token().ok()
+        }
+        AnyJsExpression::JsIdentifierExpression(node) => node.name().ok()?.value_token().ok(),
+        _ => None,
+    }
+}

crates/biome_js_analyze/src/analyzers/nursery/no_skipped_tests.rs

@@ -0,0 +1,91 @@
+use biome_analyze::{
+    context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic, RuleSource, RuleSourceKind,
+};
+use biome_console::markup;
+use biome_js_syntax::{AnyJsExpression, JsCallExpression, JsSyntaxToken};
+use biome_rowan::TextRange;
+
+declare_rule! {
+    /// Disallow disabled tests.
+    ///
+    /// Disabled test are useful when developing and debugging, although they should not be committed in production.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// describe.skip("test", () => {});
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// test.skip("test", () => {});
+    /// ```
+    ///
+    /// ## Valid
+    ///
+    /// ```js
+    /// test.only("test", () => {});
+    /// test("test", () => {});
+    /// ```
+    ///
+    pub(crate) NoSkippedTests {
+        version: "next",
+        name: "noSkippedTests",
+        recommended: false,
+        source: RuleSource::EslintJest("no-disabled-tests"),
+        source_kind: RuleSourceKind::Inspired,
+    }
+}
+
+const FUNCTION_NAMES: [&str; 4] = ["skip", "xdescribe", "xit", "xtest"];
+
+impl Rule for NoSkippedTests {
+    type Query = Ast<JsCallExpression>;
+    type State = TextRange;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let node = ctx.query();
+
+        if node.is_test_call_expression().ok()? {
+            let callee = node.callee().ok()?;
+            if callee.contains_a_test_pattern().ok()? {
+                let function_name = get_function_name(&callee)?;
+
+                if FUNCTION_NAMES.contains(&function_name.text_trimmed()) {
+                    return Some(function_name.text_trimmed_range());
+                }
+            }
+        }
+
+        None
+    }
+
+    fn diagnostic(_: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> {
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                range,
+                markup! {
+                    "Don't disable tests."
+                },
+            )
+            .note("Disabling tests is useful when debugging or creating placeholder while working.")
+            .note("If this is intentional, and you want to commit a disabled test, add a suppression comment.")
+        )
+    }
+}
+
+fn get_function_name(callee: &AnyJsExpression) -> Option<JsSyntaxToken> {
+    match callee {
+        AnyJsExpression::JsStaticMemberExpression(node) => {
+            let member = node.member().ok()?;
+            let member = member.as_js_name()?;
+            member.value_token().ok()
+        }
+        AnyJsExpression::JsIdentifierExpression(node) => node.name().ok()?.value_token().ok(),
+        _ => None,
+    }
+}

crates/biome_js_analyze/src/lib.rs

@@ -243,17 +243,7 @@ mod tests {
             String::from_utf8(buffer).unwrap()
         }
 
-        const SOURCE: &str = r#"
-        import { useEffect } from "react";
-        function MyComponent7() {
-    let someObj = getObj();
-    useEffect(() => {
-        console.log(
-					someObj
-							.name
-				);
-    }, [someObj]);
-}
+        const SOURCE: &str = r#"xdescribe('foo', () => {});
         "#;
         // const SOURCE: &str = r#"document.querySelector("foo").value = document.querySelector("foo").value
         //
@@ -268,7 +258,7 @@ mod tests {
             closure_index: Some(0),
             dependencies_index: Some(1),
         };
-        let rule_filter = RuleFilter::Rule("correctness", "useExhaustiveDependencies");
+        let rule_filter = RuleFilter::Rule("nursery", "noDisabledTests");
         options.configuration.rules.push_rule(
             RuleKey::new("nursery", "useHookAtTopLevel"),
             RuleOptions::new(HooksOptions { hooks: vec![hook] }),

crates/biome_js_analyze/tests/specs/nursery/noFocusedTests/invalid.js

@@ -0,0 +1,5 @@
+describe.only("test", () => {});
+it.only("test", () => {});
+test.only("test", () => {});
+fdescribe('foo', () => {});
+fit('foo', () => {});

crates/biome_js_analyze/tests/specs/nursery/noFocusedTests/invalid.js.snap

@@ -0,0 +1,70 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: invalid.js
+---
+# Input
+```jsx
+describe.only("test", () => {});
+it.only("test", () => {});
+test.only("test", () => {});
+fdescribe('foo', () => {});
+fit('foo', () => {});
+
+```
+
+# Diagnostics
+```
+invalid.js:1:10 lint/nursery/noFocusedTests ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Don't focus the test.
+  
+  > 1 │ describe.only("test", () => {});
+      │          ^^^^
+    2 │ it.only("test", () => {});
+    3 │ test.only("test", () => {});
+  
+  i This is likely a change done during debugging or implementation phases, but it's unlikely what you want in production.
+  
+  i Remove it.
+  
+
+```
+
+```
+invalid.js:2:4 lint/nursery/noFocusedTests ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Don't focus the test.
+  
+    1 │ describe.only("test", () => {});
+  > 2 │ it.only("test", () => {});
+      │    ^^^^
+    3 │ test.only("test", () => {});
+    4 │ fdescribe('foo', () => {});
+  
+  i This is likely a change done during debugging or implementation phases, but it's unlikely what you want in production.
+  
+  i Remove it.
+  
+
+```
+
+```
+invalid.js:3:6 lint/nursery/noFocusedTests ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Don't focus the test.
+  
+    1 │ describe.only("test", () => {});
+    2 │ it.only("test", () => {});
+  > 3 │ test.only("test", () => {});
+      │      ^^^^
+    4 │ fdescribe('foo', () => {});
+    5 │ fit('foo', () => {});
+  
+  i This is likely a change done during debugging or implementation phases, but it's unlikely what you want in production.
+  
+  i Remove it.
+  
+
+```
+
+

crates/biome_js_analyze/tests/specs/nursery/noFocusedTests/valid.js

@@ -0,0 +1,5 @@
+describe.skip("test", () => {});
+it.skip("test", () => {});
+test.skip("test", () => {});
+fdescribe('foo', () => {});
+fit('foo', () => {});

crates/biome_js_analyze/tests/specs/nursery/noFocusedTests/valid.js.snap

@@ -0,0 +1,15 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: valid.js
+---
+# Input
+```jsx
+describe.skip("test", () => {});
+it.skip("test", () => {});
+test.skip("test", () => {});
+fdescribe('foo', () => {});
+fit('foo', () => {});
+
+```
+
+

crates/biome_js_analyze/tests/specs/nursery/noSkippedTests/invalid.js

@@ -0,0 +1,6 @@
+describe.skip("test", () => {});
+it.skip("test", () => {});
+test.skip("test", () => {});
+xdescribe('foo', () => {});
+xit('foo', () => {});
+xtest('foo', () => {});