fix(rome_js_analyze): useExhaustiveDependencies support function syntax, or React.use* hooks and more.

CHANGELOG.md

@@ -36,6 +36,10 @@
 ### Linter
 
 - Fix a crash in the `NoParameterAssign` rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323)
+- Fix useExhaustiveDependencies rule in the following cases [#4330](https://github.com/rome/tools/issues/4330)
+  - when the first argument of hooks is a named function
+  - inside an export default function
+  - for React.use* hooks
 
 #### Other changes
 

crates/rome_js_analyze/src/react/hooks.rs

@@ -1,9 +1,10 @@
 use std::collections::{HashMap, HashSet};
 
-use rome_js_semantic::{Capture, ClosureExtensions, SemanticModel};
+use rome_js_semantic::{Capture, Closure, ClosureExtensions, SemanticModel};
 use rome_js_syntax::{
     binding_ext::AnyJsIdentifierBinding, AnyJsExpression, JsArrayBindingPattern,
-    JsArrayBindingPatternElementList, JsCallExpression, JsVariableDeclarator, TextRange,
+    JsArrayBindingPatternElementList, JsArrowFunctionExpression, JsCallExpression,
+    JsFunctionExpression, JsVariableDeclarator, TextRange,
 };
 use rome_rowan::AstNode;
 use serde::{Deserialize, Serialize};
@@ -15,15 +16,45 @@ pub(crate) struct ReactCallWithDependencyResult {
     pub(crate) dependencies_node: Option<AnyJsExpression>,
 }
 
+pub enum AnyJsFunctionExpression {
+    JsArrowFunctionExpression(JsArrowFunctionExpression),
+    JsFunctionExpression(JsFunctionExpression),
+}
+
+impl AnyJsFunctionExpression {
+    fn closure(&self, model: &SemanticModel) -> Closure {
+        match self {
+            Self::JsArrowFunctionExpression(arrow_function) => arrow_function.closure(model),
+            Self::JsFunctionExpression(function) => function.closure(model),
+        }
+    }
+}
+
+impl TryFrom<AnyJsExpression> for AnyJsFunctionExpression {
+    type Error = ();
+
+    fn try_from(expression: AnyJsExpression) -> Result<Self, Self::Error> {
+        match expression {
+            AnyJsExpression::JsArrowFunctionExpression(arrow_function) => {
+                Ok(Self::JsArrowFunctionExpression(arrow_function))
+            }
+            AnyJsExpression::JsFunctionExpression(function) => {
+                Ok(Self::JsFunctionExpression(function))
+            }
+            _ => Err(()),
+        }
+    }
+}
+
 impl ReactCallWithDependencyResult {
     /// Returns all [Reference] captured by the closure argument of the React hook.
     /// See [react_hook_with_dependency].
     pub fn all_captures(&self, model: &SemanticModel) -> impl Iterator<Item = Capture> {
         self.closure_node
             .as_ref()
-            .and_then(|node| node.as_js_arrow_function_expression())
-            .map(|arrow_function| {
-                let closure = arrow_function.closure(model);
+            .and_then(|node| AnyJsFunctionExpression::try_from(node.clone()).ok())
+            .map(|function_expression| {
+                let closure = function_expression.closure(model);
                 let range = *closure.closure_range();
                 closure
                     .descendents()
@@ -96,14 +127,15 @@ pub(crate) fn react_hook_with_dependency(
     call: &JsCallExpression,
     hooks: &HashMap<String, ReactHookConfiguration>,
 ) -> Option<ReactCallWithDependencyResult> {
-    let name = call
-        .callee()
-        .ok()?
-        .as_js_identifier_expression()?
-        .name()
-        .ok()?
-        .value_token()
-        .ok()?;
+    let name = match call.callee().ok()? {
+        AnyJsExpression::JsIdentifierExpression(identifier) => {
+            Some(identifier.name().ok()?.value_token().ok()?)
+        }
+        AnyJsExpression::JsStaticMemberExpression(member) => {
+            Some(member.member().ok()?.as_js_name()?.value_token().ok()?)
+        }
+        _ => None,
+    }?;
     let function_name_range = name.text_trimmed_range();
     let name = name.text_trimmed();
 

crates/rome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs

@@ -489,6 +489,7 @@ fn function_of_hook_call(call: &JsCallExpression) -> Option<JsSyntaxNode> {
         matches!(
             x.kind(),
             JsSyntaxKind::JS_FUNCTION_DECLARATION
+                | JsSyntaxKind::JS_FUNCTION_EXPORT_DEFAULT_DECLARATION
                 | JsSyntaxKind::JS_FUNCTION_EXPRESSION
                 | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION
         )

crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js

@@ -4,7 +4,7 @@ function MyComponent1() {
     useEffect(() => {
       console.log(a, b);
     });
-  }
+}
 
 // interaction with other react hooks
 
@@ -82,3 +82,41 @@ const MyComponent8 = React.memo(({ a }) => {
       console.log(a);
   });
 });
+
+// exported functions
+export function MyComponent9() {
+  let a = 1;
+  useEffect(() => {
+      console.log(a);
+  });
+}
+
+export default function MyComponent10() {
+  let a = 1;
+  useEffect(() => {
+      console.log(a);
+  });
+}
+
+// named function
+function MyComponent11() {
+  let a = 1;
+  useEffect(function inner() {
+      console.log(a);
+  });
+}
+
+function MyComponent12() {
+  let a = 1;
+  useEffect(async function inner() {
+      console.log(a);
+  });
+}
+
+// React.useXXX case
+function MyComponent13() {
+  let a = 1;
+  React.useEffect(() => {
+      console.log(a);
+  });
+}

crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js.snap

@@ -10,7 +10,7 @@ function MyComponent1() {
     useEffect(() => {
       console.log(a, b);
     });
-  }
+}
 
 // interaction with other react hooks
 
@@ -89,6 +89,44 @@ const MyComponent8 = React.memo(({ a }) => {
   });
 });
 
+// exported functions
+export function MyComponent9() {
+  let a = 1;
+  useEffect(() => {
+      console.log(a);
+  });
+}
+
+export default function MyComponent10() {
+  let a = 1;
+  useEffect(() => {
+      console.log(a);
+  });
+}
+
+// named function
+function MyComponent11() {
+  let a = 1;
+  useEffect(function inner() {
+      console.log(a);
+  });
+}
+
+function MyComponent12() {
+  let a = 1;
+  useEffect(async function inner() {
+      console.log(a);
+  });
+}
+
+// React.useXXX case
+function MyComponent13() {
+  let a = 1;
+  React.useEffect(() => {
+      console.log(a);
+  });
+}
+
 ```
 
 # Diagnostics
@@ -111,7 +149,7 @@ missingDependenciesInvalid.js:4:5 lint/nursery/useExhaustiveDependencies ━━
   > 5 │       console.log(a, b);
       │                   ^
     6 │     });
-    7 │   }
+    7 │ }
 
 
 ```
@@ -135,7 +173,7 @@ missingDependenciesInvalid.js:4:5 lint/nursery/useExhaustiveDependencies ━━
   > 5 │       console.log(a, b);
       │                      ^
     6 │     });
-    7 │   }
+    7 │ }
 
 
 ```
@@ -554,4 +592,124 @@ missingDependenciesInvalid.js:81:3 lint/nursery/useExhaustiveDependencies ━━
 
 ```
 
