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
eslint · May 21, 2021 · 9e9b5e0 · 9e9b5e0
1 parent afe9569
commit 9e9b5e0
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 3 deletions.
diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js
@@ -410,6 +410,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;
+        }
+
         /**
          * Checks whether a given reference is a read to update itself or not.
          * @param {eslint-scope.Reference} ref A reference to check.
@@ -420,20 +445,19 @@ module.exports = {
         function isReadForItself(ref, rhsNode) {
             const id = ref.identifier;
             const parent = id.parent;
-            const grandparent = parent.parent;
 
             return ref.isRead() && (
 
                 // self update. e.g. `a += 1`, `a++`
                 (// in RHS of an assignment for itself. e.g. `a = a + 1`
                     ((
                         parent.type === "AssignmentExpression" &&
-                    grandparent.type === "ExpressionStatement" &&
+                    isUnusedExpression(parent) &&
                     parent.left === id
                     ) ||
                 (
                     parent.type === "UpdateExpression" &&
-                    grandparent.type === "ExpressionStatement"
+                    isUnusedExpression(parent)
                 ) || rhsNode &&
                 isInside(id, rhsNode) &&
                 !isInsideOfStorableFunction(id, rhsNode)))

diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js
@@ -177,6 +177,10 @@ ruleTester.run("no-unused-vars", rule, {
         { code: "(function(obj) { for ( const name in obj ) { return true } })({})", parserOptions: { ecmaVersion: 6 } },
         { code: "(function(obj) { for ( const name in obj ) return true })({})", parserOptions: { ecmaVersion: 6 } },
 
+        // 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 } },
+
         // caughtErrors
         {
             code: "try{}catch(err){console.error(err);}",
@@ -995,6 +999,71 @@ ruleTester.run("no-unused-vars", rule, {
                 definedError("c")
             ]
         },
+
+        // https://github.com/eslint/eslint/issues/14325
+        {
+            code: `let x = 0;
+            x++, x = 0;`,
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 2, column: 18 }]
+        },
+        {
+            code: `let x = 0;
+            x++, x = 0;
+            x=3;`,
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 3, column: 13 }]
+        },
+        {
+            code: "let x = 0; x++, 0;",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 12 }]
+        },
+        {
+            code: "let x = 0; 0, x++;",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 15 }]
+        },
+        {
+            code: "let x = 0; 0, (1, x++);",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 19 }]
+        },
+        {
+            code: "let x = 0; foo = (x++, 0);",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 19 }]
+        },
+        {
+            code: "let x = 0; foo = ((0, x++), 0);",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 23 }]
+        },
+        {
+            code: "let x = 0; x += 1, 0;",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 12 }]
+        },
+        {
+            code: "let x = 0; 0, x += 1;",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 15 }]
+        },
+        {
+            code: "let x = 0; 0, (1, x += 1);",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 19 }]
+        },
+        {
+            code: "let x = 0; foo = (x += 1, 0);",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 19 }]
+        },
+        {
+            code: "let x = 0; foo = ((0, x += 1), 0);",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [{ ...assignedError("x"), line: 1, column: 23 }]
+        },
         {
             code: "(function ({ a, b }, { c } ) { return b; })();",
             parserOptions: { ecmaVersion: 2015 },