diff --git a/CHANGELOG.md b/CHANGELOG.md index 129c17804810..9323d33d962f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs index 25ee9b2ac3f4..14af7d8f1f1b 100644 --- a/crates/biome_diagnostics_categories/src/categories.rs +++ b/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", diff --git a/crates/biome_js_analyze/src/analyzers/nursery.rs b/crates/biome_js_analyze/src/analyzers/nursery.rs index 0847fb8b1219..fe46c2f0bae3 100644 --- a/crates/biome_js_analyze/src/analyzers/nursery.rs +++ b/crates/biome_js_analyze/src/analyzers/nursery.rs @@ -3,6 +3,7 @@ use biome_analyze::declare_group; pub mod no_barrel_file; +pub mod no_done_callback; pub mod no_duplicate_test_hooks; pub mod no_excessive_nested_test_suites; pub mod no_exports_in_test; @@ -21,6 +22,7 @@ declare_group! { name : "nursery" , rules : [ self :: no_barrel_file :: NoBarrelFile , + self :: no_done_callback :: NoDoneCallback , self :: no_duplicate_test_hooks :: NoDuplicateTestHooks , self :: no_excessive_nested_test_suites :: NoExcessiveNestedTestSuites , self :: no_exports_in_test :: NoExportsInTest , diff --git a/crates/biome_js_analyze/src/analyzers/nursery/no_done_callback.rs b/crates/biome_js_analyze/src/analyzers/nursery/no_done_callback.rs new file mode 100644 index 000000000000..d1bb59a641a5 --- /dev/null +++ b/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; + type State = TextRange; + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> 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, range: &Self::State) -> Option { + 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 { + 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 { + 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) +} diff --git a/crates/biome_js_analyze/src/options.rs b/crates/biome_js_analyze/src/options.rs index 1f50a01f1fda..fca144678a05 100644 --- a/crates/biome_js_analyze/src/options.rs +++ b/crates/biome_js_analyze/src/options.rs @@ -51,6 +51,8 @@ pub type NoDefaultExport = ::Options; pub type NoDelete = ::Options; pub type NoDistractingElements = < analyzers :: a11y :: no_distracting_elements :: NoDistractingElements as biome_analyze :: Rule > :: Options ; +pub type NoDoneCallback = + ::Options; pub type NoDoubleEquals = ::Options; pub type NoDuplicateCase = diff --git a/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/invalid.js b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/invalid.js new file mode 100644 index 000000000000..0572327d7ee0 --- /dev/null +++ b/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) => {}); diff --git a/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/invalid.js.snap new file mode 100644 index 000000000000..9ef3f723030c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/invalid.js.snap @@ -0,0 +1,510 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```jsx +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) => {}); + +``` + +# Diagnostics +``` +invalid.js:1:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + > 1 │ test("something", (done) => { + │ ^^^^ + 2 │ done(); + 3 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:4:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 2 │ done(); + 3 │ }); + > 4 │ test("something", (done) => { + │ ^^^^ + 5 │ done(); + 6 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:7:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 5 │ done(); + 6 │ }); + > 7 │ test("something", (finished) => { + │ ^^^^^^^^ + 8 │ finished(); + 9 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:10:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 8 │ finished(); + 9 │ }); + > 10 │ test("something", (done) => { + │ ^^^^ + 11 │ done(); + 12 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:13:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 11 │ done(); + 12 │ }); + > 13 │ test("something", (done) => done()); + │ ^^^^ + 14 │ test("something", (done) => done()); + 15 │ test("something", function (done) { + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:14:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 12 │ }); + 13 │ test("something", (done) => done()); + > 14 │ test("something", (done) => done()); + │ ^^^^ + 15 │ test("something", function (done) { + 16 │ done(); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:15:29 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 13 │ test("something", (done) => done()); + 14 │ test("something", (done) => done()); + > 15 │ test("something", function (done) { + │ ^^^^ + 16 │ done(); + 17 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:18:29 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 16 │ done(); + 17 │ }); + > 18 │ test("something", function (done) { + │ ^^^^ + 19 │ done(); + 20 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:21:26 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 19 │ done(); + 20 │ }); + > 21 │ test("something", async (done) => { + │ ^^^^ + 22 │ done(); + 23 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:24:26 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 22 │ done(); + 23 │ }); + > 24 │ test("something", async (done) => done()); + │ ^^^^ + 25 │ test("something", async function (done) { + 26 │ done(); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:25:35 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 23 │ }); + 24 │ test("something", async (done) => done()); + > 25 │ test("something", async function (done) { + │ ^^^^ + 26 │ done(); + 27 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:28:20 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 26 │ done(); + 27 │ }); + > 28 │ test("something", (done) => { + │ ^^^^ + 29 │ done(); + 30 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:31:12 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 29 │ done(); + 30 │ }); + > 31 │ beforeAll((done) => { + │ ^^^^ + 32 │ done(); + 33 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:34:12 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 32 │ done(); + 33 │ }); + > 34 │ beforeAll((finished) => { + │ ^^^^^^^^ + 35 │ finished(); + 36 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:37:13 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 35 │ finished(); + 36 │ }); + > 37 │ beforeEach((done) => { + │ ^^^^ + 38 │ done(); + 39 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:40:11 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 38 │ done(); + 39 │ }); + > 40 │ afterAll((done) => done()); + │ ^^^^ + 41 │ afterEach((done) => done()); + 42 │ beforeAll(function (done) { + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:41:12 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 39 │ }); + 40 │ afterAll((done) => done()); + > 41 │ afterEach((done) => done()); + │ ^^^^ + 42 │ beforeAll(function (done) { + 43 │ done(); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:42:21 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 40 │ afterAll((done) => done()); + 41 │ afterEach((done) => done()); + > 42 │ beforeAll(function (done) { + │ ^^^^ + 43 │ done(); + 44 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:45:21 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 43 │ done(); + 44 │ }); + > 45 │ afterEach(function (done) { + │ ^^^^ + 46 │ done(); + 47 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:48:18 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 46 │ done(); + 47 │ }); + > 48 │ beforeAll(async (done) => { + │ ^^^^ + 49 │ done(); + 50 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:51:18 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 49 │ done(); + 50 │ }); + > 51 │ beforeAll(async (done) => done()); + │ ^^^^ + 52 │ beforeAll(async function (done) { + 53 │ done(); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:52:27 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 50 │ }); + 51 │ beforeAll(async (done) => done()); + > 52 │ beforeAll(async function (done) { + │ ^^^^ + 53 │ done(); + 54 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:55:13 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 53 │ done(); + 54 │ }); + > 55 │ beforeEach((done) => { + │ ^^^^ + 56 │ done(); + 57 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:58:13 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 56 │ done(); + 57 │ }); + > 58 │ beforeEach((done) => { + │ ^^^^ + 59 │ done(); + 60 │ }); + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:61:35 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 59 │ done(); + 60 │ }); + > 61 │ it.each``("something", ({ a, b }, done) => {}); + │ ^^^^ + 62 │ test.each``("something", ({ a, b }, done) => {}); + 63 │ + + i Return a Promise instead of relying on callback parameter. + + +``` + +``` +invalid.js:62:37 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Disallow using a callback in asynchronous tests and hooks. + + 60 │ }); + 61 │ it.each``("something", ({ a, b }, done) => {}); + > 62 │ test.each``("something", ({ a, b }, done) => {}); + │ ^^^^ + 63 │ + + i Return a Promise instead of relying on callback parameter. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/valid.js b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/valid.js new file mode 100644 index 000000000000..6cb193a9f1f7 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/valid.js @@ -0,0 +1,23 @@ +test("something", () => {}); +test("something", async () => {}); +test("something", function () {}); +test.each``("something", ({ a, b }) => {}); +test.each()("something", ({ a, b }) => {}); +it.each()("something", ({ a, b }) => {}); +it.each([])("something", (a, b) => {}); +it.each``("something", ({ a, b }) => {}); +it.each([])("something", (a, b) => { + a(); + b(); +}); +it.each``("something", ({ a, b }) => { + a(); + b(); +}); +test("something", async function () {}); +test("something", someArg); +beforeEach(() => {}); +beforeAll(async () => {}); +afterAll(() => {}); +afterAll(async function () {}); +afterAll(async function () {}, 5); diff --git a/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/valid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/valid.js.snap new file mode 100644 index 000000000000..0fb16ea4bd8c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noDoneCallback/valid.js.snap @@ -0,0 +1,31 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```jsx +test("something", () => {}); +test("something", async () => {}); +test("something", function () {}); +test.each``("something", ({ a, b }) => {}); +test.each()("something", ({ a, b }) => {}); +it.each()("something", ({ a, b }) => {}); +it.each([])("something", (a, b) => {}); +it.each``("something", ({ a, b }) => {}); +it.each([])("something", (a, b) => { + a(); + b(); +}); +it.each``("something", ({ a, b }) => { + a(); + b(); +}); +test("something", async function () {}); +test("something", someArg); +beforeEach(() => {}); +beforeAll(async () => {}); +afterAll(() => {}); +afterAll(async function () {}); +afterAll(async function () {}, 5); + +``` diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index 0decd3e03419..a0eebac2560c 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -966,6 +966,13 @@ impl AnyJsExpression { let member = member.as_js_identifier_expression()?.name().ok()?; member.value_token().ok() } + AnyJsExpression::JsTemplateExpression(node) => { + let tag = node.tag()?; + let tag = tag.as_js_static_member_expression()?; + let member = tag.object().ok()?; + let member = member.as_js_identifier_expression()?.name().ok()?; + member.value_token().ok() + } AnyJsExpression::JsIdentifierExpression(node) => node.name().ok()?.value_token().ok(), _ => None, } @@ -978,6 +985,13 @@ impl AnyJsExpression { let member = member.as_js_name()?; member.value_token().ok() } + AnyJsExpression::JsTemplateExpression(node) => { + let tag = node.tag()?; + let tag = tag.as_js_static_member_expression()?; + let member = tag.member().ok()?; + let member = member.as_js_name()?; + member.value_token().ok() + } AnyJsExpression::JsIdentifierExpression(node) => node.name().ok()?.value_token().ok(), _ => None, } diff --git a/crates/biome_service/src/configuration/linter/rules.rs b/crates/biome_service/src/configuration/linter/rules.rs index face9a53dea7..ab38f8ebe3ea 100644 --- a/crates/biome_service/src/configuration/linter/rules.rs +++ b/crates/biome_service/src/configuration/linter/rules.rs @@ -2601,6 +2601,9 @@ pub struct Nursery { #[doc = "Disallow the use of console."] #[serde(skip_serializing_if = "Option::is_none")] pub no_console: Option>, + #[doc = "Disallow using a callback in asynchronous tests and hooks."] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_done_callback: Option>, #[doc = "Disallow two keys with the same name inside a JSON object."] #[serde(skip_serializing_if = "Option::is_none")] pub no_duplicate_json_keys: Option>, @@ -2669,9 +2672,10 @@ impl DeserializableValidator for Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 19] = [ + pub(crate) const GROUP_RULES: [&'static str; 20] = [ "noBarrelFile", "noConsole", + "noDoneCallback", "noDuplicateJsonKeys", "noDuplicateTestHooks", "noExcessiveNestedTestSuites", @@ -2690,7 +2694,8 @@ impl Nursery { "useNodeAssertStrict", "useSortedClasses", ]; - const RECOMMENDED_RULES: [&'static str; 7] = [ + const RECOMMENDED_RULES: [&'static str; 8] = [ + "noDoneCallback", "noDuplicateJsonKeys", "noDuplicateTestHooks", "noExcessiveNestedTestSuites", @@ -2699,16 +2704,17 @@ impl Nursery { "noSemicolonInJsx", "noUselessTernary", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 7] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 8] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[3]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[4]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 19] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 20] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), @@ -2728,6 +2734,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { @@ -2754,91 +2761,96 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1])); } } - if let Some(rule) = self.no_duplicate_json_keys.as_ref() { + if let Some(rule) = self.no_done_callback.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2])); } } - if let Some(rule) = self.no_duplicate_test_hooks.as_ref() { + if let Some(rule) = self.no_duplicate_json_keys.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[3])); } } - if let Some(rule) = self.no_excessive_nested_test_suites.as_ref() { + if let Some(rule) = self.no_duplicate_test_hooks.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[4])); } } - if let Some(rule) = self.no_exports_in_test.as_ref() { + if let Some(rule) = self.no_excessive_nested_test_suites.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5])); } } - if let Some(rule) = self.no_focused_tests.as_ref() { + if let Some(rule) = self.no_exports_in_test.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6])); } } - if let Some(rule) = self.no_namespace_import.as_ref() { + if let Some(rule) = self.no_focused_tests.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.no_nodejs_modules.as_ref() { + if let Some(rule) = self.no_namespace_import.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_re_export_all.as_ref() { + if let Some(rule) = self.no_nodejs_modules.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_restricted_imports.as_ref() { + if let Some(rule) = self.no_re_export_all.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.no_semicolon_in_jsx.as_ref() { + if let Some(rule) = self.no_restricted_imports.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.no_skipped_tests.as_ref() { + if let Some(rule) = self.no_semicolon_in_jsx.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.no_undeclared_dependencies.as_ref() { + if let Some(rule) = self.no_skipped_tests.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.no_useless_ternary.as_ref() { + if let Some(rule) = self.no_undeclared_dependencies.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.no_useless_ternary.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.use_jsx_key_in_iterable.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.use_node_assert_strict.as_ref() { + if let Some(rule) = self.use_jsx_key_in_iterable.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.use_sorted_classes.as_ref() { + if let Some(rule) = self.use_node_assert_strict.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } + if let Some(rule) = self.use_sorted_classes.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2853,91 +2865,96 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1])); } } - if let Some(rule) = self.no_duplicate_json_keys.as_ref() { + if let Some(rule) = self.no_done_callback.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2])); } } - if let Some(rule) = self.no_duplicate_test_hooks.as_ref() { + if let Some(rule) = self.no_duplicate_json_keys.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[3])); } } - if let Some(rule) = self.no_excessive_nested_test_suites.as_ref() { + if let Some(rule) = self.no_duplicate_test_hooks.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[4])); } } - if let Some(rule) = self.no_exports_in_test.as_ref() { + if let Some(rule) = self.no_excessive_nested_test_suites.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5])); } } - if let Some(rule) = self.no_focused_tests.as_ref() { + if let Some(rule) = self.no_exports_in_test.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6])); } } - if let Some(rule) = self.no_namespace_import.as_ref() { + if let Some(rule) = self.no_focused_tests.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.no_nodejs_modules.as_ref() { + if let Some(rule) = self.no_namespace_import.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_re_export_all.as_ref() { + if let Some(rule) = self.no_nodejs_modules.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_restricted_imports.as_ref() { + if let Some(rule) = self.no_re_export_all.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.no_semicolon_in_jsx.as_ref() { + if let Some(rule) = self.no_restricted_imports.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.no_skipped_tests.as_ref() { + if let Some(rule) = self.no_semicolon_in_jsx.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.no_undeclared_dependencies.as_ref() { + if let Some(rule) = self.no_skipped_tests.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.no_useless_ternary.as_ref() { + if let Some(rule) = self.no_undeclared_dependencies.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.no_useless_ternary.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.use_jsx_key_in_iterable.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.use_node_assert_strict.as_ref() { + if let Some(rule) = self.use_jsx_key_in_iterable.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.use_sorted_classes.as_ref() { + if let Some(rule) = self.use_node_assert_strict.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } + if let Some(rule) = self.use_sorted_classes.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2948,10 +2965,10 @@ impl Nursery { pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) } - pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 7] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 8] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 19] { + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 20] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] @@ -2985,6 +3002,10 @@ impl Nursery { .no_console .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "noDoneCallback" => self + .no_done_callback + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "noDuplicateJsonKeys" => self .no_duplicate_json_keys .as_ref() diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index e2198b6111d0..d97f5f0c9e65 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -896,6 +896,10 @@ export interface Nursery { * Disallow the use of console. */ noConsole?: RuleConfiguration_for_Null; + /** + * Disallow using a callback in asynchronous tests and hooks. + */ + noDoneCallback?: RuleConfiguration_for_Null; /** * Disallow two keys with the same name inside a JSON object. */ @@ -1882,6 +1886,7 @@ export type Category = | "lint/nursery/noApproximativeNumericConstant" | "lint/nursery/noBarrelFile" | "lint/nursery/noConsole" + | "lint/nursery/noDoneCallback" | "lint/nursery/noDuplicateJsonKeys" | "lint/nursery/noDuplicateTestHooks" | "lint/nursery/noExcessiveNestedTestSuites" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 0b2e91a8641c..448853176d9f 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -1395,6 +1395,13 @@ { "type": "null" } ] }, + "noDoneCallback": { + "description": "Disallow using a callback in asynchronous tests and hooks.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noDuplicateJsonKeys": { "description": "Disallow two keys with the same name inside a JSON object.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 69da6790e6f1..f9d5ca690c8a 100644 --- a/website/src/components/generated/NumberOfRules.astro +++ b/website/src/components/generated/NumberOfRules.astro @@ -1,2 +1,2 @@ -

