fix(linter): react rules of hooks false positive

crates/oxc_linter/src/rules/react/rules_of_hooks.rs

@@ -2,14 +2,15 @@ use std::borrow::Cow;
 
 use oxc_ast::{
     AstKind,
-    ast::{ArrowFunctionExpression, Function},
+    ast::{ArrowFunctionExpression, Expression, Function},
 };
 use oxc_cfg::{
     ControlFlowGraph, EdgeType, ErrorEdgeKind, InstructionKind,
     graph::{algo, visit::Control},
 };
 use oxc_macros::declare_oxc_lint;
 use oxc_semantic::{AstNodes, NodeId};
+use oxc_span::Span;
 use oxc_syntax::operator::AssignmentOperator;
 
 use crate::{
@@ -124,13 +125,12 @@ impl Rule for RulesOfHooks {
         }
 
         let cfg = ctx.cfg();
+        let nodes = ctx.nodes();
 
         let span = call.span;
         let hook_name =
             call.callee_name().expect("We identify hooks using their names so it should be named.");
 
-        let nodes = ctx.nodes();
-
         let is_use = is_react_function_call(call, "use");
 
         let Some(parent_func) = parent_func(nodes, node) else {
@@ -249,7 +249,7 @@ impl Rule for RulesOfHooks {
             return ctx.diagnostic(diagnostics::loop_hook(span, hook_name));
         }
 
-        if has_conditional_path_accept_throw(cfg, parent_func, node) {
+        if has_conditional_path_accept_throw(cfg, nodes, parent_func, node) {
             #[expect(clippy::needless_return)]
             return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name));
         }
@@ -258,12 +258,21 @@ impl Rule for RulesOfHooks {
 
 fn has_conditional_path_accept_throw(
     cfg: &ControlFlowGraph,
+    nodes: &AstNodes<'_>,
     from: &AstNode<'_>,
     to: &AstNode<'_>,
 ) -> bool {
     let from_graph_id = from.cfg_id();
     let to_graph_id = to.cfg_id();
     let graph = cfg.graph();
+
+    // Check if the hook call is part of a condition expression
+    // If the hook is used in a condition (like `if (useHook())`) rather than being conditionally called,
+    // then it's valid and we should return false
+    if is_hook_in_condition_expression(nodes, to) {
+        return false;
+    }
+
     if graph
         .edges(to_graph_id)
         .any(|it| matches!(it.weight(), EdgeType::Error(ErrorEdgeKind::Explicit)))
@@ -327,6 +336,81 @@ fn has_conditional_path_accept_throw(
     })
 }
 
+// Helper function to check if a hook call is part of a condition expression
+fn is_hook_in_condition_expression(nodes: &AstNodes<'_>, node: &AstNode<'_>) -> bool {
+    let node_span = match node.kind() {
+        AstKind::CallExpression(call_expr) => call_expr.span,
+        _ => return false, // Not a call expression, so not a hook call
+    };
+
+    // Get the parent of the hook call
+    if let Some(parent_id) = nodes.parent_id(node.id()) {
+        let parent = nodes.get_node(parent_id);
+        match parent.kind() {
+            // Check if the hook is used directly in an if statement condition
+            AstKind::IfStatement(if_stmt) => {
+                // Get the span of the test expression
+                let test_span = match &if_stmt.test {
+                    Expression::CallExpression(call) => call.span,
+                    Expression::Identifier(ident) => ident.span,
+                    Expression::UnaryExpression(unary) => unary.span,
+                    Expression::LogicalExpression(logical) => logical.span,
+                    Expression::BinaryExpression(binary) => binary.span,
+                    _ => return false,
+                };
+                // Check if the hook call is within the test expression of the if statement
+                is_span_within(node_span, test_span)
+            }
+            // Check if the hook is used in a logical expression (&&, ||)
+            AstKind::LogicalExpression(logical_expr) => {
+                // We need to check if the hook is in the left side (condition part)
+                // of the logical expression, not the right side
+                if let Expression::CallExpression(call) = &logical_expr.left {
+                    // If the hook is in the left side, it's a valid condition
+                    is_span_within(node_span, call.span)
+                } else if let Expression::UnaryExpression(unary) = &logical_expr.left {
+                    // Handle cases like !useHook() && something
+                    if let Expression::CallExpression(call) = &unary.argument {
+                        is_span_within(node_span, call.span)
+                    } else {
+                        false
+                    }
+                } else {
+                    false
+                }
+            }
+            // Check if the hook is used in a conditional expression (ternary)
+            AstKind::ConditionalExpression(cond_expr) => {
+                // Get the span of the test expression
+                let test_span = match &cond_expr.test {
+                    Expression::CallExpression(call) => call.span,
+                    Expression::Identifier(ident) => ident.span,
+                    Expression::UnaryExpression(unary) => unary.span,
+                    Expression::LogicalExpression(logical) => logical.span,
+                    Expression::BinaryExpression(binary) => binary.span,
+                    _ => return false,
+                };
+                // Check if the hook call is within the test part of the conditional expression
+                is_span_within(node_span, test_span)
+            }
+            // Check if the hook is used in a unary expression (like !)
+            AstKind::UnaryExpression(unary_expr) => {
+                // Check if the hook call is within the unary expression
+                is_span_within(node_span, unary_expr.span)
+            }
+            // For other cases, check if the parent is part of a condition
+            _ => is_hook_in_condition_expression(nodes, parent),
+        }
+    } else {
+        false
+    }
+}
+
+// Helper function to check if one span is within another
+fn is_span_within(inner: Span, outer: Span) -> bool {
+    inner.start >= outer.start && inner.end <= outer.end
+}
+
 fn parent_func<'a>(nodes: &'a AstNodes<'a>, node: &AstNode) -> Option<&'a AstNode<'a>> {
     nodes
         .ancestor_ids(node.id())
@@ -444,6 +528,62 @@ fn test() {
               useHook();
             }
         ",
+
+
+        // Valid because hooks can be used in condition expressions beginning with a ternary condition
+        "
+            function ComponentWithConditionalHook() {
+               if (useHook() ? good() : bad()) {
+                    check();
+               }
+            }
+        ",
+
+        // Valid because hooks can be used in condition expressions
+        "
+            function Component() {
+              if (!useHasPermission()) {
+                return null;
+              }
+              return <Content />;
+            }
+        ",
+        // Valid because hooks can be used in ternary condition expressions
+        "
+            function Component() {
+              return useHasPermission() ? <Content /> : null;
+            }
+        ",
+        // Valid because hooks can be used in logical expressions (left side)
+        "
+            function Component() {
+              return useHasPermission() && <Content />;
+            }
+        ",
+        // Valid because hooks can be used with negation in condition expressions
+        "
+            function Component() {
+              if (!useHasPermission()) {
+                return null;
+              }
+              return <Content />;
+            }
+        ",
+        // Valid because hooks can be used in complex condition expressions
+        "
+            function Component() {
+              if (useHasPermission() && isAdmin()) {
+                return <AdminContent />;
+              }
+              return <Content />;
+            }
+        ",
+        // Valid because hooks can be used in nested condition expressions
+        "
+            function Component() {
+              return (useHasPermission() && isAdmin()) ? <AdminContent /> : <Content />;
+            }
+        ",
         // Valid because components can use hooks.
         "
             function createComponentWithHook() {
@@ -968,6 +1108,28 @@ fn test() {
                }
              }
         ",
+        // Invalid because hooks are used in the right side of logical expressions
+        "
+            function useHook() {
+                a && useHook1();
+                b && useHook2();
+            }
+        ",
+        // Invalid because hooks are used conditionally after a condition
+        "
+            function Component() {
+                if (condition) {
+                    // This is invalid because the hook is called conditionally
+                    useHook();
+                }
+            }
+        ",
+        // Invalid because hooks are used in the right side of ternary expressions
+        "
+            function Component() {
+                condition ? useHook() : null;
+            }
+        ",
         // Invalid because hooks can only be called inside of a component.
         // errors: [
         //     topLevelError('Hook.useState'),

crates/oxc_linter/src/snapshots/react_rules_of_hooks.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/oxc_linter/src/tester.rs
+assertion_line: 362
 ---
   ⚠ eslint-plugin-react-hooks(rules-of-hooks): React Hook "useConditionalHook" is called conditionally. React Hooks must be called in the exact same order in every component render.
    ╭─[rules_of_hooks.tsx:4:18]
@@ -9,6 +10,38 @@ source: crates/oxc_linter/src/tester.rs
  5 │                }
    ╰────
 
