fix(lint): useHookAtTopLevel should not report issues unrelated to the nested hook call (#7406)

Co-authored-by: dyc3 <[email protected]>
Co-authored-by: ematipico <[email protected]>

Author: 3 people

.changeset/poor-horses-smash.md

@@ -0,0 +1,16 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed an issue (#6393) where the [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/) rule reported excessive diagnostics for nested hook calls.
+
+The rule now reports only the offending top-level call site, not sub-hooks of composite hooks.
+
+```js
+// Before: reported twice (useFoo and useBar).
+function useFoo() { return useBar(); }
+function Component() {
+  if (cond) useFoo();
+}
+// After: reported once at the call to useFoo().
+```

crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs

@@ -422,6 +422,7 @@ impl Queryable for FunctionCall {
 #[derive(Debug)]
 pub struct CallPath {
     call: JsCallExpression,
+    is_enclosed_in_component_or_hook: bool,
     path: Vec<TextRange>,
 }
 
@@ -449,10 +450,16 @@ impl Rule for UseHookAtTopLevel {
         let root = CallPath {
             call: call.clone(),
             path: vec![],
+            is_enclosed_in_component_or_hook: false,
         };
         let mut calls = vec![root];
 
-        while let Some(CallPath { call, mut path }) = calls.pop() {
+        while let Some(CallPath {
+            call,
+            mut path,
+            is_enclosed_in_component_or_hook,
+        }) = calls.pop()
+        {
             let range = call.syntax().text_range_with_trivia();
 
             if path.contains(&range) {
@@ -485,17 +492,27 @@ impl Rule for UseHookAtTopLevel {
                     });
                 }
 
+                let enclosed = is_enclosed_in_component_or_hook
+                    || enclosing_function.is_react_component_or_hook();
+
                 if let AnyJsFunctionOrMethod::AnyJsFunction(function) = enclosing_function
                     && let Some(calls_iter) = function.all_calls(model)
                 {
                     for call in calls_iter {
                         calls.push(CallPath {
                             call: call.tree(),
                             path: path.clone(),
+                            is_enclosed_in_component_or_hook: enclosed,
                         });
                     }
                 }
             } else {
+                // Avoid duplicate diagnostics if this path already passed through
+                // a component/hook. We still keep previously enqueued paths to
+                // allow recursion detection elsewhere.
+                if is_enclosed_in_component_or_hook {
+                    continue;
+                }
                 return Some(Suggestion {
                     hook_name_range: get_hook_name_range()?,
                     path,

crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js

@@ -68,13 +68,13 @@ export default function Component5() {
     }
 };
 
-const Component6 = () => {
+const useHook6 = () => {
     useEffect();
 };
 
 const Component7 = () => {
     if (a == 1) {
-        Component6();
+        useHook6();
     }
 };
 
@@ -189,4 +189,4 @@ function useRecursiveHookA() {
 
 function useRecursiveHookB() {
     useRecursiveHookA();
-}
+}

crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/biome_js_analyze/tests/spec_tests.rs
+assertion_line: 152
 expression: invalid.js
 ---
 # Input
@@ -74,13 +75,13 @@ export default function Component5() {
     }
 };
 
-const Component6 = () => {
+const useHook6 = () => {
     useEffect();
 };
 
 const Component7 = () => {
     if (a == 1) {
-        Component6();
+        useHook6();
     }
 };
 
@@ -196,6 +197,7 @@ function useRecursiveHookA() {
 function useRecursiveHookB() {
     useRecursiveHookA();
 }
+
 ```
 
 # Diagnostics
@@ -463,23 +465,14 @@ invalid.js:67:9 lint/correctness/useHookAtTopLevel ━━━━━━━━━
 ```
 
 ```
-invalid.js:72:5 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+invalid.js:77:9 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  × This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.
-  
-    71 │ const Component6 = () => {
-  > 72 │     useEffect();
-       │     ^^^^^^^^^
-    73 │ };
-    74 │ 
-  
-  i This is the call path until the hook.
+  × This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
   
     75 │ const Component7 = () => {
-  > 76 │     if (a == 1) {
-       │                  
-  > 77 │         Component6();
-       │         ^^^^^^^^^^^^
+    76 │     if (a == 1) {
+  > 77 │         useHook6();
+       │         ^^^^^^^^
     78 │     }
     79 │ };
   
@@ -881,6 +874,7 @@ invalid.js:187:5 lint/correctness/useHookAtTopLevel ━━━━━━━━━
   > 191 │     useRecursiveHookA();
         │     ^^^^^^^^^^^^^^^^^^^
     192 │ }
+    193 │ 
   
   i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
   
@@ -900,6 +894,7 @@ invalid.js:191:5 lint/correctness/useHookAtTopLevel ━━━━━━━━━
   > 191 │     useRecursiveHookA();
         │     ^^^^^^^^^^^^^^^^^
     192 │ }
+    193 │ 
   
   i This is the call path until the hook.
   

crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.ts

@@ -1,9 +1,9 @@
-const Component1 = () => {
+const useHook = () => {
   useEffect() as [];
 };
 
 const Component2 = () => {
   if (a == 1) {
-      Component1();
+      useHook();
   }
 };

crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.ts.snap

@@ -1,41 +1,32 @@
 ---
 source: crates/biome_js_analyze/tests/spec_tests.rs
+assertion_line: 152
 expression: invalid.ts
-snapshot_kind: text
 ---
 # Input
 ```ts
-const Component1 = () => {
+const useHook = () => {
   useEffect() as [];
 };
 
 const Component2 = () => {
   if (a == 1) {
-      Component1();
+      useHook();
   }
 };
 
 ```
 
 # Diagnostics
 ```
-invalid.ts:2:3 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+invalid.ts:7:7 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  × This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.
-  
-    1 │ const Component1 = () => {
-  > 2 │   useEffect() as [];
-      │   ^^^^^^^^^
-    3 │ };
-    4 │ 
-  
-  i This is the call path until the hook.
+  × This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
   
     5 │ const Component2 = () => {
-  > 6 │   if (a == 1) {
-      │                
-  > 7 │       Component1();
-      │       ^^^^^^^^^^^^
+    6 │   if (a == 1) {
+  > 7 │       useHook();
+      │       ^^^^^^^
     8 │   }
     9 │ };
   

crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalidCompositeHook.js

@@ -0,0 +1,11 @@
+function useFoo() {
+    return useBar();
+}
+
+function Component() {
+    if (condition) {
+        // This call should be reported just once.
+        // See https://github.com/biomejs/biome/issues/6393
+        return useFoo();
+    }
+}

crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalidCompositeHook.js.snap

@@ -0,0 +1,40 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+assertion_line: 152
+expression: invalidCompositeHook.js
+---
+# Input
+```js
+function useFoo() {
+    return useBar();
+}
+
+function Component() {
+    if (condition) {
+        // This call should be reported just once.
+        // See https://github.com/biomejs/biome/issues/6393
+        return useFoo();
+    }
+}
+
+```
+
+# Diagnostics
+```
+invalidCompositeHook.js:9:16 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
+  
+     7 │         // This call should be reported just once.
+     8 │         // See https://github.com/biomejs/biome/issues/6393
+   > 9 │         return useFoo();
+       │                ^^^^^^
+    10 │     }
+    11 │ }
+  
+  i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
+  
+  i See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
+  
+
+```