Biome's linter has a total of 208 rules

\ No newline at end of file +

Biome's linter has a total of 209 rules

\ No newline at end of file diff --git a/website/src/content/docs/internals/changelog.md b/website/src/content/docs/internals/changelog.md index d575c1197bb1..8d36c196c7fe 100644 --- a/website/src/content/docs/internals/changelog.md +++ b/website/src/content/docs/internals/changelog.md @@ -45,6 +45,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 @@ -99,7 +108,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 diff --git a/website/src/content/docs/linter/rules/index.mdx b/website/src/content/docs/linter/rules/index.mdx index 715282cedc13..c016142657c0 100644 --- a/website/src/content/docs/linter/rules/index.mdx +++ b/website/src/content/docs/linter/rules/index.mdx @@ -253,6 +253,7 @@ Rules that belong to this group are not subject to semantic versionconsole. | ⚠️ | +| [noDoneCallback](/linter/rules/no-done-callback) | Disallow using a callback in asynchronous tests and hooks. | | | [noDuplicateJsonKeys](/linter/rules/no-duplicate-json-keys) | Disallow two keys with the same name inside a JSON object. | | | [noDuplicateTestHooks](/linter/rules/no-duplicate-test-hooks) | A describe block should not contain duplicate hooks. | | | [noExcessiveNestedTestSuites](/linter/rules/no-excessive-nested-test-suites) | This rule enforces a maximum depth to nested describe() in test files. | | diff --git a/website/src/content/docs/linter/rules/no-done-callback.md b/website/src/content/docs/linter/rules/no-done-callback.md new file mode 100644 index 000000000000..9304a4bfa082 --- /dev/null +++ b/website/src/content/docs/linter/rules/no-done-callback.md @@ -0,0 +1,80 @@ +--- +title: noDoneCallback (not released) +--- + +**Diagnostic Category: `lint/nursery/noDoneCallback`** + +:::danger +This rule hasn't been released yet. +::: + +:::caution +This rule is part of the [nursery](/linter/rules/#nursery) group. +::: + +Source: no-done-callback + +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 + +```jsx +beforeEach(done => { + // ... +}); +``` + +

nursery/noDoneCallback.js:1:12 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Disallow using a callback in asynchronous tests and hooks.
+  
+  > 1 │ beforeEach(done => {
+      │            ^^^^
+    2 │     // ...
+    3 │ });
+  
+  ℹ Return a Promise instead of relying on callback parameter.
+  
+
+ +```jsx +test('myFunction()', done => { + // ... +}); +``` + +
nursery/noDoneCallback.js:1:22 lint/nursery/noDoneCallback ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Disallow using a callback in asynchronous tests and hooks.
+  
+  > 1 │ test('myFunction()', done => {
+      │                      ^^^^
+    2 │     // ...
+    3 │ });
+  
+  ℹ Return a Promise instead of relying on callback parameter.
+  
+
+ +### Valid + +```jsx +beforeEach(async () => { + // ... +}); +``` + +```jsx +test('myFunction()', () => { + expect(myFunction()).toBeTruthy(); +}); +``` + +## Related links + +- [Disable a rule](/linter/#disable-a-lint-rule) +- [Rule options](/linter/#rule-options)