fix(lint/useExhaustiveDependencies): ignore missing deps declared out…

… of component function scope (#1069)

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

CHANGELOG.md

@@ -57,6 +57,19 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
   }
   ```
 
+- Fix [#651](https://github.com/biomejs/biome/issues/651), [useExhaustiveDependencies](https://biomejs.dev/linter/rules/use-exhaustive-dependencies) no longer reports out of scope dependecies. Contributed by @kalleep
+
+  The following code is no longer reported:
+  ```ts
+  let outer = false;
+
+  const Component = ({}) => {
+    useEffect(() => {
+      outer = true;
+    }, []);
+  }
+  ```
+
 ### Parser
 
 

crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs

@@ -7,11 +7,11 @@ use biome_deserialize::{
     VisitableType,
 };
 use biome_js_semantic::{Capture, SemanticModel};
-use biome_js_syntax::TsTypeofType;
 use biome_js_syntax::{
     binding_ext::AnyJsBindingDeclaration, JsCallExpression, JsStaticMemberExpression, JsSyntaxKind,
     JsSyntaxNode, JsVariableDeclaration, TextRange,
 };
+use biome_js_syntax::{AnyJsExpression, JsIdentifierExpression, TsTypeofType};
 use biome_rowan::{AstNode, SyntaxNodeCast};
 use bpaf::Bpaf;
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -131,6 +131,16 @@ declare_rule! {
     /// }
     /// ```
     ///
+    /// ```js
+    /// import { useEffect } from "react";
+    /// let outer = false;
+    /// function component() {
+    ///     useEffect(() => {
+    ///         outer = true;
+    ///     }, []);
+    /// }
+    /// ```
+    ///
     /// ## Options
     ///
     /// Allows to specify custom hooks - from libraries or internal projects - that can be considered stable.
