Update: no-unused-vars ignores read it modifies itself (fixes #6348) (

#6535)

* Update: `no-unused-vars` ignores read it modifies itself (fixes #6348)

`no-unused-vars` comes to ignore a read of a modification of itself.

- A read in RHS of an assignment to itself (e.g. `a = a + 1`)
- A read in LHS of an assignment (e.g. `a += 1`)
- Increments/Decrements (e.g. `a++`)

Those are ignored only if those exist at the direct child of an
ExpressionStatement node. This is in order to avoid false positive such
`foo(a = a + 1)`

* Fix: remove unused variables which were found by this fix.

* Chore: add some tests

docs/rules/no-unused-vars.md

@@ -20,14 +20,19 @@ Examples of **incorrect** code for this rule:
 /*eslint no-unused-vars: "error"*/
 /*global some_unused_var*/
 
-//It checks variables you have defined as global
+// It checks variables you have defined as global
 some_unused_var = 42;
 
 var x;
 
+// Write-only variables are not considered as used.
 var y = 10;
 y = 5;
 
+// A read for a modification of itself is not considered as used.
+var z = 0;
+z = z + 1;
+
 // By default, unused arguments cause warnings.
 (function(foo) {
     return 5;

lib/config/environments.js

@@ -8,15 +8,12 @@
 // Requirements
 //------------------------------------------------------------------------------
 
-var debug = require("debug"),
-    envs = require("../../conf/environments");
+var envs = require("../../conf/environments");
 
 //------------------------------------------------------------------------------
 // Private
 //------------------------------------------------------------------------------
 
-debug = debug("eslint:enviroments");
-
 var environments = Object.create(null);
 
 /**

lib/rules/no-unused-vars.js

@@ -152,6 +152,73 @@ module.exports = {
             return false;
         }
 
+        /**
+         * If a given reference is left-hand side of an assignment, this gets
+         * the right-hand side node of the assignment.
+         *
+         * @param {escope.Reference} ref - A reference to check.
+         * @param {ASTNode} prevRhsNode - The previous RHS node.
+         * @returns {ASTNode} The RHS node.
+         */
+        function getRhsNode(ref, prevRhsNode) {
+            var id = ref.identifier;
+            var parent = id.parent;
+            var granpa = parent.parent;
+
+            /*
+             * Inherits the previous node if this reference is in the node.
+             * This is for `a = a + a`-like code.
+             */
+            if (prevRhsNode &&
+                prevRhsNode.range[0] <= id.range[0] &&
+                prevRhsNode.range[1] >= id.range[1]
+            ) {
+                return prevRhsNode;
+            }
+
+            if (parent.type === "AssignmentExpression" &&
+                granpa.type === "ExpressionStatement" &&
+                id === parent.left
+            ) {
+                return parent.right;
+            }
+            return null;
+        }
+
+        /**
+         * Checks whether a given reference is a read to update itself or not.
+         *
+         * @param {escope.Reference} ref - A reference to check.
+         * @param {ASTNode} rhsNode - The RHS node of the previous assignment.
+         * @returns {boolean} The reference is a read to update itself.
+         */
+        function isReadForItself(ref, rhsNode) {
+            var id = ref.identifier;
+            var parent = id.parent;
+            var granpa = parent.parent;
+
+            return ref.isRead() && (
+
+                // self update. e.g. `a += 1`, `a++`
+                (
+                    parent.type === "AssignmentExpression" &&
+                    granpa.type === "ExpressionStatement" &&
+                    parent.left === id
+                ) ||
+                (
+                    parent.type === "UpdateExpression" &&
+                    granpa.type === "ExpressionStatement"
+                ) ||
+
+                // in RHS of an assignment for itself. e.g. `a = a + 1`
+                (
+                    rhsNode &&
+                    rhsNode.range[0] <= id.range[0] &&
+                    rhsNode.range[1] >= id.range[1]
+                )
+            );
+        }
+
         /**
          * Determines if the variable is used.
          * @param {Variable} variable - The variable to check.
@@ -164,10 +231,19 @@ module.exports = {
                 }).map(function(def) {
                     return def.node;
                 }),
-                isFunctionDefinition = functionNodes.length > 0;
+                isFunctionDefinition = functionNodes.length > 0,
+                rhsNode = null;
 
             return variable.references.some(function(ref) {
-                return isReadRef(ref) && !(isFunctionDefinition && isSelfReference(ref, functionNodes));
+                var forItself = isReadForItself(ref, rhsNode);
+
+                rhsNode = getRhsNode(ref, rhsNode);
+
+                return (
+                    isReadRef(ref) &&
+                    !forItself &&
+                    !(isFunctionDefinition && isSelfReference(ref, functionNodes))
+                );
             });
         }
 

tests/lib/config/environments.js

@@ -9,16 +9,9 @@
 //------------------------------------------------------------------------------
 
 var assert = require("chai").assert,
-    proxyquire = require("proxyquire"),
     envs = require("../../../conf/environments"),
     Environments = require("../../../lib/config/environments");
 
-//------------------------------------------------------------------------------
-// Helpers
-//------------------------------------------------------------------------------
-
-proxyquire = proxyquire.noCallThru().noPreserveCache();
-
 //------------------------------------------------------------------------------
 // Tests
 //------------------------------------------------------------------------------

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

@@ -143,8 +143,15 @@ ruleTester.run("no-unused-vars", rule, {
         {
             code: "try{}catch(err){}",
             options: [{vars: "all", args: "all"}]
-        }
+        },
 
+        // https://github.com/eslint/eslint/issues/6348
+        {code: "var a = 0, b; b = a = a + 1; foo(b);"},
+        {code: "var a = 0, b; b = a += a + 1; foo(b);"},
+        {code: "var a = 0, b; b = a++; foo(b);"},
+        {code: "function foo(a) { var b = a = a + 1; bar(b) } foo();"},
+        {code: "function foo(a) { var b = a += a + 1; bar(b) } foo();"},
+        {code: "function foo(a) { var b = a++; bar(b) } foo();"}
     ],
     invalid: [
         { code: "function foox() { return foox(); }", errors: [{ message: "'foox' is defined but never used", type: "Identifier"}] },
@@ -336,6 +343,17 @@ ruleTester.run("no-unused-vars", rule, {
                 }
             ],
             errors: [{message: "'err' is defined but never used"}]
-        }
+        },
+
+        // Ignore reads for modifications to itself: https://github.com/eslint/eslint/issues/6348
+        {code: "var a = 0; a = a + 1;", errors: [{message: "'a' is defined but never used"}]},
+        {code: "var a = 0; a = a + a;", errors: [{message: "'a' is defined but never used"}]},
+        {code: "var a = 0; a += a + 1;", errors: [{message: "'a' is defined but never used"}]},
+        {code: "var a = 0; a++;", errors: [{message: "'a' is defined but never used"}]},
+        {code: "function foo(a) { a = a + 1 } foo();", errors: [{message: "'a' is defined but never used"}]},
+        {code: "function foo(a) { a += a + 1 } foo();", errors: [{message: "'a' is defined but never used"}]},
+        {code: "function foo(a) { a++ } foo();", errors: [{message: "'a' is defined but never used"}]},
+        {code: "var a = 3; a = a * 5 + 6;", errors: [{message: "'a' is defined but never used"}]},
+        {code: "var a = 2, b = 4; a = a * 2 + b;", errors: [{message: "'a' is defined but never used"}]}
     ]
 });