feat(lint/noDoneCallback): add rule (#1938)

CHANGELOG.md

@@ -39,6 +39,15 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 #### New features
 
+- Add rule [noDoneCallback](https://biomejs.dev/linter/rules/no-done-callback), this rule checks the function parameter of hooks & tests
+  for use of the done argument, suggesting you return a promise instead. Contributed by @vasucp1207
+
+  ```js
+  beforeEach(done => {
+    // ...
+  });
+  ```
+
 #### Enhamcements
 
 #### Bug fixes
@@ -93,7 +102,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
   Contributed by @Sec-ant
 
-## 1.6.0 (2024-03-08)
+## 1.6.0 (2024-04-08)
 
 ### Analyzer
 

crates/biome_diagnostics_categories/src/categories.rs

@@ -109,6 +109,7 @@ define_categories! {
     "lint/nursery/noApproximativeNumericConstant": "https://biomejs.dev/linter/rules/no-approximative-numeric-constant",
     "lint/nursery/noBarrelFile": "https://biomejs.dev/linter/rules/no-barrel-file",
     "lint/nursery/noConsole": "https://biomejs.dev/linter/rules/no-console",
+    "lint/nursery/noDoneCallback": "https://biomejs.dev/linter/rules/no-done-callback",
     "lint/nursery/noDuplicateJsonKeys": "https://biomejs.dev/linter/rules/no-duplicate-json-keys",
     "lint/nursery/noDuplicateTestHooks": "https://biomejs.dev/linter/rules/no-duplicate-test-hooks",
     "lint/nursery/noExcessiveNestedTestSuites": "https://biomejs.dev/linter/rules/no-excessive-nested-test-suites",

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

@@ -0,0 +1,155 @@
+use biome_analyze::{
+    context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic, RuleSource, RuleSourceKind,
+};
+use biome_console::markup;
+use biome_js_syntax::{
+    AnyJsArrowFunctionParameters, AnyJsBinding, AnyJsExpression, JsCallExpression, JsParameters,
+    JsTemplateExpression,
+};
+use biome_rowan::{AstNode, TextRange};
+
+declare_rule! {
+    /// Disallow using a callback in asynchronous tests and hooks.
+    ///
+    /// This rule checks the function parameter of hooks & tests for use of the done argument, suggesting you return a promise instead.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// beforeEach(done => {
+    ///     // ...
+    /// });
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// test('myFunction()', done => {
+    ///     // ...
+    /// });
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// beforeEach(async () => {
+    ///     // ...
+    /// });
+    /// ```
+    ///
+    /// ```js
+    /// test('myFunction()', () => {
+    ///     expect(myFunction()).toBeTruthy();
+    /// });
+    /// ```
+    ///
+    pub NoDoneCallback {
+        version: "next",
+        name: "noDoneCallback",
+        recommended: true,
+        source: RuleSource::EslintJest("no-done-callback"),
+        source_kind: RuleSourceKind::SameLogic,
+    }
+}
+
+impl Rule for NoDoneCallback {
+    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();
+
+        let callee = node.callee().ok()?;
+
+        let is_test_each = callee
+            .get_callee_member_name()
+            .map_or(false, |m| m.text_trimmed() == "each");
+
+        if is_test_each && !JsTemplateExpression::can_cast(callee.syntax().kind()) {
+            return None;
+        }
+
+        let arguments = &node.arguments().ok()?;
+        let callee_name = callee.get_callee_object_name()?;
+
+        let argument_index = match callee_name.text_trimmed() {
+            "after" | "afterAll" | "afterEach" | "before" | "beforeAll" | "beforeEach" => 0,
+            "it" | "test" => 1, // for test.each() and test() we want the second argument
+            _ => return None,
+        };
+        let argument = arguments
+            .get_arguments_by_index([argument_index])
+            .first()?
+            .clone();
+
+        let callback = argument?;
+        let callback = callback.as_any_js_expression()?;
+
+        match callback {
+            AnyJsExpression::JsArrowFunctionExpression(arrow_function) => {
+                let parameter = arrow_function.parameters().ok()?;
+                match parameter {
+                    AnyJsArrowFunctionParameters::AnyJsBinding(binding) => {
+                        let param = binding.as_js_identifier_binding()?;
+                        let text_range = param.name_token().ok()?;
+                        let text_range = text_range.text_trimmed_range();
+                        return Some(text_range);
+                    }
+                    AnyJsArrowFunctionParameters::JsParameters(js_parameters) => {
+                        return analyze_js_parameters(&js_parameters, is_test_each)
+                    }
+                }
+            }
+            AnyJsExpression::JsFunctionExpression(js_function) => {
+                let js_parameters = js_function.parameters().ok()?;
+                return analyze_js_parameters(&js_parameters, is_test_each);
+            }
+            _ => {}
+        }
+
+        None
+    }
+
+    fn diagnostic(_: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> {
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                range,
+                markup! {
+                    "Disallow using a callback in asynchronous tests and hooks."
+                },
+            )
+            .note(markup! {
+                "Return a Promise instead of relying on callback parameter."
+            }),
+        )
+    }
+}
+
+fn analyze_js_parameters(js_parameters: &JsParameters, is_test_each: bool) -> Option<TextRange> {
+    let items = js_parameters.items();
+
+    let param = match is_test_each {
+        true => items.into_iter().nth(1)?,
+        false => items.into_iter().next()?,
+    };
+
+    let param = param.ok()?;
+    let formal_parameter = param.as_any_js_formal_parameter()?;
+    let formal_parameter = formal_parameter.as_js_formal_parameter()?;
+
+    let binding = formal_parameter.binding().ok()?;
+    let binding = binding.as_any_js_binding()?;
+    let text_range = get_js_binding_range(binding)?;
+
+    Some(text_range)
+}
+
+fn get_js_binding_range(binding: &AnyJsBinding) -> Option<TextRange> {
+    let param = binding.as_js_identifier_binding()?;
+    let text_range = param.name_token().ok()?;
+    let text_range = text_range.text_trimmed_range();
+    Some(text_range)
+}

crates/biome_js_analyze/src/options.rs

@@ -51,6 +51,8 @@ pub type NoDefaultExport =
     <analyzers::style::no_default_export::NoDefaultExport as biome_analyze::Rule>::Options;
 pub type NoDelete = <analyzers::performance::no_delete::NoDelete as biome_analyze::Rule>::Options;
 pub type NoDistractingElements = < analyzers :: a11y :: no_distracting_elements :: NoDistractingElements as biome_analyze :: Rule > :: Options ;
+pub type NoDoneCallback =
+    <analyzers::nursery::no_done_callback::NoDoneCallback as biome_analyze::Rule>::Options;
 pub type NoDoubleEquals =
     <analyzers::suspicious::no_double_equals::NoDoubleEquals as biome_analyze::Rule>::Options;
 pub type NoDuplicateCase =

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

@@ -0,0 +1,62 @@
+test("something", (done) => {
+    done();
+});
+test("something", (done) => {
+    done();
+});
+test("something", (finished) => {
+    finished();
+});
+test("something", (done) => {
+    done();
+});
+test("something", (done) => done());
+test("something", (done) => done());
+test("something", function (done) {
+    done();
+});
+test("something", function (done) {
+    done();
+});
+test("something", async (done) => {
+    done();
+});
+test("something", async (done) => done());
+test("something", async function (done) {
+    done();
+});
+test("something", (done) => {
+    done();
+});
+beforeAll((done) => {
+    done();
+});
+beforeAll((finished) => {
+    finished();
+});
+beforeEach((done) => {
+    done();
+});
+afterAll((done) => done());
+afterEach((done) => done());
+beforeAll(function (done) {
+    done();
+});
+afterEach(function (done) {
+    done();
+});
+beforeAll(async (done) => {
+    done();
+});
+beforeAll(async (done) => done());
+beforeAll(async function (done) {
+    done();
+});
+beforeEach((done) => {
+    done();
+});
+beforeEach((done) => {
+    done();
+});
+it.each``("something", ({ a, b }, done) => {});
+test.each``("something", ({ a, b }, done) => {});