+```
+missingDependenciesInvalid.js:89:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This hook do not specify all of its dependencies.
+
+    87 │ export function MyComponent9() {
+    88 │   let a = 1;
+  > 89 │   useEffect(() => {
+       │   ^^^^^^^^^
+    90 │       console.log(a);
+    91 │   });
+
+  i This dependency is not specified in the hook dependency list.
+
+    88 │   let a = 1;
+    89 │   useEffect(() => {
+  > 90 │       console.log(a);
+       │                   ^
+    91 │   });
+    92 │ }
+
+
+```
+
+```
+missingDependenciesInvalid.js:96:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This hook do not specify all of its dependencies.
+
+    94 │ export default function MyComponent10() {
+    95 │   let a = 1;
+  > 96 │   useEffect(() => {
+       │   ^^^^^^^^^
+    97 │       console.log(a);
+    98 │   });
+
+  i This dependency is not specified in the hook dependency list.
+
+    95 │   let a = 1;
+    96 │   useEffect(() => {
+  > 97 │       console.log(a);
+       │                   ^
+    98 │   });
+    99 │ }
+
+
+```
+
+```
+missingDependenciesInvalid.js:104:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This hook do not specify all of its dependencies.
+
+    102 │ function MyComponent11() {
+    103 │   let a = 1;
+  > 104 │   useEffect(function inner() {
+        │   ^^^^^^^^^
+    105 │       console.log(a);
+    106 │   });
+
+  i This dependency is not specified in the hook dependency list.
+
+    103 │   let a = 1;
+    104 │   useEffect(function inner() {
+  > 105 │       console.log(a);
+        │                   ^
+    106 │   });
+    107 │ }
+
+
+```
+
+```
+missingDependenciesInvalid.js:111:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This hook do not specify all of its dependencies.
+
+    109 │ function MyComponent12() {
+    110 │   let a = 1;
+  > 111 │   useEffect(async function inner() {
+        │   ^^^^^^^^^
+    112 │       console.log(a);
+    113 │   });
+
+  i This dependency is not specified in the hook dependency list.
+
+    110 │   let a = 1;
+    111 │   useEffect(async function inner() {
+  > 112 │       console.log(a);
+        │                   ^
+    113 │   });
+    114 │ }
+
+
+```
+
+```
+missingDependenciesInvalid.js:119:9 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This hook do not specify all of its dependencies.
+
+    117 │ function MyComponent13() {
+    118 │   let a = 1;
+  > 119 │   React.useEffect(() => {
+        │         ^^^^^^^^^
+    120 │       console.log(a);
+    121 │   });
+
+  i This dependency is not specified in the hook dependency list.
+
+    118 │   let a = 1;
+    119 │   React.useEffect(() => {
+  > 120 │       console.log(a);
+        │                   ^
+    121 │   });
+    122 │ }
+
+
+```
+
 

crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js

@@ -127,3 +127,41 @@ const MyComponent11 = React.memo(() => {
         console.log(outside);
     });
 });
+
+// exported functions
+export function MyComponent12() {
+    let a = 1;
+    useEffect(() => {
+        console.log(a);
+    }, [a]);
+}
+
+export default function MyComponent13() {
+    let a = 1;
+    useEffect(() => {
+        console.log(a);
+    }, [a]);
+}
+
+// named function
+function MyComponent14() {
+    let a = 1;
+    useEffect(function inner() {
+        console.log(a);
+    }, [a]);
+}
+
+function MyComponent15() {
+    let a = 1;
+    useEffect(async function inner() {
+        console.log(a);
+    }, [a]);
+}
+
+// React.useXXX case
+function MyComponent16() {
+    let a = 1;
+    React.useEffect(() => {
+        console.log(a);
+    }, [a]);
+}