Skip to content

Commit 9e9b5e0

Browse files
authoredMay 21, 2021
Update: no-unused-vars false negative with comma operator (fixes #14325) (#14354)
* Fix: report error for sequence expression in no-unused-vars (fixes #14325) * Chore: add tests * Update: suggestions * Chore: refactor * Chore: refactor * Fix: logic * Chore: add tests
1 parent afe9569 commit 9e9b5e0

File tree

2 files changed

+96
-3
lines changed

2 files changed

+96
-3
lines changed
 

‎lib/rules/no-unused-vars.js

+27-3
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,31 @@ module.exports = {
410410
);
411411
}
412412

413+
/**
414+
* Checks whether a given node is unused expression or not.
415+
* @param {ASTNode} node The node itself
416+
* @returns {boolean} The node is an unused expression.
417+
* @private
418+
*/
419+
function isUnusedExpression(node) {
420+
const parent = node.parent;
421+
422+
if (parent.type === "ExpressionStatement") {
423+
return true;
424+
}
425+
426+
if (parent.type === "SequenceExpression") {
427+
const isLastExpression = parent.expressions[parent.expressions.length - 1] === node;
428+
429+
if (!isLastExpression) {
430+
return true;
431+
}
432+
return isUnusedExpression(parent);
433+
}
434+
435+
return false;
436+
}
437+
413438
/**
414439
* Checks whether a given reference is a read to update itself or not.
415440
* @param {eslint-scope.Reference} ref A reference to check.
@@ -420,20 +445,19 @@ module.exports = {
420445
function isReadForItself(ref, rhsNode) {
421446
const id = ref.identifier;
422447
const parent = id.parent;
423-
const grandparent = parent.parent;
424448

425449
return ref.isRead() && (
426450

427451
// self update. e.g. `a += 1`, `a++`
428452
(// in RHS of an assignment for itself. e.g. `a = a + 1`
429453
((
430454
parent.type === "AssignmentExpression" &&
431-
grandparent.type === "ExpressionStatement" &&
455+
isUnusedExpression(parent) &&
432456
parent.left === id
433457
) ||
434458
(
435459
parent.type === "UpdateExpression" &&
436-
grandparent.type === "ExpressionStatement"
460+
isUnusedExpression(parent)
437461
) || rhsNode &&
438462
isInside(id, rhsNode) &&
439463
!isInsideOfStorableFunction(id, rhsNode)))

‎tests/lib/rules/no-unused-vars.js

+69
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,10 @@ ruleTester.run("no-unused-vars", rule, {
177177
{ code: "(function(obj) { for ( const name in obj ) { return true } })({})", parserOptions: { ecmaVersion: 6 } },
178178
{ code: "(function(obj) { for ( const name in obj ) return true })({})", parserOptions: { ecmaVersion: 6 } },
179179

180+
// Sequence Expressions (See https://github.com/eslint/eslint/issues/14325)
181+
{ code: "let x = 0; foo = (0, x++);", parserOptions: { ecmaVersion: 6 } },
182+
{ code: "let x = 0; foo = (0, x += 1);", parserOptions: { ecmaVersion: 6 } },
183+
180184
// caughtErrors
181185
{
182186
code: "try{}catch(err){console.error(err);}",
@@ -995,6 +999,71 @@ ruleTester.run("no-unused-vars", rule, {
995999
definedError("c")
9961000
]
9971001
},
1002+
1003+
// https://github.com/eslint/eslint/issues/14325
1004+
{
1005+
code: `let x = 0;
1006+
x++, x = 0;`,
1007+
parserOptions: { ecmaVersion: 2015 },
1008+
errors: [{ ...assignedError("x"), line: 2, column: 18 }]
1009+
},
1010+
{
1011+
code: `let x = 0;
1012+
x++, x = 0;
1013+
x=3;`,
1014+
parserOptions: { ecmaVersion: 2015 },
1015+
errors: [{ ...assignedError("x"), line: 3, column: 13 }]
1016+
},
1017+
{
1018+
code: "let x = 0; x++, 0;",
1019+
parserOptions: { ecmaVersion: 2015 },
1020+
errors: [{ ...assignedError("x"), line: 1, column: 12 }]
1021+
},
1022+
{
1023+
code: "let x = 0; 0, x++;",
1024+
parserOptions: { ecmaVersion: 2015 },
1025+
errors: [{ ...assignedError("x"), line: 1, column: 15 }]
1026+
},
1027+
{
1028+
code: "let x = 0; 0, (1, x++);",
1029+
parserOptions: { ecmaVersion: 2015 },
1030+
errors: [{ ...assignedError("x"), line: 1, column: 19 }]
1031+
},
1032+
{
1033+
code: "let x = 0; foo = (x++, 0);",
1034+
parserOptions: { ecmaVersion: 2015 },
1035+
errors: [{ ...assignedError("x"), line: 1, column: 19 }]
1036+
},
1037+
{
1038+
code: "let x = 0; foo = ((0, x++), 0);",
1039+
parserOptions: { ecmaVersion: 2015 },
1040+
errors: [{ ...assignedError("x"), line: 1, column: 23 }]
1041+
},
1042+
{
1043+
code: "let x = 0; x += 1, 0;",
1044+
parserOptions: { ecmaVersion: 2015 },
1045+
errors: [{ ...assignedError("x"), line: 1, column: 12 }]
1046+
},
1047+
{
1048+
code: "let x = 0; 0, x += 1;",
1049+
parserOptions: { ecmaVersion: 2015 },
1050+
errors: [{ ...assignedError("x"), line: 1, column: 15 }]
1051+
},
1052+
{
1053+
code: "let x = 0; 0, (1, x += 1);",
1054+
parserOptions: { ecmaVersion: 2015 },
1055+
errors: [{ ...assignedError("x"), line: 1, column: 19 }]
1056+
},
1057+
{
1058+
code: "let x = 0; foo = (x += 1, 0);",
1059+
parserOptions: { ecmaVersion: 2015 },
1060+
errors: [{ ...assignedError("x"), line: 1, column: 19 }]
1061+
},
1062+
{
1063+
code: "let x = 0; foo = ((0, x += 1), 0);",
1064+
parserOptions: { ecmaVersion: 2015 },
1065+
errors: [{ ...assignedError("x"), line: 1, column: 23 }]
1066+
},
9981067
{
9991068
code: "(function ({ a, b }, { c } ) { return b; })();",
10001069
parserOptions: { ecmaVersion: 2015 },

0 commit comments

Comments
 (0)
Please sign in to comment.