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.
This commit is contained in:
DonIsaac 2024-10-27 17:06:58 +00:00
parent dde095c7e1
commit f5a7134834
3 changed files with 46 additions and 0 deletions

View file

@ -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> {

View file

@ -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![

View file

@ -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(),