From f5a71348348751432e408bd60fb9b2c27328989f Mon Sep 17 00:00:00 2001
From: DonIsaac <22823424+DonIsaac@users.noreply.github.com>
Date: Sun, 27 Oct 2024 17:06:58 +0000
Subject: [PATCH] fix(linter/no-unused-vars): false positive for discarded
 reads within sequences (#6907)

Fixes a case where no-unused-vars incorectly reports a read as unused in edge
cases where a logical/binary expression is used as a conditional shorthand to
write a variable within sequence expressions.

Some code examples will make this more clear.
```js
function foo(a) {
  let x = somePropertyIWantToCheck();
  (x in a && x.hasPropA = true, console.log(a))
}
```
Since the logical expression is not in the last position within the sequence
expression list, it's getting discarded as unused. However, the right expression
(`x.hasPropA = true`) has side effects, so it _is_ being used.
---
 crates/oxc_ast/src/ast_impl/js.rs             |  5 ++++
 .../rules/eslint/no_unused_vars/tests/oxc.rs  | 24 +++++++++++++++++++
 .../src/rules/eslint/no_unused_vars/usage.rs  | 17 +++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs
index e8709c561aac9..29162b4103da0 100644
--- a/crates/oxc_ast/src/ast_impl/js.rs
+++ b/crates/oxc_ast/src/ast_impl/js.rs
@@ -279,6 +279,11 @@ impl<'a> Expression<'a> {
             false
         }
     }
+
+    /// Returns `true` if this is an [assignment expression](AssignmentExpression).
+    pub fn is_assignment(&self) -> bool {
+        matches!(self, Expression::AssignmentExpression(_))
+    }
 }
 
 impl<'a> fmt::Display for IdentifierName<'a> {
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
index ba16292523178..5c21298d8a680 100644
--- 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
@@ -208,6 +208,30 @@ fn test_vars_discarded_reads() {
 
         new Test();
         ",
+        "function foo(a) {
+            const Bar = require('./bar');
+            a instanceof Bar && (this.a = a);
+        }
+        foo(1)
+        ",
+        "function foo(a) {
+            const Bar = require('./bar');
+            (a instanceof Bar && (this.a = a), this.b = 1);
+        }
+        foo(1)
+        ",
+        "function foo(a) {
+            const Bar = require('./bar');
+            (a instanceof Bar && (this.a ||= 2), this.b = 1);
+        }
+        foo(1)
+        ",
+        "function foo(a) {
+            const bar = require('./bar');
+            (a in bar && (this.a = a), this.b = 1);
+        }
+        foo(1)
+        ",
     ];
 
     let fail = vec![
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
index 46dc3bca48fdc..b888442265586 100644
--- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs
+++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs
@@ -559,6 +559,23 @@ impl<'s, 'a> Symbol<'s, 'a> {
                         return false;
                     }
                 }
+                // x && (a = x)
+                (AstKind::LogicalExpression(expr), _) => {
+                    if expr.left.span().contains_inclusive(ref_span())
+                        && expr.right.get_inner_expression().is_assignment()
+                    {
+                        return false;
+                    }
+                }
+                // x instanceof Foo && (a = x)
+                (AstKind::BinaryExpression(expr), _) if expr.operator.is_relational() => {
+                    if expr.left.span().contains_inclusive(ref_span())
+                        && expr.right.get_inner_expression().is_assignment()
+                    {
+                        return false;
+                    }
+                    continue;
+                }
                 (parent, AstKind::SequenceExpression(seq)) => {
                     debug_assert!(
                         !seq.expressions.is_empty(),