Update: fix no-unused-vars false negative with comma operator (#14928)

* Update: fix no-unused-vars false negative with comma operator * Fix: no-unused-vars false positive
eslint · Aug 21, 2021 · f110926 · f110926
1 parent e98f14d
commit f110926
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 27 deletions.
diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js
@@ -295,6 +295,31 @@ module.exports = {
             );
         }
 
+        /**
+         * Checks whether a given node is unused expression or not.
+         * @param {ASTNode} node The node itself
+         * @returns {boolean} The node is an unused expression.
+         * @private
+         */
+        function isUnusedExpression(node) {
+            const parent = node.parent;
+
+            if (parent.type === "ExpressionStatement") {
+                return true;
+            }
+
+            if (parent.type === "SequenceExpression") {
+                const isLastExpression = parent.expressions[parent.expressions.length - 1] === node;
+
+                if (!isLastExpression) {
+                    return true;
+                }
+                return isUnusedExpression(parent);
+            }
+
+            return false;
+        }
+
         /**
          * If a given reference is left-hand side of an assignment, this gets
          * the right-hand side node of the assignment.
@@ -313,7 +338,6 @@ module.exports = {
         function getRhsNode(ref, prevRhsNode) {
             const id = ref.identifier;
             const parent = id.parent;
-            const grandparent = parent.parent;
             const refScope = ref.from.variableScope;
             const varScope = ref.resolved.scope.variableScope;
             const canBeUsedLater = refScope !== varScope || astUtils.isInLoop(id);
@@ -327,7 +351,7 @@ module.exports = {
             }
 
             if (parent.type === "AssignmentExpression" &&
-                grandparent.type === "ExpressionStatement" &&
+                isUnusedExpression(parent) &&
                 id === parent.left &&
                 !canBeUsedLater
             ) {
@@ -410,31 +434,6 @@ module.exports = {
             );
         }
 
-        /**
-         * Checks whether a given node is unused expression or not.
-         * @param {ASTNode} node The node itself
-         * @returns {boolean} The node is an unused expression.
-         * @private
-         */
-        function isUnusedExpression(node) {
-            const parent = node.parent;
-
-            if (parent.type === "ExpressionStatement") {
-                return true;
-            }
-
-            if (parent.type === "SequenceExpression") {
-                const isLastExpression = parent.expressions[parent.expressions.length - 1] === node;
-
-                if (!isLastExpression) {
-                    return true;
-                }
-                return isUnusedExpression(parent);
-            }
-
-            return false;
-        }
-
         /**
          * Checks whether a given reference is a read to update itself or not.
          * @param {eslint-scope.Reference} ref A reference to check.

diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js
@@ -180,6 +180,7 @@ ruleTester.run("no-unused-vars", rule, {
         // Sequence Expressions (See https://github.com/eslint/eslint/issues/14325)
         { code: "let x = 0; foo = (0, x++);", parserOptions: { ecmaVersion: 6 } },
         { code: "let x = 0; foo = (0, x += 1);", parserOptions: { ecmaVersion: 6 } },
+        { code: "let x = 0; foo = (0, x = x + 1);", parserOptions: { ecmaVersion: 6 } },
 
         // caughtErrors
         {
@@ -1064,6 +1065,55 @@ ruleTester.run("no-unused-vars", rule, {
             parserOptions: { ecmaVersion: 2015 },
             errors: [{ ...assignedError("x"), line: 1, column: 23 }]
         },
+
+        // https://github.com/eslint/eslint/issues/14866
+        {
+            code: `let z = 0;
+            z = z + 1, z = 2;
+            `,
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("z"), line: 2, column: 24 }]
+        },
+        {
+            code: `let z = 0;
+            z = z+1, z = 2;
+            z = 3;`,
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("z"), line: 3, column: 13 }]
+        },
+        {
+            code: `let z = 0;
+            z = z+1, z = 2;
+            z = z+3;
+            `,
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("z"), line: 3, column: 13 }]
+        },
+        {
+            code: "let x = 0; 0, x = x+1;",
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("x"), line: 1, column: 15 }]
+        },
+        {
+            code: "let x = 0; x = x+1, 0;",
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("x"), line: 1, column: 12 }]
+        },
+        {
+            code: "let x = 0; foo = ((0, x = x + 1), 0);",
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("x"), line: 1, column: 23 }]
+        },
+        {
+            code: "let x = 0; foo = (x = x+1, 0);",
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("x"), line: 1, column: 19 }]
+        },
+        {
+            code: "let x = 0; 0, (1, x=x+1);",
+            parserOptions: { ecmaVersion: 2020 },
+            errors: [{ ...assignedError("x"), line: 1, column: 19 }]
+        },
         {
             code: "(function ({ a, b }, { c } ) { return b; })();",
             parserOptions: { ecmaVersion: 2015 },