@@ -394,7 +404,8 @@ pub enum Fix {
     /// When a dependency needs to be removed.
     RemoveDependency {
         function_name_range: TextRange,
-        dependencies: Vec<TextRange>,
+        component_function: JsSyntaxNode,
+        dependencies: Vec<AnyJsExpression>,
     },
     /// When a dependency is more deep than the capture
     DependencyTooDeep {
@@ -450,7 +461,6 @@ fn capture_needs_to_be_in_the_dependency_list(
         | AnyJsBindingDeclaration::TsInferType(_)
         | AnyJsBindingDeclaration::TsMappedType(_)
         | AnyJsBindingDeclaration::TsTypeParameter(_) => None,
-
         // Variable declarators are stable if ...
         AnyJsBindingDeclaration::JsVariableDeclarator(declarator) => {
             let declaration = declarator
@@ -459,10 +469,10 @@ fn capture_needs_to_be_in_the_dependency_list(
                 .find_map(JsVariableDeclaration::cast)?;
             let declaration_range = declaration.syntax().text_range();
 
-            if declaration.is_const() {
-                // ... they are `const` and declared outside of the component function
-                let _ = component_function_range.intersect(declaration_range)?;
+            // ... they are declared outside of the component function
+            let _ = component_function_range.intersect(declaration_range)?;
 
+            if declaration.is_const() {
                 // ... they are `const` and their initializer is constant
                 let initializer = declarator.initializer()?;
                 let expr = initializer.expression().ok()?;
@@ -519,6 +529,25 @@ fn function_of_hook_call(call: &JsCallExpression) -> Option<JsSyntaxNode> {
     })
 }
 
+/// Check if `identifier` is declared outside of `function` scope
+fn is_out_of_function_scope(
+    identifier: &AnyJsExpression,
+    function: &JsSyntaxNode,
+    model: &SemanticModel,
+) -> Option<bool> {
+    let identifier_name = JsIdentifierExpression::cast_ref(identifier.syntax())?
+        .name()
+        .ok()?;
+
+    let declaration = model.binding(&identifier_name)?.tree().declaration()?;
+
+    Some(
+        model
+            .scope(declaration.syntax())
+            .is_ancestor_of(&model.scope(function)),
+    )
+}
+
 impl Rule for UseExhaustiveDependencies {
     type Query = Semantic<JsCallExpression>;
     type State = Fix;
@@ -574,33 +603,26 @@ impl Rule for UseExhaustiveDependencies {
                 })
                 .collect();
 
-            let deps: Vec<(String, TextRange)> = result
-                .all_dependencies()
-                .map(|dep| {
-                    (
-                        dep.syntax().text_trimmed().to_string(),
-                        dep.syntax().text_trimmed_range(),
-                    )
-                })
-                .collect();
+            let deps: Vec<_> = result.all_dependencies().collect();
             let dependencies_len = deps.len();
 
             let mut add_deps: BTreeMap<String, Vec<TextRange>> = BTreeMap::new();
-            let mut remove_deps: Vec<TextRange> = vec![];
 
             // Evaluate all the captures
             for (capture_text, capture_range, _) in captures.iter() {
                 let mut suggested_fix = None;
                 let mut is_captured_covered = false;
-                for (dependency_text, dependency_range) in deps.iter() {
+                for dep in deps.iter() {
                     // capture_text and dependency_text should filter the "?" inside
                     // in order to ignore optional chaining
                     let filter_capture_text = capture_text.replace('?', "");
-                    let filter_dependency_text = dependency_text.replace('?', "");
+                    let filter_dependency_text =
+                        dep.syntax().text_trimmed().to_string().replace('?', "");
                     let capture_deeper_than_dependency =
                         filter_capture_text.starts_with(&filter_dependency_text);
                     let dependency_deeper_than_capture =
                         filter_dependency_text.starts_with(&filter_capture_text);
+
                     match (
                         capture_deeper_than_dependency,
                         dependency_deeper_than_capture,
@@ -631,7 +653,7 @@ impl Rule for UseExhaustiveDependencies {
                             suggested_fix = Some(Fix::DependencyTooDeep {
                                 function_name_range: result.function_name_range,
                                 capture_range: *capture_range,
-                                dependency_range: *dependency_range,
+                                dependency_range: dep.syntax().text_trimmed_range(),
                             });
                         }
                         _ => {}
@@ -648,14 +670,16 @@ impl Rule for UseExhaustiveDependencies {
                 }
             }
 
+            let mut remove_deps: Vec<AnyJsExpression> = vec![];
             // Search for dependencies not captured
-            for (dependency_text, dep_range) in deps {
+            for dep in deps {
                 let mut covers_any_capture = false;
                 for (capture_text, _, _) in captures.iter() {
                     // capture_text and dependency_text should filter the "?" inside
                     // in order to ignore optional chaining
                     let filter_capture_text = capture_text.replace('?', "");
-                    let filter_dependency_text = dependency_text.replace('?', "");
+                    let filter_dependency_text =
+                        dep.syntax().text_trimmed().to_string().replace('?', "");
                     let capture_deeper_dependency =
                         filter_capture_text.starts_with(&filter_dependency_text);
                     let dependency_deeper_capture =
@@ -667,7 +691,7 @@ impl Rule for UseExhaustiveDependencies {
                 }
 
                 if !covers_any_capture {
-                    remove_deps.push(dep_range);
+                    remove_deps.push(dep);
                 }
             }
 
@@ -683,6 +707,7 @@ impl Rule for UseExhaustiveDependencies {
             if !remove_deps.is_empty() {
                 signals.push(Fix::RemoveDependency {
                     function_name_range: result.function_name_range,
+                    component_function,
                     dependencies: remove_deps,
                 });
             }
@@ -691,7 +716,7 @@ impl Rule for UseExhaustiveDependencies {
         signals
     }
 
-    fn diagnostic(_: &RuleContext<Self>, dep: &Self::State) -> Option<RuleDiagnostic> {
+    fn diagnostic(ctx: &RuleContext<Self>, dep: &Self::State) -> Option<RuleDiagnostic> {
         match dep {
             Fix::AddDependency {
                 function_name_range,
@@ -726,6 +751,7 @@ impl Rule for UseExhaustiveDependencies {
             Fix::RemoveDependency {
                 function_name_range,
                 dependencies,
+                component_function,
             } => {
                 let mut diag = RuleDiagnostic::new(
                     rule_category!(),
@@ -735,8 +761,20 @@ impl Rule for UseExhaustiveDependencies {
                     },
                 );
 
-                for range in dependencies {
-                    diag = diag.detail(range, "This dependency can be removed from the list.");
+                let model = ctx.model();
+                for dep in dependencies {
+                    if is_out_of_function_scope(dep, component_function, model).unwrap_or(false) {
+                        diag = diag.detail(
+                            dep.syntax().text_trimmed_range(),
+                            "Outer scope values aren't valid dependencies because mutating them doesn't re-render the component.",
+
+                        );
+                    } else {
+                        diag = diag.detail(
+                            dep.syntax().text_trimmed_range(),
+                            "This dependency can be removed from the list.",
+                        );
+                    }
                 }
 
                 Some(diag)

crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/extraDependenciesInvalid.js

@@ -29,3 +29,11 @@ function MyComponent1() {
       console.log(someObj)
   }, [someObj.id]);
 }
+
+let outer = false;
+// Dependencies from outer scope should not be valid
+function MyComponent3() {
+  useEffect(() => {
+      outer = true;
+  }, [outer]);
+}

crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/extraDependenciesInvalid.js.snap

@@ -36,6 +36,14 @@ function MyComponent1() {
   }, [someObj.id]);
 }
 
+let outer = false;
+// Dependencies from outer scope should not be valid
+function MyComponent3() {
+  useEffect(() => {
+      outer = true;
+  }, [outer]);
+}
+
 ```
 
 # Diagnostics
@@ -177,4 +185,28 @@ extraDependenciesInvalid.js:28:3 lint/correctness/useExhaustiveDependencies ━
 
 ```
 
+```
+extraDependenciesInvalid.js:36:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This hook specifies more dependencies than necessary.
+  
+    34 │ // Dependencies from outer scope should not be valid
+    35 │ function MyComponent3() {
+  > 36 │   useEffect(() => {
+       │   ^^^^^^^^^
+    37 │       outer = true;
+    38 │   }, [outer]);
+  
+  i Outer scope values aren't valid dependencies because mutating them doesn't re-render the component.
+  
+    36 │   useEffect(() => {
+    37 │       outer = true;
+  > 38 │   }, [outer]);
+       │       ^^^^^
+    39 │ }
+    40 │ 
+  
+
+```
+
 

crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js

@@ -223,3 +223,13 @@ function MyComponent21() {
 			console.log(ref.current)
 	}, [])
 }
+
+let outer = false;
+
+// Capture from outer scope
+// https://github.com/biomejs/biome/issues/651
+function MyComponent22() {
+  useEffect(() => {
+    outer = true;
+  }, [])
+}

crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js.snap

@@ -230,6 +230,16 @@ function MyComponent21() {
 	}, [])
 }
 
+let outer = false;
+
+// Capture from outer scope
+// https://github.com/biomejs/biome/issues/651
+function MyComponent22() {
+  useEffect(() => {
+    outer = true;
+  }, [])
+}
+
 ```
 
 

website/src/content/docs/internals/changelog.mdx

@@ -63,6 +63,19 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
   }
   ```
 
+- Fix [#651](https://github.com/biomejs/biome/issues/651), [useExhaustiveDependencies](https://biomejs.dev/linter/rules/use-exhaustive-dependencies) no longer reports out of scope dependecies. Contributed by @kalleep
+
+  The following code is no longer reported:
+  ```ts
+  let outer = false;
+
+  const Component = ({}) => {
+    useEffect(() => {
+      outer = true;
+    }, []);
+  }
+  ```
+
 ### Parser
 
 

website/src/content/docs/linter/rules/use-exhaustive-dependencies.md

@@ -208,6 +208,16 @@ function component() {
 }
 ```
 
+```jsx
+import { useEffect } from "react";
+let outer = false;
+function component() {
+    useEffect(() => {
+        outer = true;
+    }, []);
+}
+```
+
 ## Options
 
 Allows to specify custom hooks - from libraries or internal projects - that can be considered stable.