+  ⚠ eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook1" is called conditionally. React Hooks must be called in the exact same order in every component render.
+   ╭─[rules_of_hooks.tsx:3:22]
+ 2 │             function useHook() {
+ 3 │                 a && useHook1();
+   ·                      ──────────
+ 4 │                 b && useHook2();
+   ╰────
+
+  ⚠ eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook2" is called conditionally. React Hooks must be called in the exact same order in every component render.
+   ╭─[rules_of_hooks.tsx:4:22]
+ 3 │                 a && useHook1();
+ 4 │                 b && useHook2();
+   ·                      ──────────
+ 5 │             }
+   ╰────
+
+  ⚠ eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook" is called conditionally. React Hooks must be called in the exact same order in every component render.
+   ╭─[rules_of_hooks.tsx:5:21]
+ 4 │                     // This is invalid because the hook is called conditionally
+ 5 │                     useHook();
+   ·                     ─────────
+ 6 │                 }
+   ╰────
+
+  ⚠ eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook" is called conditionally. React Hooks must be called in the exact same order in every component render.
+   ╭─[rules_of_hooks.tsx:3:29]
+ 2 │             function Component() {
+ 3 │                 condition ? useHook() : null;
+   ·                             ─────────
+ 4 │             }
+   ╰────
+
   ⚠ eslint-plugin-react-hooks(rules-of-hooks): React Hook "useState" cannot be called at the top level. React Hooks must be called in a React function component or a custom React Hook function.
    ╭─[rules_of_hooks.tsx:2:13]
  1 │