diff --git a/.changeset/puny-turtles-sniff.md b/.changeset/puny-turtles-sniff.md
new file mode 100644
index 000000000000..f77ea903a168
--- /dev/null
+++ b/.changeset/puny-turtles-sniff.md
@@ -0,0 +1,23 @@
+---
+"@biomejs/biome": patch
+---
+
+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_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 05b2ccf5d929..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
@@ -71,7 +71,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(() => {});
@@ -80,7 +80,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.
```
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 42b1629e029e..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
@@ -77,7 +77,7 @@ test1.js:3:18 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(() => {});
@@ -86,7 +86,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.
```
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.
```
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 9be66e424f6b..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
@@ -45,7 +45,7 @@ lint ━━━━━━━━━━━━━━━━━━━━━━━━━
```block
test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
3 │ beforeEach(() => {
4 │ });
@@ -56,7 +56,7 @@ test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━
7 │ test("bar", () => {
8 │ 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.
```
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 54a12a53d214..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
@@ -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 Self::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"
+ && Self::is_test_describe_call(&node)
{
self.stack.pop();
}
@@ -178,6 +159,36 @@ 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)
+ {
+ let text = callee.to_trimmed_text();
+ let mut split = text.split('.');
+
+ let first = split.next();
+ let second = split.next();
+
+ if matches!(first, Some("describe" | "fdescribe" | "xdescribe"))
+ || second == Some("describe")
+ {
+ return true;
+ }
+ }
+
+ false
+ }
+}
+
// Declare a query match struct type containing a JavaScript function node
pub struct DuplicateHooks(JsCallExpression);
@@ -229,10 +240,11 @@ impl Rule for NoDuplicateTestHooks {
rule_category!(),
ctx.query().range(),
markup! {
- "The test hook "{node_name.text_trimmed()}" is used multiple times in the same test block."
+ "Duplicate "{node_name.text_trimmed()}" hook found."
},
- ).note(markup! {
- "The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them."
+ )
+ .note(markup! {
+ "Remove this duplicate hook or consolidate the logic into a single hook."
}),
)
}
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_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/invalid.js.snap
index 466f2f77dae1..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
@@ -179,7 +179,7 @@ describe("foo", () => {
```
invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
2 │ beforeEach(() => {
3 │ });
@@ -190,7 +190,7 @@ invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
6 │ test("bar", () => {
7 │ 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.
```
@@ -198,7 +198,7 @@ invalid.js:4:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeAll is used multiple times in the same test block.
+ × Duplicate beforeAll hook found.
14 │ beforeAll(() => {
15 │ });
@@ -209,7 +209,7 @@ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
18 │ test("bar", () => {
19 │ 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.
```
@@ -217,7 +217,7 @@ invalid.js:16:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook afterEach is used multiple times in the same test block.
+ × Duplicate afterEach hook found.
24 │ afterEach(() => {
25 │ });
@@ -228,7 +228,7 @@ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
28 │ test("bar", () => {
29 │ 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.
```
@@ -236,7 +236,7 @@ invalid.js:26:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook afterAll is used multiple times in the same test block.
+ × Duplicate afterAll hook found.
34 │ afterAll(() => {
35 │ });
@@ -247,7 +247,7 @@ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
38 │ test("bar", () => {
39 │ 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.
```
@@ -255,7 +255,7 @@ invalid.js:36:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
44 │ beforeEach(() => {
45 │ });
@@ -266,7 +266,7 @@ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
48 │ beforeEach(() => {
49 │ });
- 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.
```
@@ -274,7 +274,7 @@ invalid.js:46:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
46 │ beforeEach(() => {
47 │ });
@@ -285,7 +285,7 @@ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
50 │ test("bar", () => {
51 │ 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.
```
@@ -293,7 +293,7 @@ invalid.js:48:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook afterAll is used multiple times in the same test block.
+ × Duplicate afterAll hook found.
56 │ afterAll(() => {
57 │ });
@@ -304,7 +304,7 @@ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
60 │ beforeAll(() => {
61 │ });
- 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.
```
@@ -312,7 +312,7 @@ invalid.js:58:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeAll is used multiple times in the same test block.
+ × Duplicate beforeAll hook found.
60 │ beforeAll(() => {
61 │ });
@@ -323,7 +323,7 @@ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
64 │ test("bar", () => {
65 │ 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.
```
@@ -331,7 +331,7 @@ invalid.js:62:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
70 │ beforeEach(() => {
71 │ });
@@ -342,7 +342,7 @@ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
74 │ beforeAll(() => {
75 │ });
- 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.
```
@@ -350,7 +350,7 @@ invalid.js:72:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
88 │ beforeEach(() => {
89 │ });
@@ -361,7 +361,7 @@ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
92 │ test("inner bar", () => {
93 │ 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.
```
@@ -369,7 +369,7 @@ invalid.js:90:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
```
invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
99 │ beforeEach(() => {
100 │ });
@@ -380,7 +380,7 @@ invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
103 │
104 │ it("is not fine", () => {
- 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.
```
@@ -388,7 +388,7 @@ invalid.js:101:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
```
invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
118 │ beforeEach(() => {
119 │ });
@@ -399,7 +399,7 @@ invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
122 │
123 │ it("is not fine", () => {
- 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.
```
@@ -407,7 +407,7 @@ invalid.js:120:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
```
invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook beforeEach is used multiple times in the same test block.
+ × Duplicate beforeEach hook found.
139 │ beforeEach(() => {
140 │ });
@@ -418,7 +418,7 @@ invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
143 │
144 │ it("is not fine", () => {
- 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.
```
@@ -426,7 +426,7 @@ invalid.js:141:4 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
```
invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook before is used multiple times in the same test block.
+ × Duplicate before hook found.
151 │ before(() => {
152 │ });
@@ -437,7 +437,7 @@ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
155 │ test("bar", () => {
156 │ 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.
```
@@ -445,7 +445,7 @@ invalid.js:153:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
```
invalid.js:163:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
- × The test hook after is used multiple times in the same test block.
+ × Duplicate after hook found.
161 │ after(() => {
162 │ });
@@ -456,7 +456,7 @@ invalid.js:163:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━
165 │ test("bar", () => {
166 │ 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.
```
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..ca5addfaa507 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,46 @@ 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(() => {});
+ });
+});
+
+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 a6406626f509..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
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
+assertion_line: 152
expression: valid.js
---
# Input
@@ -88,5 +89,48 @@ 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(() => {});
+ });
+});
+
+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_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs
index 61aa24ff32e1..5a4e236535d0 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";
@@ -669,111 +670,13 @@ 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]
+ /// 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
///
- /// [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
}
@@ -1011,22 +914,17 @@ 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`
+ ///
+ /// - `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`
- /// - `test.describe.parallel`
- /// - `test.describe.parallel.only`
- /// - `test.describe.serial`
- /// - `test.describe.serial.only`
+ /// - `test.describe.(only|skip)`
+ /// - `test.describe.(parallel|serial)`
+ /// - `test.describe.(parallel|serial).only`
/// - `skip`
/// - `xit`
/// - `xdescribe`
@@ -1036,21 +934,23 @@ impl AnyJsExpression {
/// - `ftest`
/// - `Deno.test`
///
- /// Based on 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) -> SyntaxResult {
- let mut members = CalleeNamesIterator::new(self.clone());
+ pub fn contains_a_test_pattern(&self) -> bool {
+ let members = CalleeNamesIterator::new(self.clone()).collect::>();
- let texts: [Option; 5] = [
- members.next(),
- members.next(),
- members.next(),
- members.next(),
- members.next(),
- ];
+ let mut set = HashSet::new();
+ let has_duplicates = members.iter().any(|x| !set.insert(x));
- let mut rev = texts.iter().rev().flatten();
+ if has_duplicates {
+ return false;
+ }
+
+ let mut rev = members.iter().rev();
let first = rev.next().map(|t| t.text());
let second = rev.next().map(|t| t.text());
@@ -1058,18 +958,39 @@ impl AnyJsExpression {
let fourth = rev.next().map(|t| t.text());
let fifth = rev.next().map(|t| t.text());
- Ok(match first {
- Some("it" | "describe" | "Deno") => match second {
+ match first {
+ Some("describe") => match second {
None => true,
- Some("only" | "skip" | "test") => third.is_none(),
+ Some("concurrent" | "sequential" | "only" | "skip" | "todo" | "shuffle") => {
+ matches!(
+ third,
+ None | Some(
+ "concurrent" | "sequential" | "only" | "skip" | "todo" | "shuffle",
+ )
+ )
+ }
_ => false,
},
- Some("test") => match second {
+ Some("it" | "test") => match second {
None => true,
- Some("only" | "skip" | "step") => third.is_none(),
+ Some("step") => third.is_none(),
+ Some(
+ "concurrent" | "sequential" | "only" | "skip" | "todo" | "fails" | "failing",
+ ) => matches!(
+ third,
+ None | Some(
+ "concurrent"
+ | "sequential"
+ | "only"
+ | "skip"
+ | "todo"
+ | "fails"
+ | "failing",
+ )
+ ),
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(),
@@ -1079,9 +1000,55 @@ impl AnyJsExpression {
},
_ => false,
},
+ Some("Deno") => match second {
+ Some("test") => third.is_none(),
+ _ => false,
+ },
Some("skip" | "xit" | "xdescribe" | "xtest" | "fit" | "fdescribe" | "ftest") => true,
_ => false,
- })
+ }
+ }
+
+ /// 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
+ ///
+ /// - `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;
+ };
+
+ if matches!(member_expression.object(), Ok(rest) if !rest.contains_a_test_pattern()) {
+ return false;
+ }
+
+ 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,
+ }
}
/// Checks whether the current function call is:
@@ -2045,7 +2012,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!(
@@ -2251,43 +2220,34 @@ 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]
+ 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]
@@ -2295,6 +2255,9 @@ mod test {
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));
@@ -2311,6 +2274,17 @@ 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));
+
+ 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.each([])(name, () => {});");
+ assert_eq!(call_expression.is_test_call_expression(), Ok(true));
}
#[test]