fix(js_analyzer): don't flag duplicate hooks in describe.each for lin…

…t/suspicious/noDuplicateTestHooks (#3806)
biomejs · Sep 10, 2024 · d7070f1 · d7070f1
1 parent 37cf1e0
commit d7070f1
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 35 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
@@ -110,42 +110,51 @@ impl Visitor for DuplicateHooksVisitor {
     ) {
         match event {
             WalkEvent::Enter(node) => {
+                let Some(node) = JsCallExpression::cast_ref(node) else {
+                    return;
+                };
+
                 // When the visitor enters a function node, push a new entry on the stack
-                if let Some(node) = JsCallExpression::cast_ref(node) {
-                    if let Ok(callee) = node.callee() {
-                        if callee.contains_a_test_pattern() == Ok(true) {
-                            if let Some(function_name) = callee.get_callee_object_name() {
-                                if function_name.text_trimmed() == "describe" {
-                                    self.stack.push(HooksContext::default());
-                                }
+                if let Ok(callee) = node.callee() {
+                    if callee.contains_a_test_pattern() == Ok(true) {
+                        if let Some(function_name) = callee.get_callee_object_name() {
+                            if function_name.text_trimmed() == "describe" {
+                                self.stack.push(HooksContext::default());
+                            }
+                        }
+                    }
+                    // describe.each has a different syntax
+                    else if let AnyJsExpression::JsCallExpression(call_expression) = callee {
+                        if let Ok(callee) = call_expression.callee() {
+                            if matches!(
+                                callee.text().as_str(),
+                                "describe.each" | "describe.only.each" | "fdescribe.each"
+                            ) {
+                                self.stack.push(HooksContext::default());
                             }
                         }
                     }
                 }
 
-                if let Some(node) = JsCallExpression::cast_ref(node) {
-                    if let Ok(AnyJsExpression::JsIdentifierExpression(identifier)) = node.callee() {
-                        identifier
-                            .name()
-                            .and_then(|name| name.value_token())
-                            .map_or((), |name| {
-                                if let Some(hooks_context) = self.stack.last_mut() {
-                                    match name.text_trimmed() {
-                                        "beforeEach" | "beforeAll" | "afterEach" | "afterAll"
-                                        | "after" | "before" => {
-                                            let counter = HooksContext::add(
-                                                hooks_context,
-                                                name.text_trimmed(),
-                                            );
-                                            if counter > 1 {
-                                                ctx.match_query(DuplicateHooks(node.clone()));
-                                            }
+                if let Ok(AnyJsExpression::JsIdentifierExpression(identifier)) = node.callee() {
+                    identifier
+                        .name()
+                        .and_then(|name| name.value_token())
+                        .map_or((), |name| {
+                            if let Some(hooks_context) = self.stack.last_mut() {
+                                match name.text_trimmed() {
+                                    "beforeEach" | "beforeAll" | "afterEach" | "afterAll"
+                                    | "after" | "before" => {
+                                        let counter =
+                                            HooksContext::add(hooks_context, name.text_trimmed());
+                                        if counter > 1 {
+                                            ctx.match_query(DuplicateHooks(node.clone()));
                                         }
-                                        _ => {}
-                                    };
+                                    }
+                                    _ => {}
                                 };
-                            })
-                    }
+                            };
+                        })
                 }
             }
             WalkEvent::Leave(node) => {

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
@@ -308,15 +308,16 @@ invalid.js:69:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━
 ```
 
 ```
-invalid.js:91:3 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+invalid.js:78:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Disallow duplicate setup and teardown hooks.
   
-    90 │ 	describe.each(["world"])("%s", () => {
-  > 91 │ 		beforeEach(() => {});
-       │ 		^^^^^^^^^^^^^^^^^^^^
-    92 │ 		beforeEach(() => {});
-    93 │ 
+    76 │ describe.each(["hello"])("%s", () => {
+    77 │ 	beforeEach(() => {});
+  > 78 │ 	beforeEach(() => {});
+       │ 	^^^^^^^^^^^^^^^^^^^^
+    79 │ 
+    80 │ 	it("is not fine", () => {});
   
   i Disallow beforeEach duplicacy inside the describe function.
   

diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js b/crates/biome_js_analyze/tests/specs/suspicious/noDuplicateTestHooks/valid.js
@@ -67,3 +67,17 @@ describe("something", () => {
 		it("is fine", () => {});
 	});
 });
+
+describe("hello", () => {
+	beforeEach(() => {});
+
+	describe.each(['hello'])(
+		"given an input %p",
+		() => {
+			beforeEach(() => {});
+
+			it("should be fine", () => {});
+		},
+	);
+});
+
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
@@ -74,6 +74,18 @@ describe("something", () => {
 	});
 });
 
-```
+describe("hello", () => {
+	beforeEach(() => {});
+
+	describe.each(['hello'])(
+		"given an input %p",
+		() => {
+			beforeEach(() => {});
+
+			it("should be fine", () => {});
+		},
+	);
+});
 
 
+```