From 3ad47b0bbf41f84244e92bf9dc97b232a6c3734f Mon Sep 17 00:00:00 2001 From: tobinio Date: Tue, 19 Aug 2025 18:43:37 +0200 Subject: [PATCH 01/14] chore(noDuplicateTestHooks): use is_test_call_expression() instead of custom handeling --- .../suspicious/no_duplicate_test_hooks.rs | 52 ++-- .../src/lint/suspicious/no_skipped_tests.rs | 2 +- crates/biome_js_syntax/src/expr_ext.rs | 248 +++++++++--------- 3 files changed, 152 insertions(+), 150 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index aefcaf66485f..0650b6dad9b6 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -121,24 +121,8 @@ impl Visitor for DuplicateHooksVisitor { }; // When the visitor enters a function node, push a new entry on the stack - if let Ok(callee) = node.callee() { - if callee.contains_a_test_pattern() == Ok(true) { - if let Some(function_name) = callee.get_callee_object_name() - && function_name.text_trimmed() == "describe" - { - self.stack.push(HooksContext::default()); - } - } - // describe.each has a different syntax - else if let AnyJsExpression::JsCallExpression(call_expression) = callee - && let Ok(callee) = call_expression.callee() - && matches!( - callee.to_trimmed_text().text(), - "describe.each" | "describe.only.each" | "fdescribe.each" - ) - { - self.stack.push(HooksContext::default()); - } + if DuplicateHooksVisitor::is_test_describe_call(&node) { + self.stack.push(HooksContext::default()); } if let Ok(AnyJsExpression::JsIdentifierExpression(identifier)) = node.callee() { @@ -166,10 +150,7 @@ impl Visitor for DuplicateHooksVisitor { // When the visitor exits a function, if it matches the node of the top-most // entry of the stack and the `has_yield` flag is `false`, emit a query match if let Some(node) = JsCallExpression::cast_ref(node) - && let Ok(callee) = node.callee() - && callee.contains_a_test_pattern() == Ok(true) - && let Some(function_name) = callee.get_callee_object_name() - && function_name.text_trimmed() == "describe" + && DuplicateHooksVisitor::is_test_describe_call(&node) { self.stack.pop(); } @@ -178,6 +159,33 @@ impl Visitor for DuplicateHooksVisitor { } } +impl DuplicateHooksVisitor { + fn is_test_describe_call(node: &JsCallExpression) -> bool { + if let Ok(callee) = node.callee() + && node.is_test_call_expression() == Ok(true) + { + // describe.each has a nested call expression + let callee = if let AnyJsExpression::JsCallExpression(call_expression) = callee { + if let Ok(callee) = call_expression.callee() { + callee + } else { + return false; + } + } else { + callee + }; + + if let Some(function_name) = callee.get_callee_object_name() + && function_name.text_trimmed() == "describe" + { + return true; + } + } + + false + } +} + // Declare a query match struct type containing a JavaScript function node pub struct DuplicateHooks(JsCallExpression); diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_skipped_tests.rs b/crates/biome_js_analyze/src/lint/suspicious/no_skipped_tests.rs index b09c086004ae..259873b5d113 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_skipped_tests.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_skipped_tests.rs @@ -61,7 +61,7 @@ impl Rule for NoSkippedTests { if node.is_test_call_expression().ok()? { let callee = node.callee().ok()?; - if callee.contains_a_test_pattern().ok()? { + if callee.contains_a_test_pattern() { let function_name = callee.get_callee_member_name()?; if FUNCTION_NAMES.contains(&function_name.text_trimmed()) { diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index c1396a950376..d0922654bef9 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -690,111 +690,9 @@ impl JsTemplateExpression { self.is_test_each_pattern_callee() && self.is_test_each_pattern_elements() } - /// This function checks if a call expressions has one of the following members: - /// - `describe.each` - /// - `describe.only.each` - /// - `describe.skip.each` - /// - `test.concurrent.each` - /// - `test.concurrent.only.each` - /// - `test.concurrent.skip.each` - /// - `test.each` - /// - `test.only.each` - /// - `test.skip.each` - /// - `test.failing.each` - /// - `it.concurrent.each` - /// - `it.concurrent.only.each` - /// - `it.concurrent.skip.each` - /// - `it.each` - /// - `it.only.each` - /// - `it.skip.each` - /// - `it.failing.each` - /// - /// - `xdescribe.each` - /// - `xdescribe.only.each` - /// - `xdescribe.skip.each` - /// - `xtest.concurrent.each` - /// - `xtest.concurrent.only.each` - /// - `xtest.concurrent.skip.each` - /// - `xtest.each` - /// - `xtest.only.each` - /// - `xtest.skip.each` - /// - `xtest.failing.each` - /// - `xit.concurrent.each` - /// - `xit.concurrent.only.each` - /// - `xit.concurrent.skip.each` - /// - `xit.each` - /// - `xit.only.each` - /// - `xit.skip.each` - /// - `xit.failing.each` - /// - /// - `fdescribe.each` - /// - `fdescribe.only.each` - /// - `fdescribe.skip.each` - /// - `ftest.concurrent.each` - /// - `ftest.concurrent.only.each` - /// - `ftest.concurrent.skip.each` - /// - `ftest.each` - /// - `ftest.only.each` - /// - `ftest.skip.each` - /// - `ftest.failing.each` - /// - `fit.concurrent.each` - /// - `fit.concurrent.only.each` - /// - `fit.concurrent.skip.each` - /// - `fit.each` - /// - `fit.only.each` - /// - `fit.skip.each` - /// - `xit.failing.each` - /// - /// Based on this [article] - /// - /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines pub fn is_test_each_pattern_callee(&self) -> bool { if let Some(tag) = self.tag() { - let mut members = CalleeNamesIterator::new(tag); - - let texts: [Option; 5] = [ - members.next(), - members.next(), - members.next(), - members.next(), - members.next(), - ]; - - let mut rev = texts.iter().rev().flatten(); - - let first = rev.next().map(|t| t.text()); - let second = rev.next().map(|t| t.text()); - let third = rev.next().map(|t| t.text()); - let fourth = rev.next().map(|t| t.text()); - let fifth = rev.next().map(|t| t.text()); - - match first { - Some("describe" | "xdescribe" | "fdescribe") => match second { - Some("each") => third.is_none(), - Some("skip" | "only") => match third { - Some("each") => fourth.is_none(), - _ => false, - }, - _ => false, - }, - Some("test" | "xtest" | "ftest" | "it" | "xit" | "fit") => match second { - Some("each") => third.is_none(), - Some("skip" | "only" | "failing") => match third { - Some("each") => fourth.is_none(), - _ => false, - }, - Some("concurrent") => match third { - Some("each") => fourth.is_none(), - Some("only" | "skip") => match fourth { - Some("each") => fifth.is_none(), - _ => false, - }, - _ => false, - }, - _ => false, - }, - _ => false, - } + tag.contains_a_test_each_pattern() } else { false } @@ -1060,7 +958,7 @@ impl AnyJsExpression { /// Based on this [article] /// /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines - pub fn contains_a_test_pattern(&self) -> SyntaxResult { + pub fn contains_a_test_pattern(&self) -> bool { let mut members = CalleeNamesIterator::new(self.clone()); let texts: [Option; 5] = [ @@ -1079,7 +977,7 @@ impl AnyJsExpression { let fourth = rev.next().map(|t| t.text()); let fifth = rev.next().map(|t| t.text()); - Ok(match first { + match first { Some("it" | "describe" | "Deno") => match second { None => true, Some("only" | "skip" | "test") => third.is_none(), @@ -1102,7 +1000,113 @@ impl AnyJsExpression { }, Some("skip" | "xit" | "xdescribe" | "xtest" | "fit" | "fdescribe" | "ftest") => true, _ => false, - }) + } + } + + /// This function checks if a call expressions has one of the following members: + /// - `describe.each` + /// - `describe.only.each` + /// - `describe.skip.each` + /// - `test.concurrent.each` + /// - `test.concurrent.only.each` + /// - `test.concurrent.skip.each` + /// - `test.each` + /// - `test.only.each` + /// - `test.skip.each` + /// - `test.failing.each` + /// - `it.concurrent.each` + /// - `it.concurrent.only.each` + /// - `it.concurrent.skip.each` + /// - `it.each` + /// - `it.only.each` + /// - `it.skip.each` + /// - `it.failing.each` + /// + /// - `xdescribe.each` + /// - `xdescribe.only.each` + /// - `xdescribe.skip.each` + /// - `xtest.concurrent.each` + /// - `xtest.concurrent.only.each` + /// - `xtest.concurrent.skip.each` + /// - `xtest.each` + /// - `xtest.only.each` + /// - `xtest.skip.each` + /// - `xtest.failing.each` + /// - `xit.concurrent.each` + /// - `xit.concurrent.only.each` + /// - `xit.concurrent.skip.each` + /// - `xit.each` + /// - `xit.only.each` + /// - `xit.skip.each` + /// - `xit.failing.each` + /// + /// - `fdescribe.each` + /// - `fdescribe.only.each` + /// - `fdescribe.skip.each` + /// - `ftest.concurrent.each` + /// - `ftest.concurrent.only.each` + /// - `ftest.concurrent.skip.each` + /// - `ftest.each` + /// - `ftest.only.each` + /// - `ftest.skip.each` + /// - `ftest.failing.each` + /// - `fit.concurrent.each` + /// - `fit.concurrent.only.each` + /// - `fit.concurrent.skip.each` + /// - `fit.each` + /// - `fit.only.each` + /// - `fit.skip.each` + /// - `xit.failing.each` + /// + /// Based on this [article] + /// + /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines + pub fn contains_a_test_each_pattern(&self) -> bool { + let mut members = CalleeNamesIterator::new(self.clone()); + + let texts: [Option; 5] = [ + members.next(), + members.next(), + members.next(), + members.next(), + members.next(), + ]; + + let mut rev = texts.iter().rev().flatten(); + + let first = rev.next().map(|t| t.text()); + let second = rev.next().map(|t| t.text()); + let third = rev.next().map(|t| t.text()); + let fourth = rev.next().map(|t| t.text()); + let fifth = rev.next().map(|t| t.text()); + + match first { + Some("describe" | "xdescribe" | "fdescribe") => match second { + Some("each") => third.is_none(), + Some("skip" | "only") => match third { + Some("each") => fourth.is_none(), + _ => false, + }, + _ => false, + }, + Some("test" | "xtest" | "ftest" | "it" | "xit" | "fit") => match second { + Some("each") => third.is_none(), + Some("skip" | "only" | "failing") => match third { + Some("each") => fourth.is_none(), + _ => false, + }, + Some("concurrent") => match third { + Some("each") => fourth.is_none(), + Some("only" | "skip") => match fourth { + Some("each") => fifth.is_none(), + _ => false, + }, + _ => false, + }, + _ => false, + }, + _ => false, + } } /// Checks whether the current function call is: @@ -2066,7 +2070,9 @@ impl JsCallExpression { // it("description", ..) // it(Test.name, ..) (Some(Ok(AnyJsCallArgument::AnyJsExpression(_))), Some(Ok(second)), third) - if arguments.args().len() <= 3 && callee.contains_a_test_pattern()? => + if arguments.args().len() <= 3 + && (callee.contains_a_test_pattern() + || matches!(callee, AnyJsExpression::JsCallExpression(call_expression) if call_expression.callee()?.contains_a_test_each_pattern())) => { // it('name', callback, duration) if !matches!( @@ -2272,43 +2278,28 @@ mod test { #[test] fn matches_simple_call() { let call_expression = extract_call_expression("test();"); - assert_eq!( - call_expression.callee().unwrap().contains_a_test_pattern(), - Ok(true) - ); + assert!(call_expression.callee().unwrap().contains_a_test_pattern()); let call_expression = extract_call_expression("it();"); - assert_eq!( - call_expression.callee().unwrap().contains_a_test_pattern(), - Ok(true) - ); + assert!(call_expression.callee().unwrap().contains_a_test_pattern()); } #[test] fn matches_static_member_expression() { let call_expression = extract_call_expression("test.only();"); - assert_eq!( - call_expression.callee().unwrap().contains_a_test_pattern(), - Ok(true) - ); + assert!(call_expression.callee().unwrap().contains_a_test_pattern()); } #[test] fn matches_static_member_expression_deep() { let call_expression = extract_call_expression("test.describe.parallel.only();"); - assert_eq!( - call_expression.callee().unwrap().contains_a_test_pattern(), - Ok(true) - ); + assert!(call_expression.callee().unwrap().contains_a_test_pattern()); } #[test] fn doesnt_static_member_expression_deep() { let call_expression = extract_call_expression("test.describe.parallel.only.AHAHA();"); - assert_eq!( - call_expression.callee().unwrap().contains_a_test_pattern(), - Ok(false) - ); + assert!(!call_expression.callee().unwrap().contains_a_test_pattern()); } #[test] @@ -2332,6 +2323,9 @@ mod test { let call_expression = extract_call_expression("describe.only(name = name || 'test', () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); + + let call_expression = extract_call_expression("describe.each([])(name, () => {});"); + assert_eq!(call_expression.is_test_call_expression(), Ok(true)); } #[test] From 0287bc27bc783a1ffc6dbe4bae045ae062de3885 Mon Sep 17 00:00:00 2001 From: tobinio Date: Thu, 21 Aug 2025 20:13:02 +0200 Subject: [PATCH 02/14] fix: increase amout of detected test functions --- .../suspicious/no_duplicate_test_hooks.rs | 7 +- .../suspicious/noDuplicateTestHooks/valid.js | 22 + .../noDuplicateTestHooks/valid.js.snap | 23 + .../test_declarations.js.snap | 479 ++++++++++++++++++ crates/biome_js_syntax/src/expr_ext.rs | 199 +++----- 5 files changed, 595 insertions(+), 135 deletions(-) create mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index 0650b6dad9b6..acc24879eb1d 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -175,9 +175,10 @@ impl DuplicateHooksVisitor { callee }; - if let Some(function_name) = callee.get_callee_object_name() - && function_name.text_trimmed() == "describe" - { + let text = callee.to_trimmed_text(); + let base = text.split(".").next().unwrap_or_default(); + + if base == "describe" { return true; } } diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js index 36c332ec481a..8694ab071344 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js +++ b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js @@ -82,3 +82,25 @@ describe("hello", () => { ); }); +describe("something", () => { + beforeEach(() => {}); + describe("something", () => { + beforeEach(() => {}); + }); + + describe.skip.each([])("something", () => { + beforeEach(() => {}); + }); + + describe.for([])("something", () => { + beforeEach(() => {}); + }); + + describe.todo("something", () => { + beforeEach(() => {}); + }); + + describe.todo.each([])("something", () => { + beforeEach(() => {}); + }); +}); diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap index a6406626f509..9b2fbce2c79c 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap +++ b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap @@ -1,5 +1,6 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 152 expression: valid.js --- # Input @@ -88,5 +89,27 @@ describe("hello", () => { ); }); +describe("something", () => { + beforeEach(() => {}); + describe("something", () => { + beforeEach(() => {}); + }); + + describe.skip.each([])("something", () => { + beforeEach(() => {}); + }); + + describe.for([])("something", () => { + beforeEach(() => {}); + }); + + describe.todo("something", () => { + beforeEach(() => {}); + }); + + describe.todo.each([])("something", () => { + beforeEach(() => {}); + }); +}); ``` diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap new file mode 100644 index 000000000000..038bb4423aff --- /dev/null +++ b/crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap @@ -0,0 +1,479 @@ +--- +source: crates/biome_formatter_test/src/snapshot_builder.rs +assertion_line: 211 +info: js/test-declarations/test_declarations.js +--- +# Input + +```js +// Shouldn't break + +it("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); +}); + +it("does something really long and complicated so I have to write a very long name for the test", function() { + console.log("hello!"); +}); + +it("does something really long and complicated so I have to write a very long name for the test", function(done) { + console.log("hello!"); +}); + +it("does something really long and complicated so I have to write a very long name for the test", function myAssertions(done) { + console.log("hello!"); +}); + +it(`does something really long and complicated so I have to write a very long name for the test`, function() { + console.log("hello!"); +}); + +it(`{foo + bar} does something really long and complicated so I have to write a very long name for the test`, function() { + console.log("hello!"); +}); + +it(`handles + some + newlines + does something really long and complicated so I have to write a very long name for the test`, () => { + console.log("hello!"); +}) + +test("does something really long and complicated so I have to write a very long name for the test", (done) => { + console.log("hello!"); +}); + +test(`does something really long and complicated so I have to write a very long name for the test`, (done) => { + console.log("hello!"); +}); + +describe("does something really long and complicated so I have to write a very long name for the describe block", () => { + it("an example test", (done) => { + console.log("hello!"); + }); +}); + +describe(`does something really long and complicated so I have to write a very long name for the describe block`, () => { + it(`an example test`, (done) => { + console.log("hello!"); + }); +}); + +xdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +fdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +describe.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +fit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +xit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +it.only("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); +}); + +it.only(`does something really long and complicated so I have to write a very long name for the test`, () => { + console.log("hello!"); +}); + +it.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +ftest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +xtest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +skip("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.step("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.step(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe.only("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe.parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.parallel(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe.parallel.only("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); + +test.describe.parallel.only(`does something really long and complicated so I have to write a very long name for the testThis is a very`, () => {}); + +test.describe.serial("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.serial(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe.serial.only("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.serial.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +// Should break + +it.only("does something really long and complicated so I have to write a very long name for the test", 10, () => { + console.log("hello!"); +}); + +it.only.only("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); +}); + +it.only.only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { + console.log("hello!"); +}); + +xskip("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.only.parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.parallel.serial("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); + +test.serial("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.dummy.serial("does something really long and complicated so I have to write a very long name for the test", () => {}); + +// timeout + +it(`handles + some + newlines + does something really long and complicated so I have to write a very long name for the test`, () => { + console.log("hello!"); +}, 2500) + +it("does something quick", () => { + console.log("hello!") +}, 1000000000) + +it( + 'succeeds if the test finishes in time', + () => new Promise(resolve => setTimeout(resolve, 10)) +); + +it( + 'succeeds if the test finishes in time', + () => new Promise(resolve => setTimeout(resolve, 10)), + 250 +); + +``` + + +# Prettier differences + +```diff +--- Prettier ++++ Biome +@@ -85,9 +85,15 @@ + + skip("does something really long and complicated so I have to write a very long name for the test", () => {}); + +-test.step("does something really long and complicated so I have to write a very long name for the test", () => {}); ++test.step( ++ "does something really long and complicated so I have to write a very long name for the test", ++ () => {}, ++); + +-test.step(`does something really long and complicated so I have to write a very long name for the test`, () => {}); ++test.step( ++ `does something really long and complicated so I have to write a very long name for the test`, ++ () => {}, ++); + + test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); + +@@ -133,19 +139,15 @@ + }, + ); + +-it.only.only( +- "does something really long and complicated so I have to write a very long name for the test", +- () => { ++it.only ++ .only("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); +- }, +-); ++ }); + +-it.only.only( +- "does something really long and complicated so I have to write a very long name for the test", +- (a, b, c) => { ++it.only ++ .only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { + console.log("hello!"); +- }, +-); ++ }); + + xskip( + "does something really long and complicated so I have to write a very long name for the test", +``` + +# Output + +```js +// Shouldn't break + +it("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); +}); + +it("does something really long and complicated so I have to write a very long name for the test", function () { + console.log("hello!"); +}); + +it("does something really long and complicated so I have to write a very long name for the test", function (done) { + console.log("hello!"); +}); + +it("does something really long and complicated so I have to write a very long name for the test", function myAssertions(done) { + console.log("hello!"); +}); + +it(`does something really long and complicated so I have to write a very long name for the test`, function () { + console.log("hello!"); +}); + +it(`{foo + bar} does something really long and complicated so I have to write a very long name for the test`, function () { + console.log("hello!"); +}); + +it(`handles + some + newlines + does something really long and complicated so I have to write a very long name for the test`, () => { + console.log("hello!"); +}); + +test("does something really long and complicated so I have to write a very long name for the test", (done) => { + console.log("hello!"); +}); + +test(`does something really long and complicated so I have to write a very long name for the test`, (done) => { + console.log("hello!"); +}); + +describe("does something really long and complicated so I have to write a very long name for the describe block", () => { + it("an example test", (done) => { + console.log("hello!"); + }); +}); + +describe(`does something really long and complicated so I have to write a very long name for the describe block`, () => { + it(`an example test`, (done) => { + console.log("hello!"); + }); +}); + +xdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +fdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +describe.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +fit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +xit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +it.only("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); +}); + +it.only(`does something really long and complicated so I have to write a very long name for the test`, () => { + console.log("hello!"); +}); + +it.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +ftest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +xtest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + +skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +skip("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.step( + "does something really long and complicated so I have to write a very long name for the test", + () => {}, +); + +test.step( + `does something really long and complicated so I have to write a very long name for the test`, + () => {}, +); + +test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe + .only("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe + .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe + .parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe + .parallel(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe.parallel + .only("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); + +test.describe.parallel + .only(`does something really long and complicated so I have to write a very long name for the testThis is a very`, () => {}); + +test.describe + .serial("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe + .serial(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +test.describe.serial + .only("does something really long and complicated so I have to write a very long name for the test", () => {}); + +test.describe.serial + .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + +// Should break + +it.only( + "does something really long and complicated so I have to write a very long name for the test", + 10, + () => { + console.log("hello!"); + }, +); + +it.only + .only("does something really long and complicated so I have to write a very long name for the test", () => { + console.log("hello!"); + }); + +it.only + .only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { + console.log("hello!"); + }); + +xskip( + "does something really long and complicated so I have to write a very long name for the test", + () => {}, +); + +test.describe.only.parallel( + "does something really long and complicated so I have to write a very long name for the test", + () => {}, +); + +test.describe.parallel.serial( + "does something really long and complicated so I have to write a very long name for the testThis is a very", + () => {}, +); + +test.serial( + "does something really long and complicated so I have to write a very long name for the test", + () => {}, +); + +test.describe.dummy.serial( + "does something really long and complicated so I have to write a very long name for the test", + () => {}, +); + +// timeout + +it(`handles + some + newlines + does something really long and complicated so I have to write a very long name for the test`, () => { + console.log("hello!"); +}, 2500); + +it("does something quick", () => { + console.log("hello!"); +}, 1000000000); + +it("succeeds if the test finishes in time", () => + new Promise((resolve) => setTimeout(resolve, 10))); + +it( + "succeeds if the test finishes in time", + () => new Promise((resolve) => setTimeout(resolve, 10)), + 250, +); +``` + +# Lines exceeding max width of 80 characters +``` + 3: it("does something really long and complicated so I have to write a very long name for the test", () => { + 7: it("does something really long and complicated so I have to write a very long name for the test", function () { + 11: it("does something really long and complicated so I have to write a very long name for the test", function (done) { + 15: it("does something really long and complicated so I have to write a very long name for the test", function myAssertions(done) { + 19: it(`does something really long and complicated so I have to write a very long name for the test`, function () { + 23: it(`{foo + bar} does something really long and complicated so I have to write a very long name for the test`, function () { + 30: does something really long and complicated so I have to write a very long name for the test`, () => { + 34: test("does something really long and complicated so I have to write a very long name for the test", (done) => { + 38: test(`does something really long and complicated so I have to write a very long name for the test`, (done) => { + 42: describe("does something really long and complicated so I have to write a very long name for the describe block", () => { + 48: describe(`does something really long and complicated so I have to write a very long name for the describe block`, () => { + 54: xdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + 56: fdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + 58: describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 60: describe.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 62: fit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + 64: xit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + 66: it.only("does something really long and complicated so I have to write a very long name for the test", () => { + 70: it.only(`does something really long and complicated so I have to write a very long name for the test`, () => { + 74: it.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 76: test.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 78: test.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 80: ftest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + 82: xtest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); + 84: skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 86: skip("does something really long and complicated so I have to write a very long name for the test", () => {}); + 89: "does something really long and complicated so I have to write a very long name for the test", + 94: `does something really long and complicated so I have to write a very long name for the test`, + 98: test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); + 100: test.describe(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 103: .only("does something really long and complicated so I have to write a very long name for the test", () => {}); + 106: .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 109: .parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); + 112: .parallel(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 115: .only("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); + 118: .only(`does something really long and complicated so I have to write a very long name for the testThis is a very`, () => {}); + 121: .serial("does something really long and complicated so I have to write a very long name for the test", () => {}); + 124: .serial(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 127: .only("does something really long and complicated so I have to write a very long name for the test", () => {}); + 130: .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); + 135: "does something really long and complicated so I have to write a very long name for the test", + 143: .only("does something really long and complicated so I have to write a very long name for the test", () => { + 148: .only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { + 153: "does something really long and complicated so I have to write a very long name for the test", + 158: "does something really long and complicated so I have to write a very long name for the test", + 163: "does something really long and complicated so I have to write a very long name for the testThis is a very", + 168: "does something really long and complicated so I have to write a very long name for the test", + 173: "does something really long and complicated so I have to write a very long name for the test", + 182: does something really long and complicated so I have to write a very long name for the test`, () => { +``` diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index d0922654bef9..d591f9796157 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -930,22 +930,12 @@ impl AnyJsExpression { } /// This function checks if a call expressions has one of the following members: - /// - `it` - /// - `it.only` - /// - `it.skip` - /// - `describe` - /// - `describe.only` - /// - `describe.skip` - /// - `test` - /// - `test.only` - /// - `test.skip` - /// - `test.step` - /// - `test.describe` - /// - `test.describe.only` - /// - `test.describe.parallel` - /// - `test.describe.parallel.only` - /// - `test.describe.serial` - /// - `test.describe.serial.only` + /// + /// - `it[.(only|skip|todo|fails|failing|concurrent|sequential)]*` + /// - `test[.(only|skip|todo|fails|failing|concurrent|sequential)]*` + /// - `describe[.(only|skip|todo|shuffle|concurrent|sequential)]*` + /// - `test.describe[.(only|skip)]?` + /// - `test.describe.(parallel|serial)[.only]?` /// - `skip` /// - `xit` /// - `xdescribe` @@ -959,17 +949,8 @@ impl AnyJsExpression { /// /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines pub fn contains_a_test_pattern(&self) -> bool { - let mut members = CalleeNamesIterator::new(self.clone()); - - let texts: [Option; 5] = [ - members.next(), - members.next(), - members.next(), - members.next(), - members.next(), - ]; - - let mut rev = texts.iter().rev().flatten(); + let members = CalleeNamesIterator::new(self.clone()).collect::>(); + let mut rev = members.iter().rev(); let first = rev.next().map(|t| t.text()); let second = rev.next().map(|t| t.text()); @@ -978,17 +959,34 @@ impl AnyJsExpression { let fifth = rev.next().map(|t| t.text()); match first { - Some("it" | "describe" | "Deno") => match second { + Some("describe") => match second { None => true, - Some("only" | "skip" | "test") => third.is_none(), + Some("concurrent" | "sequential" | "only" | "skip" | "todo" | "shuffle") => { + match third { + None + | Some( + "concurrent" | "sequential" | "only" | "skip" | "todo" | "shuffle", + ) => true, + _ => false, + } + } _ => false, }, - Some("test") => match second { + Some("it" | "test") => match second { None => true, - Some("only" | "skip" | "step") => third.is_none(), + Some( + "concurrent" | "sequential" | "only" | "skip" | "todo" | "fails" | "failing", + ) => match third { + None + | Some( + "concurrent" | "sequential" | "only" | "skip" | "todo" | "fails" + | "failing", + ) => true, + _ => false, + }, Some("describe") => match third { None => true, - Some("only") => fourth.is_none(), + Some("only" | "skip") => fourth.is_none(), Some("parallel" | "serial") => match fourth { None => true, Some("only") => fifth.is_none(), @@ -998,115 +996,38 @@ impl AnyJsExpression { }, _ => false, }, + Some("Deno") => match second { + Some("test") => third.is_none(), + _ => false, + }, Some("skip" | "xit" | "xdescribe" | "xtest" | "fit" | "fdescribe" | "ftest") => true, _ => false, } } - /// This function checks if a call expressions has one of the following members: - /// - `describe.each` - /// - `describe.only.each` - /// - `describe.skip.each` - /// - `test.concurrent.each` - /// - `test.concurrent.only.each` - /// - `test.concurrent.skip.each` - /// - `test.each` - /// - `test.only.each` - /// - `test.skip.each` - /// - `test.failing.each` - /// - `it.concurrent.each` - /// - `it.concurrent.only.each` - /// - `it.concurrent.skip.each` - /// - `it.each` - /// - `it.only.each` - /// - `it.skip.each` - /// - `it.failing.each` - /// - /// - `xdescribe.each` - /// - `xdescribe.only.each` - /// - `xdescribe.skip.each` - /// - `xtest.concurrent.each` - /// - `xtest.concurrent.only.each` - /// - `xtest.concurrent.skip.each` - /// - `xtest.each` - /// - `xtest.only.each` - /// - `xtest.skip.each` - /// - `xtest.failing.each` - /// - `xit.concurrent.each` - /// - `xit.concurrent.only.each` - /// - `xit.concurrent.skip.each` - /// - `xit.each` - /// - `xit.only.each` - /// - `xit.skip.each` - /// - `xit.failing.each` - /// - /// - `fdescribe.each` - /// - `fdescribe.only.each` - /// - `fdescribe.skip.each` - /// - `ftest.concurrent.each` - /// - `ftest.concurrent.only.each` - /// - `ftest.concurrent.skip.each` - /// - `ftest.each` - /// - `ftest.only.each` - /// - `ftest.skip.each` - /// - `ftest.failing.each` - /// - `fit.concurrent.each` - /// - `fit.concurrent.only.each` - /// - `fit.concurrent.skip.each` - /// - `fit.each` - /// - `fit.only.each` - /// - `fit.skip.each` - /// - `xit.failing.each` - /// - /// Based on this [article] - /// - /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines + /// A valid pattern: + /// - Starts with a valid test pattern + /// - Ends with `.each` or `.for` pub fn contains_a_test_each_pattern(&self) -> bool { - let mut members = CalleeNamesIterator::new(self.clone()); - - let texts: [Option; 5] = [ - members.next(), - members.next(), - members.next(), - members.next(), - members.next(), - ]; + let AnyJsExpression::JsStaticMemberExpression(member_expression) = (*self).clone() else { + return false; + }; - let mut rev = texts.iter().rev().flatten(); + if matches!(member_expression.object().ok(), Some(rest) if !rest.contains_a_test_pattern()) + { + return false; + } - let first = rev.next().map(|t| t.text()); - let second = rev.next().map(|t| t.text()); - let third = rev.next().map(|t| t.text()); - let fourth = rev.next().map(|t| t.text()); - let fifth = rev.next().map(|t| t.text()); + if let Ok(AnyJsName::JsName(name)) = member_expression.member() + && let Some(first) = name + .value_token() + .ok() + .map(|name| name.token_text_trimmed()) + { + return matches!(first.text(), "each" | "for"); + }; - match first { - Some("describe" | "xdescribe" | "fdescribe") => match second { - Some("each") => third.is_none(), - Some("skip" | "only") => match third { - Some("each") => fourth.is_none(), - _ => false, - }, - _ => false, - }, - Some("test" | "xtest" | "ftest" | "it" | "xit" | "fit") => match second { - Some("each") => third.is_none(), - Some("skip" | "only" | "failing") => match third { - Some("each") => fourth.is_none(), - _ => false, - }, - Some("concurrent") => match third { - Some("each") => fourth.is_none(), - Some("only" | "skip") => match fourth { - Some("each") => fifth.is_none(), - _ => false, - }, - _ => false, - }, - _ => false, - }, - _ => false, - } + false } /// Checks whether the current function call is: @@ -2294,6 +2215,10 @@ mod test { fn matches_static_member_expression_deep() { let call_expression = extract_call_expression("test.describe.parallel.only();"); assert!(call_expression.callee().unwrap().contains_a_test_pattern()); + + let call_expression = + extract_call_expression("test.skip.sequential.only.skip.sequential.only();"); + assert!(call_expression.callee().unwrap().contains_a_test_pattern()); } #[test] @@ -2326,6 +2251,16 @@ mod test { let call_expression = extract_call_expression("describe.each([])(name, () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); + + let call_expression = extract_call_expression( + "test.skip.sequential.only.skip.sequential.only(Test.name, () => {});", + ); + assert_eq!(call_expression.is_test_call_expression(), Ok(true)); + + let call_expression = extract_call_expression( + "test.skip.sequential.only.todo.sequential.only.each([])(name, () => {});", + ); + assert_eq!(call_expression.is_test_call_expression(), Ok(true)); } #[test] From fba55400c54fda3722b67ede393745f2f6cf14ff Mon Sep 17 00:00:00 2001 From: tobinio Date: Fri, 22 Aug 2025 17:40:55 +0200 Subject: [PATCH 03/14] chore: somewhat improve code --- .../suspicious/no_duplicate_test_hooks.rs | 17 +----- crates/biome_js_syntax/src/expr_ext.rs | 58 ++++++++++--------- 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index acc24879eb1d..3a3e9f18816c 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -121,7 +121,7 @@ impl Visitor for DuplicateHooksVisitor { }; // When the visitor enters a function node, push a new entry on the stack - if DuplicateHooksVisitor::is_test_describe_call(&node) { + if Self::is_test_describe_call(&node) { self.stack.push(HooksContext::default()); } @@ -150,7 +150,7 @@ impl Visitor for DuplicateHooksVisitor { // When the visitor exits a function, if it matches the node of the top-most // entry of the stack and the `has_yield` flag is `false`, emit a query match if let Some(node) = JsCallExpression::cast_ref(node) - && DuplicateHooksVisitor::is_test_describe_call(&node) + && Self::is_test_describe_call(&node) { self.stack.pop(); } @@ -164,19 +164,8 @@ impl DuplicateHooksVisitor { if let Ok(callee) = node.callee() && node.is_test_call_expression() == Ok(true) { - // describe.each has a nested call expression - let callee = if let AnyJsExpression::JsCallExpression(call_expression) = callee { - if let Ok(callee) = call_expression.callee() { - callee - } else { - return false; - } - } else { - callee - }; - let text = callee.to_trimmed_text(); - let base = text.split(".").next().unwrap_or_default(); + let base = text.split('.').next().unwrap_or_default(); if base == "describe" { return true; diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index d591f9796157..c1181ca6d486 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -962,13 +962,12 @@ impl AnyJsExpression { Some("describe") => match second { None => true, Some("concurrent" | "sequential" | "only" | "skip" | "todo" | "shuffle") => { - match third { - None - | Some( + matches!( + third, + None | Some( "concurrent" | "sequential" | "only" | "skip" | "todo" | "shuffle", - ) => true, - _ => false, - } + ) + ) } _ => false, }, @@ -976,14 +975,18 @@ impl AnyJsExpression { None => true, Some( "concurrent" | "sequential" | "only" | "skip" | "todo" | "fails" | "failing", - ) => match third { - None - | Some( - "concurrent" | "sequential" | "only" | "skip" | "todo" | "fails" - | "failing", - ) => true, - _ => false, - }, + ) => matches!( + third, + None | Some( + "concurrent" + | "sequential" + | "only" + | "skip" + | "todo" + | "fails" + | "failing", + ) + ), Some("describe") => match third { None => true, Some("only" | "skip") => fourth.is_none(), @@ -1009,25 +1012,28 @@ impl AnyJsExpression { /// - Starts with a valid test pattern /// - Ends with `.each` or `.for` pub fn contains_a_test_each_pattern(&self) -> bool { - let AnyJsExpression::JsStaticMemberExpression(member_expression) = (*self).clone() else { + let Self::JsStaticMemberExpression(member_expression) = self else { return false; }; - if matches!(member_expression.object().ok(), Some(rest) if !rest.contains_a_test_pattern()) - { + if matches!(member_expression.object(), Ok(rest) if !rest.contains_a_test_pattern()) { return false; } - if let Ok(AnyJsName::JsName(name)) = member_expression.member() - && let Some(first) = name - .value_token() - .ok() - .map(|name| name.token_text_trimmed()) - { - return matches!(first.text(), "each" | "for"); - }; + match member_expression.member().ok() { + Some(AnyJsName::JsName(name)) => { + if let Some(token) = name + .value_token() + .ok() + .map(|token| token.token_text_trimmed()) + { + return matches!(token.text(), "each" | "for"); + } - false + false + } + _ => false, + } } /// Checks whether the current function call is: From c00617ac3ad97bdbde2c8c44176edd2cc061c68d Mon Sep 17 00:00:00 2001 From: tobinio Date: Fri, 22 Aug 2025 17:53:14 +0200 Subject: [PATCH 04/14] chore(noDuplicateTestHooks): improve diagnostic message --- .../suspicious/no_duplicate_test_hooks.rs | 4 +- .../noDuplicateTestHooks/invalid.js.snap | 62 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index 3a3e9f18816c..3dae4e112552 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -226,11 +226,11 @@ impl Rule for NoDuplicateTestHooks { rule_category!(), ctx.query().range(), markup! { - "Disallow duplicate setup and teardown hooks." + "Duplicate "{node_name.text_trimmed()}" hook found." }, ) .note(markup! { - "Disallow "{node_name.text_trimmed()}" duplicacy inside the describe function." + "Remove this duplicate hook or consolidate the logic into a single hook." }), ) } diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap index 90abe2773f7a..658dfac350d2 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap +++ b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap @@ -1,7 +1,7 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 152 expression: invalid.js -snapshot_kind: text --- # Input ```js @@ -180,7 +180,7 @@ describe("foo", () => { ``` invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 2 │ beforeEach(() => { 3 │ }); @@ -191,7 +191,7 @@ invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 6 │ test("bar", () => { 7 │ someFn(); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -199,7 +199,7 @@ invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeAll hook found. 14 │ beforeAll(() => { 15 │ }); @@ -210,7 +210,7 @@ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 18 │ test("bar", () => { 19 │ someFn(); - i Disallow beforeAll duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -218,7 +218,7 @@ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate afterEach hook found. 24 │ afterEach(() => { 25 │ }); @@ -229,7 +229,7 @@ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 28 │ test("bar", () => { 29 │ someFn(); - i Disallow afterEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -237,7 +237,7 @@ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate afterAll hook found. 34 │ afterAll(() => { 35 │ }); @@ -248,7 +248,7 @@ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 38 │ test("bar", () => { 39 │ someFn(); - i Disallow afterAll duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -256,7 +256,7 @@ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 44 │ beforeEach(() => { 45 │ }); @@ -267,7 +267,7 @@ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 48 │ beforeEach(() => { 49 │ }); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -275,7 +275,7 @@ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 46 │ beforeEach(() => { 47 │ }); @@ -286,7 +286,7 @@ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 50 │ test("bar", () => { 51 │ someFn(); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -294,7 +294,7 @@ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate afterAll hook found. 56 │ afterAll(() => { 57 │ }); @@ -305,7 +305,7 @@ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 60 │ beforeAll(() => { 61 │ }); - i Disallow afterAll duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -313,7 +313,7 @@ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeAll hook found. 60 │ beforeAll(() => { 61 │ }); @@ -324,7 +324,7 @@ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 64 │ test("bar", () => { 65 │ someFn(); - i Disallow beforeAll duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -332,7 +332,7 @@ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 70 │ beforeEach(() => { 71 │ }); @@ -343,7 +343,7 @@ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 74 │ beforeAll(() => { 75 │ }); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -351,7 +351,7 @@ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 88 │ beforeEach(() => { 89 │ }); @@ -362,7 +362,7 @@ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ 92 │ test("inner bar", () => { 93 │ someFn(); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -370,7 +370,7 @@ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ ``` invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 99 │ beforeEach(() => { 100 │ }); @@ -381,7 +381,7 @@ invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ 103 │ 104 │ it("is not fine", () => { - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -389,7 +389,7 @@ invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ ``` invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 118 │ beforeEach(() => { 119 │ }); @@ -400,7 +400,7 @@ invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ 122 │ 123 │ it("is not fine", () => { - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -408,7 +408,7 @@ invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ ``` invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 139 │ beforeEach(() => { 140 │ }); @@ -419,7 +419,7 @@ invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ 143 │ 144 │ it("is not fine", () => { - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -427,7 +427,7 @@ invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ ``` invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate before hook found. 151 │ before(() => { 152 │ }); @@ -438,7 +438,7 @@ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ 155 │ test("bar", () => { 156 │ someFn(); - i Disallow before duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` @@ -446,7 +446,7 @@ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ ``` invalid.js:163:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate after hook found. 161 │ after(() => { 162 │ }); @@ -457,7 +457,7 @@ invalid.js:163:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ 165 │ test("bar", () => { 166 │ someFn(); - i Disallow after duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` From fba6c5d4188d63ffee34688c22defe6836a1dc4f Mon Sep 17 00:00:00 2001 From: tobinio Date: Fri, 22 Aug 2025 18:04:26 +0200 Subject: [PATCH 05/14] chore: add changeset --- .changeset/puny-turtles-sniff.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/puny-turtles-sniff.md diff --git a/.changeset/puny-turtles-sniff.md b/.changeset/puny-turtles-sniff.md new file mode 100644 index 000000000000..1ff0568239a4 --- /dev/null +++ b/.changeset/puny-turtles-sniff.md @@ -0,0 +1,5 @@ +--- +"@biomejs/biome": patch +--- + +Fixed [#7205](https://github.com/biomejs/biome/issues/7205): The noDuplicateTestHook rule now correctly detects most describe blocks. From bfdd7a4300882b00c325d9e7d09dbe5b6867e5e8 Mon Sep 17 00:00:00 2001 From: tobinio Date: Fri, 22 Aug 2025 18:30:26 +0200 Subject: [PATCH 06/14] chore: catch more describe blocks --- .changeset/puny-turtles-sniff.md | 2 +- .../suspicious/no_duplicate_test_hooks.rs | 7 +++++-- .../suspicious/noDuplicateTestHooks/valid.js | 21 +++++++++++++++++++ .../noDuplicateTestHooks/valid.js.snap | 21 +++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/.changeset/puny-turtles-sniff.md b/.changeset/puny-turtles-sniff.md index 1ff0568239a4..b53bdd2a709f 100644 --- a/.changeset/puny-turtles-sniff.md +++ b/.changeset/puny-turtles-sniff.md @@ -2,4 +2,4 @@ "@biomejs/biome": patch --- -Fixed [#7205](https://github.com/biomejs/biome/issues/7205): The noDuplicateTestHook rule now correctly detects most describe blocks. +Fixed [#7205](https://github.com/biomejs/biome/issues/7205): The noDuplicateTestHooks rule now treats chained describe variants (e.g., describe.each/for/todo) as proper describe scopes, eliminating false positives. diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index 3dae4e112552..f360325c1f4f 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -165,9 +165,12 @@ impl DuplicateHooksVisitor { && node.is_test_call_expression() == Ok(true) { let text = callee.to_trimmed_text(); - let base = text.split('.').next().unwrap_or_default(); + let mut split = text.split('.'); - if base == "describe" { + let first = split.next().unwrap_or_default(); + let second = split.next().unwrap_or_default(); + + if matches!(first, "describe" | "fdescribe" | "xdescribe") || second == "describe" { return true; } } diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js index 8694ab071344..ca5addfaa507 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js +++ b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js @@ -84,6 +84,7 @@ describe("hello", () => { describe("something", () => { beforeEach(() => {}); + describe("something", () => { beforeEach(() => {}); }); @@ -104,3 +105,23 @@ describe("something", () => { beforeEach(() => {}); }); }); + +describe("something", () => { + beforeEach(() => {}); + + describe("something", () => { + beforeEach(() => {}); + }); + + test.describe.skip.each([])("something", () => { + beforeEach(() => {}); + }); + + xdescribe.for([])("something", () => { + beforeEach(() => {}); + }); + + fdescribe.todo("something", () => { + beforeEach(() => {}); + }); +}); diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap index 9b2fbce2c79c..bf24ac93eccf 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap +++ b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js.snap @@ -91,6 +91,7 @@ describe("hello", () => { describe("something", () => { beforeEach(() => {}); + describe("something", () => { beforeEach(() => {}); }); @@ -112,4 +113,24 @@ describe("something", () => { }); }); +describe("something", () => { + beforeEach(() => {}); + + describe("something", () => { + beforeEach(() => {}); + }); + + test.describe.skip.each([])("something", () => { + beforeEach(() => {}); + }); + + xdescribe.for([])("something", () => { + beforeEach(() => {}); + }); + + fdescribe.todo("something", () => { + beforeEach(() => {}); + }); +}); + ``` From 2aab1b64b19a013f53a90905d843198af8870673 Mon Sep 17 00:00:00 2001 From: tobinio Date: Sat, 23 Aug 2025 09:35:48 +0200 Subject: [PATCH 07/14] chore: update .snap files --- .../main_cases_linter_domains/does_enable_test_rules.snap | 5 +++-- ...recommended_rules_are_disabled_but_domain_is_enabled.snap | 5 +++-- .../enables_test_globals_via_dependencies.snap | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap index 32f6e566f518..bea4cd77fd9e 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap @@ -1,5 +1,6 @@ --- source: crates/biome_cli/tests/snap_test.rs +assertion_line: 432 expression: redactor(content) --- ## `biome.json` @@ -71,7 +72,7 @@ test1.js:1:10 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━ ```block test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 2 │ describe("foo", () => { 3 │ beforeEach(() => {}); @@ -80,7 +81,7 @@ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━ 5 │ test("bar", () => { 6 │ someFn(); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` diff --git a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap index f9d556e83b15..4cfa1032f2fa 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap @@ -1,5 +1,6 @@ --- source: crates/biome_cli/tests/snap_test.rs +assertion_line: 432 expression: redactor(content) --- ## `biome.json` @@ -77,7 +78,7 @@ test1.js:3:18 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━ ```block test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 2 │ describe("foo", () => { 3 │ beforeEach(() => {}); @@ -86,7 +87,7 @@ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━ 5 │ test("bar", () => { 6 │ someFn(); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` diff --git a/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap b/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap index 134d60e0f147..66e83cbfbe2b 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap @@ -1,5 +1,6 @@ --- source: crates/biome_cli/tests/snap_test.rs +assertion_line: 432 expression: redactor(content) --- ## `package.json` @@ -45,7 +46,7 @@ lint ━━━━━━━━━━━━━━━━━━━━━━━━━ ```block test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × Disallow duplicate setup and teardown hooks. + × Duplicate beforeEach hook found. 3 │ beforeEach(() => { 4 │ }); @@ -56,7 +57,7 @@ test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━ 7 │ test("bar", () => { 8 │ someFn(); - i Disallow beforeEach duplicacy inside the describe function. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` From 310b05a27c0c4761e07234e8574e355af1aa6a4a Mon Sep 17 00:00:00 2001 From: tobinio Date: Sat, 23 Aug 2025 10:39:31 +0200 Subject: [PATCH 08/14] chore: improve docs --- .changeset/puny-turtles-sniff.md | 20 ++++++++++++++- .../suspicious/no_duplicate_test_hooks.rs | 17 ++++++++++--- crates/biome_js_syntax/src/expr_ext.rs | 25 ++++++++++++++++--- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/.changeset/puny-turtles-sniff.md b/.changeset/puny-turtles-sniff.md index b53bdd2a709f..f77ea903a168 100644 --- a/.changeset/puny-turtles-sniff.md +++ b/.changeset/puny-turtles-sniff.md @@ -2,4 +2,22 @@ "@biomejs/biome": patch --- -Fixed [#7205](https://github.com/biomejs/biome/issues/7205): The noDuplicateTestHooks rule now treats chained describe variants (e.g., describe.each/for/todo) as proper describe scopes, eliminating false positives. +Fixed [#7205](https://github.com/biomejs/biome/issues/7205): The [`noDuplicateTestHooks`](https://biomejs.dev/linter/rules/no-duplicate-test-hooks/) rule now treats chained describe variants (e.g., describe.each/for/todo) as proper describe scopes, eliminating false positives. + +The following code will no longer be a false positive: + +```js +describe("foo", () => { + describe.for([])("baz", () => { + beforeEach(() => {}); + }); + + describe.todo("qux", () => { + beforeEach(() => {}); + }); + + describe.todo.each([])("baz", () => { + beforeEach(() => {}); + }); +}); +``` diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index f360325c1f4f..9532bd6bb90b 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -160,6 +160,15 @@ impl Visitor for DuplicateHooksVisitor { } impl DuplicateHooksVisitor { + /// Determines if a [call expression] is a `describe.` call by checking: + /// 1. The node must be a test call expression + /// e.g. `it.only`, `describe.skip`, `test` + /// 2. The test call must be a `describe.` call + /// first section = `describe` | `fdescribe` | `xdescribe` e.g. `describe.only`, `describe.skip` + /// or + /// second section = `describe` e.g. `test.describe` + /// + /// [call expression]: crate::JsCallExpression fn is_test_describe_call(node: &JsCallExpression) -> bool { if let Ok(callee) = node.callee() && node.is_test_call_expression() == Ok(true) @@ -167,10 +176,12 @@ impl DuplicateHooksVisitor { let text = callee.to_trimmed_text(); let mut split = text.split('.'); - let first = split.next().unwrap_or_default(); - let second = split.next().unwrap_or_default(); + let first = split.next(); + let second = split.next(); - if matches!(first, "describe" | "fdescribe" | "xdescribe") || second == "describe" { + if matches!(first, Some("describe" | "fdescribe" | "xdescribe")) + || second == Some("describe") + { return true; } } diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index c1181ca6d486..cfb7c6cfee25 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -945,7 +945,7 @@ impl AnyJsExpression { /// - `ftest` /// - `Deno.test` /// - /// Based on this [article] + /// Inspired by this [article] /// /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines pub fn contains_a_test_pattern(&self) -> bool { @@ -1008,9 +1008,26 @@ impl AnyJsExpression { } } - /// A valid pattern: - /// - Starts with a valid test pattern - /// - Ends with `.each` or `.for` + /// Checks if this expression contains a test.each pattern. + /// + /// A valid test.each pattern must: + /// - Start with a valid test pattern (see [`contains_a_test_pattern`]) + /// - End with `.each` or `.for` + /// + /// ## Examples + /// + /// Valid patterns: + /// ```javascript + /// test.each([...]) + /// describe.each([...]) + /// it.each([...]) + /// test.only.each([...]) + /// describe.skip.each([...]) + /// it.concurrent.each([...]) + /// ``` + /// + /// [`contains_a_test_pattern`]: crate::AnyJsExpression::contains_a_test_pattern + /// # pub fn contains_a_test_each_pattern(&self) -> bool { let Self::JsStaticMemberExpression(member_expression) = self else { return false; From 5217d66428f8bfc3411f37877879827f633127c9 Mon Sep 17 00:00:00 2001 From: tobinio Date: Sun, 24 Aug 2025 10:19:12 +0200 Subject: [PATCH 09/14] chore: further doc updates --- crates/biome_js_syntax/src/expr_ext.rs | 40 +++++++++++++++++--------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index cfb7c6cfee25..23ee18a9289e 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -690,6 +690,10 @@ impl JsTemplateExpression { self.is_test_each_pattern_callee() && self.is_test_each_pattern_elements() } + /// Checks if this expression contains a test.each pattern based on [`contains_a_test_each_pattern`]. + /// + /// [`contains_a_test_each_pattern`]: crate::AnyJsExpression::contains_a_test_each_pattern + /// pub fn is_test_each_pattern_callee(&self) -> bool { if let Some(tag) = self.tag() { tag.contains_a_test_each_pattern() @@ -931,11 +935,16 @@ impl AnyJsExpression { /// This function checks if a call expressions has one of the following members: /// - /// - `it[.(only|skip|todo|fails|failing|concurrent|sequential)]*` - /// - `test[.(only|skip|todo|fails|failing|concurrent|sequential)]*` - /// - `describe[.(only|skip|todo|shuffle|concurrent|sequential)]*` - /// - `test.describe[.(only|skip)]?` - /// - `test.describe.(parallel|serial)[.only]?` + /// - `it.(only|skip|todo|fails|failing|concurrent|sequential)` + /// - `it.(only|skip|todo|fails|failing|concurrent|sequential).(only|skip|todo|fails|failing|concurrent|sequential)` + /// - `test.(only|skip|todo|fails|failing|concurrent|sequential)` + /// - `test.(only|skip|todo|fails|failing|concurrent|sequential).(only|skip|todo|fails|failing|concurrent|sequential)` + /// - `describe.(only|skip|todo|shuffle|concurrent|sequential)` + /// - `describe.(only|skip|todo|shuffle|concurrent|sequential).(only|skip|todo|shuffle|concurrent|sequential)` + /// - `test.describe` + /// - `test.describe.(only|skip)` + /// - `test.describe.(parallel|serial)` + /// - `test.describe.(parallel|serial).only` /// - `skip` /// - `xit` /// - `xdescribe` @@ -945,7 +954,10 @@ impl AnyJsExpression { /// - `ftest` /// - `Deno.test` /// - /// Inspired by this [article] + /// Elements within parentheses `()` can be any of the listed options separated by `|`. + /// + /// Implementation first collects the tokens of callee names + /// and checks if they match any of the listed options via the tries data structure described in this [article]. /// /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines pub fn contains_a_test_pattern(&self) -> bool { @@ -1017,17 +1029,17 @@ impl AnyJsExpression { /// ## Examples /// /// Valid patterns: - /// ```javascript - /// test.each([...]) - /// describe.each([...]) - /// it.each([...]) - /// test.only.each([...]) - /// describe.skip.each([...]) - /// it.concurrent.each([...]) + /// ``` + /// test.each + /// describe.each + /// it.each + /// test.only.each + /// describe.skip.each + /// it.concurrent.each /// ``` /// /// [`contains_a_test_pattern`]: crate::AnyJsExpression::contains_a_test_pattern - /// # + /// pub fn contains_a_test_each_pattern(&self) -> bool { let Self::JsStaticMemberExpression(member_expression) = self else { return false; From 3a2b009b77812ebd184eae833a7895b5a31d37fa Mon Sep 17 00:00:00 2001 From: tobinio Date: Thu, 28 Aug 2025 18:42:12 +0200 Subject: [PATCH 10/14] fix: test declarations incorrectly formatted --- .../test_declarations.js.snap | 479 ------------------ crates/biome_js_syntax/src/expr_ext.rs | 48 +- 2 files changed, 29 insertions(+), 498 deletions(-) delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap deleted file mode 100644 index 038bb4423aff..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/test-declarations/test_declarations.js.snap +++ /dev/null @@ -1,479 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -assertion_line: 211 -info: js/test-declarations/test_declarations.js ---- -# Input - -```js -// Shouldn't break - -it("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); -}); - -it("does something really long and complicated so I have to write a very long name for the test", function() { - console.log("hello!"); -}); - -it("does something really long and complicated so I have to write a very long name for the test", function(done) { - console.log("hello!"); -}); - -it("does something really long and complicated so I have to write a very long name for the test", function myAssertions(done) { - console.log("hello!"); -}); - -it(`does something really long and complicated so I have to write a very long name for the test`, function() { - console.log("hello!"); -}); - -it(`{foo + bar} does something really long and complicated so I have to write a very long name for the test`, function() { - console.log("hello!"); -}); - -it(`handles - some - newlines - does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}) - -test("does something really long and complicated so I have to write a very long name for the test", (done) => { - console.log("hello!"); -}); - -test(`does something really long and complicated so I have to write a very long name for the test`, (done) => { - console.log("hello!"); -}); - -describe("does something really long and complicated so I have to write a very long name for the describe block", () => { - it("an example test", (done) => { - console.log("hello!"); - }); -}); - -describe(`does something really long and complicated so I have to write a very long name for the describe block`, () => { - it(`an example test`, (done) => { - console.log("hello!"); - }); -}); - -xdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -fdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -describe.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -fit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -xit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -it.only("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); -}); - -it.only(`does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}); - -it.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -ftest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -xtest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -skip("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.step("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.step(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe.only("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe.parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.parallel(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe.parallel.only("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); - -test.describe.parallel.only(`does something really long and complicated so I have to write a very long name for the testThis is a very`, () => {}); - -test.describe.serial("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.serial(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe.serial.only("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.serial.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -// Should break - -it.only("does something really long and complicated so I have to write a very long name for the test", 10, () => { - console.log("hello!"); -}); - -it.only.only("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); -}); - -it.only.only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { - console.log("hello!"); -}); - -xskip("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.only.parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.parallel.serial("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); - -test.serial("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.dummy.serial("does something really long and complicated so I have to write a very long name for the test", () => {}); - -// timeout - -it(`handles - some - newlines - does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}, 2500) - -it("does something quick", () => { - console.log("hello!") -}, 1000000000) - -it( - 'succeeds if the test finishes in time', - () => new Promise(resolve => setTimeout(resolve, 10)) -); - -it( - 'succeeds if the test finishes in time', - () => new Promise(resolve => setTimeout(resolve, 10)), - 250 -); - -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -85,9 +85,15 @@ - - skip("does something really long and complicated so I have to write a very long name for the test", () => {}); - --test.step("does something really long and complicated so I have to write a very long name for the test", () => {}); -+test.step( -+ "does something really long and complicated so I have to write a very long name for the test", -+ () => {}, -+); - --test.step(`does something really long and complicated so I have to write a very long name for the test`, () => {}); -+test.step( -+ `does something really long and complicated so I have to write a very long name for the test`, -+ () => {}, -+); - - test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); - -@@ -133,19 +139,15 @@ - }, - ); - --it.only.only( -- "does something really long and complicated so I have to write a very long name for the test", -- () => { -+it.only -+ .only("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); -- }, --); -+ }); - --it.only.only( -- "does something really long and complicated so I have to write a very long name for the test", -- (a, b, c) => { -+it.only -+ .only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { - console.log("hello!"); -- }, --); -+ }); - - xskip( - "does something really long and complicated so I have to write a very long name for the test", -``` - -# Output - -```js -// Shouldn't break - -it("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); -}); - -it("does something really long and complicated so I have to write a very long name for the test", function () { - console.log("hello!"); -}); - -it("does something really long and complicated so I have to write a very long name for the test", function (done) { - console.log("hello!"); -}); - -it("does something really long and complicated so I have to write a very long name for the test", function myAssertions(done) { - console.log("hello!"); -}); - -it(`does something really long and complicated so I have to write a very long name for the test`, function () { - console.log("hello!"); -}); - -it(`{foo + bar} does something really long and complicated so I have to write a very long name for the test`, function () { - console.log("hello!"); -}); - -it(`handles - some - newlines - does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}); - -test("does something really long and complicated so I have to write a very long name for the test", (done) => { - console.log("hello!"); -}); - -test(`does something really long and complicated so I have to write a very long name for the test`, (done) => { - console.log("hello!"); -}); - -describe("does something really long and complicated so I have to write a very long name for the describe block", () => { - it("an example test", (done) => { - console.log("hello!"); - }); -}); - -describe(`does something really long and complicated so I have to write a very long name for the describe block`, () => { - it(`an example test`, (done) => { - console.log("hello!"); - }); -}); - -xdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -fdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -describe.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -fit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -xit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -it.only("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); -}); - -it.only(`does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}); - -it.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -ftest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -xtest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - -skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -skip("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.step( - "does something really long and complicated so I have to write a very long name for the test", - () => {}, -); - -test.step( - `does something really long and complicated so I have to write a very long name for the test`, - () => {}, -); - -test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe - .only("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe - .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe - .parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe - .parallel(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe.parallel - .only("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); - -test.describe.parallel - .only(`does something really long and complicated so I have to write a very long name for the testThis is a very`, () => {}); - -test.describe - .serial("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe - .serial(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -test.describe.serial - .only("does something really long and complicated so I have to write a very long name for the test", () => {}); - -test.describe.serial - .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - -// Should break - -it.only( - "does something really long and complicated so I have to write a very long name for the test", - 10, - () => { - console.log("hello!"); - }, -); - -it.only - .only("does something really long and complicated so I have to write a very long name for the test", () => { - console.log("hello!"); - }); - -it.only - .only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { - console.log("hello!"); - }); - -xskip( - "does something really long and complicated so I have to write a very long name for the test", - () => {}, -); - -test.describe.only.parallel( - "does something really long and complicated so I have to write a very long name for the test", - () => {}, -); - -test.describe.parallel.serial( - "does something really long and complicated so I have to write a very long name for the testThis is a very", - () => {}, -); - -test.serial( - "does something really long and complicated so I have to write a very long name for the test", - () => {}, -); - -test.describe.dummy.serial( - "does something really long and complicated so I have to write a very long name for the test", - () => {}, -); - -// timeout - -it(`handles - some - newlines - does something really long and complicated so I have to write a very long name for the test`, () => { - console.log("hello!"); -}, 2500); - -it("does something quick", () => { - console.log("hello!"); -}, 1000000000); - -it("succeeds if the test finishes in time", () => - new Promise((resolve) => setTimeout(resolve, 10))); - -it( - "succeeds if the test finishes in time", - () => new Promise((resolve) => setTimeout(resolve, 10)), - 250, -); -``` - -# Lines exceeding max width of 80 characters -``` - 3: it("does something really long and complicated so I have to write a very long name for the test", () => { - 7: it("does something really long and complicated so I have to write a very long name for the test", function () { - 11: it("does something really long and complicated so I have to write a very long name for the test", function (done) { - 15: it("does something really long and complicated so I have to write a very long name for the test", function myAssertions(done) { - 19: it(`does something really long and complicated so I have to write a very long name for the test`, function () { - 23: it(`{foo + bar} does something really long and complicated so I have to write a very long name for the test`, function () { - 30: does something really long and complicated so I have to write a very long name for the test`, () => { - 34: test("does something really long and complicated so I have to write a very long name for the test", (done) => { - 38: test(`does something really long and complicated so I have to write a very long name for the test`, (done) => { - 42: describe("does something really long and complicated so I have to write a very long name for the describe block", () => { - 48: describe(`does something really long and complicated so I have to write a very long name for the describe block`, () => { - 54: xdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - 56: fdescribe("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - 58: describe.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 60: describe.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 62: fit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - 64: xit("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - 66: it.only("does something really long and complicated so I have to write a very long name for the test", () => { - 70: it.only(`does something really long and complicated so I have to write a very long name for the test`, () => { - 74: it.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 76: test.only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 78: test.skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 80: ftest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - 82: xtest("does something really long and complicated so I have to write a very long name for the describe block", () => {}); - 84: skip(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 86: skip("does something really long and complicated so I have to write a very long name for the test", () => {}); - 89: "does something really long and complicated so I have to write a very long name for the test", - 94: `does something really long and complicated so I have to write a very long name for the test`, - 98: test.describe("does something really long and complicated so I have to write a very long name for the test", () => {}); - 100: test.describe(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 103: .only("does something really long and complicated so I have to write a very long name for the test", () => {}); - 106: .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 109: .parallel("does something really long and complicated so I have to write a very long name for the test", () => {}); - 112: .parallel(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 115: .only("does something really long and complicated so I have to write a very long name for the testThis is a very", () => {}); - 118: .only(`does something really long and complicated so I have to write a very long name for the testThis is a very`, () => {}); - 121: .serial("does something really long and complicated so I have to write a very long name for the test", () => {}); - 124: .serial(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 127: .only("does something really long and complicated so I have to write a very long name for the test", () => {}); - 130: .only(`does something really long and complicated so I have to write a very long name for the test`, () => {}); - 135: "does something really long and complicated so I have to write a very long name for the test", - 143: .only("does something really long and complicated so I have to write a very long name for the test", () => { - 148: .only("does something really long and complicated so I have to write a very long name for the test", (a, b, c) => { - 153: "does something really long and complicated so I have to write a very long name for the test", - 158: "does something really long and complicated so I have to write a very long name for the test", - 163: "does something really long and complicated so I have to write a very long name for the testThis is a very", - 168: "does something really long and complicated so I have to write a very long name for the test", - 173: "does something really long and complicated so I have to write a very long name for the test", - 182: does something really long and complicated so I have to write a very long name for the test`, () => { -``` diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index 23ee18a9289e..bfa0c021cca5 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -19,6 +19,7 @@ use biome_rowan::{ TextSize, TokenText, declare_node_union, }; use core::iter; +use std::collections::HashSet; const GLOBAL_THIS: &str = "globalThis"; const UNDEFINED: &str = "undefined"; @@ -962,6 +963,14 @@ impl AnyJsExpression { /// [article]: https://craftinginterpreters.com/scanning-on-demand.html#tries-and-state-machines pub fn contains_a_test_pattern(&self) -> bool { let members = CalleeNamesIterator::new(self.clone()).collect::>(); + + let mut set = HashSet::new(); + let has_duplicates = members.iter().any(|x| !set.insert(x)); + + if has_duplicates { + return false; + } + let mut rev = members.iter().rev(); let first = rev.next().map(|t| t.text()); @@ -985,6 +994,7 @@ impl AnyJsExpression { }, Some("it" | "test") => match second { None => true, + Some("step") => third.is_none(), Some( "concurrent" | "sequential" | "only" | "skip" | "todo" | "fails" | "failing", ) => matches!( @@ -1028,15 +1038,12 @@ impl AnyJsExpression { /// /// ## Examples /// - /// Valid patterns: - /// ``` - /// test.each - /// describe.each - /// it.each - /// test.only.each - /// describe.skip.each - /// it.concurrent.each - /// ``` + /// - `test.each` + /// - `describe.each` + /// - `it.each` + /// - `test.only.each` + /// - `describe.skip.each` + /// - `it.concurrent.each` /// /// [`contains_a_test_pattern`]: crate::AnyJsExpression::contains_a_test_pattern /// @@ -2250,10 +2257,6 @@ mod test { fn matches_static_member_expression_deep() { let call_expression = extract_call_expression("test.describe.parallel.only();"); assert!(call_expression.callee().unwrap().contains_a_test_pattern()); - - let call_expression = - extract_call_expression("test.skip.sequential.only.skip.sequential.only();"); - assert!(call_expression.callee().unwrap().contains_a_test_pattern()); } #[test] @@ -2262,11 +2265,20 @@ mod test { assert!(!call_expression.callee().unwrap().contains_a_test_pattern()); } + #[test] + fn doesnt_test_call_expression_with_duplicates() { + let call_expression = extract_call_expression("test.only.only.only();"); + assert!(!call_expression.callee().unwrap().contains_a_test_pattern()); + } + #[test] fn matches_test_call_expression() { let call_expression = extract_call_expression("test.only(name, () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); + let call_expression = extract_call_expression("test.step(name, () => {});"); + assert_eq!(call_expression.is_test_call_expression(), Ok(true)); + let call_expression = extract_call_expression("test.only(Test.name, () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); @@ -2287,14 +2299,12 @@ mod test { let call_expression = extract_call_expression("describe.each([])(name, () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); - let call_expression = extract_call_expression( - "test.skip.sequential.only.skip.sequential.only(Test.name, () => {});", - ); + let call_expression = + extract_call_expression("test.skip.sequential.only.todo(Test.name, () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); - let call_expression = extract_call_expression( - "test.skip.sequential.only.todo.sequential.only.each([])(name, () => {});", - ); + let call_expression = + extract_call_expression("test.skip.sequential.only.todo.each([])(name, () => {});"); assert_eq!(call_expression.is_test_call_expression(), Ok(true)); } From 5c471504ada2134391ba6c5656d5a4074ce10d07 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 11 Nov 2025 16:23:03 +0000 Subject: [PATCH 11/14] fix snapshot --- .../noDuplicateTestHooks/invalid.js.snap | 97 +++++++++---------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap index 9d845bf7bc5e..057a228bba99 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap +++ b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap @@ -1,6 +1,5 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs -assertion_line: 152 expression: invalid.js --- # Input @@ -181,7 +180,7 @@ describe("foo", () => { invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 2 │ beforeEach(() => { 3 │ }); > 4 │ beforeEach(() => { @@ -190,9 +189,9 @@ invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 6 │ test("bar", () => { 7 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -200,7 +199,7 @@ invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeAll hook found. - + 14 │ beforeAll(() => { 15 │ }); > 16 │ beforeAll(() => { @@ -209,9 +208,9 @@ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 18 │ test("bar", () => { 19 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -219,7 +218,7 @@ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate afterEach hook found. - + 24 │ afterEach(() => { 25 │ }); > 26 │ afterEach(() => { @@ -228,9 +227,9 @@ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 28 │ test("bar", () => { 29 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -238,7 +237,7 @@ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate afterAll hook found. - + 34 │ afterAll(() => { 35 │ }); > 36 │ afterAll(() => { @@ -247,9 +246,9 @@ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 38 │ test("bar", () => { 39 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -257,7 +256,7 @@ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 44 │ beforeEach(() => { 45 │ }); > 46 │ beforeEach(() => { @@ -266,9 +265,9 @@ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 48 │ beforeEach(() => { 49 │ }); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -276,7 +275,7 @@ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 46 │ beforeEach(() => { 47 │ }); > 48 │ beforeEach(() => { @@ -285,9 +284,9 @@ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 50 │ test("bar", () => { 51 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -295,7 +294,7 @@ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate afterAll hook found. - + 56 │ afterAll(() => { 57 │ }); > 58 │ afterAll(() => { @@ -304,9 +303,9 @@ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 60 │ beforeAll(() => { 61 │ }); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -314,7 +313,7 @@ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeAll hook found. - + 60 │ beforeAll(() => { 61 │ }); > 62 │ beforeAll(() => { @@ -323,9 +322,9 @@ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 64 │ test("bar", () => { 65 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -333,7 +332,7 @@ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 70 │ beforeEach(() => { 71 │ }); > 72 │ beforeEach(() => { @@ -342,9 +341,9 @@ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 74 │ beforeAll(() => { 75 │ }); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -352,7 +351,7 @@ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 88 │ beforeEach(() => { 89 │ }); > 90 │ beforeEach(() => { @@ -361,9 +360,9 @@ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ │ ^^ 92 │ test("inner bar", () => { 93 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -371,18 +370,18 @@ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━ invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 99 │ beforeEach(() => { 100 │ }); > 101 │ beforeEach(() => { │ ^^^^^^^^^^^^^^^^^^ > 102 │ }); │ ^^ - 103 │ + 103 │ 104 │ it("is not fine", () => { - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -390,18 +389,18 @@ invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 118 │ beforeEach(() => { 119 │ }); > 120 │ beforeEach(() => { │ ^^^^^^^^^^^^^^^^^^ > 121 │ }); │ ^^ - 122 │ + 122 │ 123 │ it("is not fine", () => { - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -409,18 +408,18 @@ invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 139 │ beforeEach(() => { 140 │ }); > 141 │ beforeEach(() => { │ ^^^^^^^^^^^^^^^^^^ > 142 │ }); │ ^^ - 143 │ + 143 │ 144 │ it("is not fine", () => { - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -428,7 +427,7 @@ invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate before hook found. - + 151 │ before(() => { 152 │ }); > 153 │ before(() => { @@ -437,9 +436,9 @@ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ │ ^^ 155 │ test("bar", () => { 156 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` @@ -447,7 +446,7 @@ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ invalid.js:163:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate after hook found. - + 161 │ after(() => { 162 │ }); > 163 │ after(() => { @@ -456,8 +455,8 @@ invalid.js:163:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━ │ ^^ 165 │ test("bar", () => { 166 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` From 0a86cf3c8b32af8bf9f2b572d64338398805ef38 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 11 Nov 2025 16:38:33 +0000 Subject: [PATCH 12/14] update snapshots --- .../does_enable_test_rules.snap | 25 +++++++++---------- ...es_are_disabled_but_domain_is_enabled.snap | 25 +++++++++---------- .../should_enable_domain_via_cli.snap | 4 +-- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap index 493bb0b81b9e..204b4be4bf59 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules.snap @@ -1,6 +1,5 @@ --- source: crates/biome_cli/tests/snap_test.rs -assertion_line: 432 expression: redactor(content) --- ## `biome.json` @@ -33,7 +32,7 @@ describe("foo", () => { someFn(); }); }); - + ``` # Termination Message @@ -42,7 +41,7 @@ describe("foo", () => { lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Some errors were emitted while running checks. - + ``` @@ -53,19 +52,19 @@ lint ━━━━━━━━━━━━━━━━━━━━━━━━━ test1.js:1:10 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Don't focus the test. - + > 1 │ describe.only("bar", () => {}); │ ^^^^ - 2 │ - + 2 │ + i The 'only' method is often used for debugging or during implementation. - + i Consider removing 'only' to ensure all tests are executed. - + i Unsafe fix: Remove focus from test. - + 1 │ describe.only("bar",·()·=>·{}); - │ ----- + │ ----- ``` @@ -73,16 +72,16 @@ test1.js:1:10 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 2 │ describe("foo", () => { 3 │ beforeEach(() => {}); > 4 │ beforeEach(() => {}); │ ^^^^^^^^^^^^^^^^^^^^ 5 │ test("bar", () => { 6 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` diff --git a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap index 720a4c542fd1..8ed33536a2d4 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/does_enable_test_rules_when_recommended_rules_are_disabled_but_domain_is_enabled.snap @@ -1,6 +1,5 @@ --- source: crates/biome_cli/tests/snap_test.rs -assertion_line: 432 expression: redactor(content) --- ## `biome.json` @@ -38,7 +37,7 @@ describe("foo", () => { someFn(); }); }); - + ``` # Termination Message @@ -47,7 +46,7 @@ describe("foo", () => { lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Some errors were emitted while running checks. - + ``` @@ -58,20 +57,20 @@ lint ━━━━━━━━━━━━━━━━━━━━━━━━━ test1.js:3:18 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Don't focus the test. - + 2 │ debugger; > 3 │ describe.only("bar", () => {}); │ ^^^^ - 4 │ - + 4 │ + i The 'only' method is often used for debugging or during implementation. - + i Consider removing 'only' to ensure all tests are executed. - + i Unsafe fix: Remove focus from test. - + 3 │ ········describe.only("bar",·()·=>·{}); - │ ----- + │ ----- ``` @@ -79,16 +78,16 @@ test1.js:3:18 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 2 │ describe("foo", () => { 3 │ beforeEach(() => {}); > 4 │ beforeEach(() => {}); │ ^^^^^^^^^^^^^^^^^^^^ 5 │ test("bar", () => { 6 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ``` diff --git a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/should_enable_domain_via_cli.snap b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/should_enable_domain_via_cli.snap index 35ecee406dce..de5831cea3d7 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_linter_domains/should_enable_domain_via_cli.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_linter_domains/should_enable_domain_via_cli.snap @@ -74,7 +74,7 @@ test1.js:1:10 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━ ```block test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × The test hook beforeEach is used multiple times in the same test block. + × Duplicate beforeEach hook found. 2 │ describe("foo", () => { 3 │ beforeEach(() => {}); @@ -83,7 +83,7 @@ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━ 5 │ test("bar", () => { 6 │ someFn(); - i The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them. + i Remove this duplicate hook or consolidate the logic into a single hook. ``` From 318a8b552a4730337da1da6c489757ee5429c156 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 11 Nov 2025 16:39:38 +0000 Subject: [PATCH 13/14] linting --- .../src/lint/suspicious/no_duplicate_test_hooks.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs index 2801c7d2056c..86106826db80 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_duplicate_test_hooks.rs @@ -162,11 +162,10 @@ impl Visitor for DuplicateHooksVisitor { impl DuplicateHooksVisitor { /// Determines if a [call expression] is a `describe.` call by checking: /// 1. The node must be a test call expression - /// e.g. `it.only`, `describe.skip`, `test` + /// e.g. `it.only`, `describe.skip`, `test` /// 2. The test call must be a `describe.` call - /// first section = `describe` | `fdescribe` | `xdescribe` e.g. `describe.only`, `describe.skip` - /// or - /// second section = `describe` e.g. `test.describe` + /// first section = `describe` | `fdescribe` | `xdescribe` e.g. `describe.only`, `describe.skip` + /// or second section = `describe` e.g. `test.describe` /// /// [call expression]: crate::JsCallExpression fn is_test_describe_call(node: &JsCallExpression) -> bool { From 7a256216e405fd1fa2b8a2b0e62551642574cbef Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 11 Nov 2025 17:43:01 +0000 Subject: [PATCH 14/14] update snapshot --- .../enables_test_globals_via_dependencies.snap | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap b/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap index 79064de30f9e..3acd9454bcf6 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/enables_test_globals_via_dependencies.snap @@ -1,6 +1,5 @@ --- source: crates/biome_cli/tests/snap_test.rs -assertion_line: 432 expression: redactor(content) --- ## `package.json` @@ -27,7 +26,7 @@ describe("foo", () => { someFn(); }); }); - + ``` # Termination Message @@ -36,7 +35,7 @@ describe("foo", () => { lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Some errors were emitted while running checks. - + ``` @@ -47,7 +46,7 @@ lint ━━━━━━━━━━━━━━━━━━━━━━━━━ test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Duplicate beforeEach hook found. - + 3 │ beforeEach(() => { 4 │ }); > 5 │ beforeEach(() => { @@ -56,9 +55,9 @@ test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━ │ ^^ 7 │ test("bar", () => { 8 │ someFn(); - + i Remove this duplicate hook or consolidate the logic into a single hook. - + ```