fix(linter): false positive in no-unused-vars (#11002)

camc314 · camc314 · commit 8956870cd171 · 2025-05-14T10:18:03.000Z
close: #10994 fixes a false positive when a variable is reassigned in a different scope
diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
@@ -281,7 +281,6 @@ impl NoUnusedVars {
                 }
                 let report = match symbol.references().rev().find(|r| r.is_write()) {
                     Some(last_write) => {
-                        // ahg
                         let span = ctx.nodes().get_node(last_write.node_id()).kind().span();
                         diagnostic::assign(symbol, span, &self.vars_ignore_pattern)
                     }
diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
@@ -171,6 +171,10 @@ fn test_vars_self_use() {
         }
         foo();
         ",
+        "
+        let cancel = () => {}
+        export function close() { cancel = cancel?.() }
+        ",
     ];
     let fail = vec![
         "
@@ -183,6 +187,14 @@ fn test_vars_self_use() {
             return foo
         }
         ",
+        "
+        let cancel = () => {};
+        cancel = cancel?.();
+        ",
+        "
+        let cancel = () => {};
+        { cancel = cancel?.(); }
+        ",
     ];
 
     Tester::new(NoUnusedVars::NAME, NoUnusedVars::PLUGIN, pass, fail)
diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs
@@ -1,6 +1,7 @@
 //! This module contains logic for checking if any [`Reference`]s to a
 //! [`Symbol`] are considered a usage.
 
+use itertools::Itertools;
 use oxc_ast::{AstKind, ast::*};
 use oxc_semantic::{AstNode, NodeId, Reference, ScopeId, SymbolFlags, SymbolId};
 use oxc_span::{GetSpan, Span};
@@ -427,9 +428,31 @@ impl<'a> Symbol<'_, 'a> {
                     match left {
                         AssignmentTarget::AssignmentTargetIdentifier(id) => {
                             if id.name == name {
+                                // Compare *variable scopes* (the nearest function / TS&nbsp;module / class&#8209;static block).
+                                //
+                                // If the variable scope is the same, the the variable is still unused
+                                // ```ts
+                                // let cancel = () => {};
+                                // {                      // plain block
+                                //   cancel = cancel?.(); // `cancel` is unused
+                                // }
+                                // ```
+                                //
+                                // If the variable scope is different, the read can be observed later, so it counts as a real usage:
+                                // ```ts
+                                // let cancel = () => {};
+                                // function foo() {        // new var&#8209;scope
+                                //   cancel = cancel?.();  // `cancel` is used
+                                // }
+                                // ```
+                                if self.get_parent_variable_scope(self.get_ref_scope(reference))
+                                    != self.get_parent_variable_scope(self.scope_id())
+                                {
+                                    return false;
+                                }
                                 is_used_by_others = false;
                             } else {
-                                return false; // we can short-circuit
+                                return false;
                             }
                         }
                         AssignmentTarget::TSAsExpression(v)
@@ -832,4 +855,18 @@ impl<'a> Symbol<'_, 'a> {
             };
         }
     }
+
+    /// Return the **variable scope** for the given `scope_id`.
+    ///
+    /// A variable scope is the closest ancestor scope (including `scope_id`
+    /// itself) whose kind can *outlive* the current execution slice:
+    ///   * function&#8209;like scopes  
+    ///   * class static blocks  
+    ///   * TypeScript namespace/module blocks
+    fn get_parent_variable_scope(&self, scope_id: ScopeId) -> ScopeId {
+        self.scoping()
+            .scope_ancestors(scope_id)
+            .find_or_last(|scope_id| self.scoping().scope_flags(*scope_id).is_var())
+            .expect("scope iterator will always contain at least one element")
+    }
 }
diff --git a/crates/oxc_linter/src/snapshots/eslint_no_unused_vars@oxc-vars-self-use.snap b/crates/oxc_linter/src/snapshots/eslint_no_unused_vars@oxc-vars-self-use.snap
@@ -20,3 +20,29 @@ source: crates/oxc_linter/src/tester.rs
  3 &#9474;             return foo
    &#9584;&#9472;&#9472;&#9472;&#9472;
   help: Consider removing this declaration.
+
+  &#9888; eslint(no-unused-vars): Variable 'cancel' is assigned a value but never used. Unused variables should start with a '_'.
+   &#9581;&#9472;[no_unused_vars.tsx:2:13]
+ 1 &#9474; 
+ 2 &#9474;         let cancel = () => {};
+   &middot;             &#9472;&#9472;&#9472;&#9516;&#9472;&#9472;
+   &middot;                &#9584;&#9472;&#9472; 'cancel' is declared here
+ 3 &#9474;         cancel = cancel?.();
+   &middot;         &#9472;&#9472;&#9472;&#9516;&#9472;&#9472;
+   &middot;            &#9584;&#9472;&#9472; it was last assigned here
+ 4 &#9474;         
+   &#9584;&#9472;&#9472;&#9472;&#9472;
+  help: Did you mean to use this variable?
+
+  &#9888; eslint(no-unused-vars): Variable 'cancel' is assigned a value but never used. Unused variables should start with a '_'.
+   &#9581;&#9472;[no_unused_vars.tsx:2:13]
+ 1 &#9474; 
+ 2 &#9474;         let cancel = () => {};
+   &middot;             &#9472;&#9472;&#9472;&#9516;&#9472;&#9472;
+   &middot;                &#9584;&#9472;&#9472; 'cancel' is declared here
+ 3 &#9474;         { cancel = cancel?.(); }
+   &middot;           &#9472;&#9472;&#9472;&#9516;&#9472;&#9472;
+   &middot;              &#9584;&#9472;&#9472; it was last assigned here
+ 4 &#9474;         
+   &#9584;&#9472;&#9472;&#9472;&#9472;
+  help: Did you mean to use this variable?

Original file line number	Diff line number	Diff line change
`@@ -281,7 +281,6 @@ impl NoUnusedVars {`
`281`	`281`	`}`
`282`	`282`	`let report = match symbol.references().rev().find(\|r\| r.is_write()) {`
`283`	`283`	`Some(last_write) => {`
`284`		`- // ahg`
`285`	`284`	`let span = ctx.nodes().get_node(last_write.node_id()).kind().span();`
`286`	`285`	`diagnostic::assign(symbol, span, &self.vars_ignore_pattern)`
`287`	`286`	`}`
-Original file line number
+Diff line change
 │             return foo
    ╰────
   help: Consider removing this declaration.
++
 +  ⚠ eslint(no-unused-vars): Variable 'cancel' is assigned a value but never used. Unused variables should start with a '_'.
 +   ╭─[no_unused_vars.tsx:2:13]
 + 1 │
 + 2 │         let cancel = () => {};
 +   ·             ───┬──
 +   ·                ╰── 'cancel' is declared here
 + 3 │         cancel = cancel?.();
 +   ·         ───┬──
 +   ·            ╰── it was last assigned here
 + 4 │
 +   ╰────
 +  help: Did you mean to use this variable?
++
 +  ⚠ eslint(no-unused-vars): Variable 'cancel' is assigned a value but never used. Unused variables should start with a '_'.
 +   ╭─[no_unused_vars.tsx:2:13]
 + 1 │
 + 2 │         let cancel = () => {};
 +   ·             ───┬──
 +   ·                ╰── 'cancel' is declared here
 + 3 │         { cancel = cancel?.(); }
 +   ·           ───┬──
 +   ·              ╰── it was last assigned here
 + 4 │
 +   ╰────
 +  help: Did you mean to use this variable?