Update: make no-unused-vars ignore for...in loops with one return (f…

…ixes #2342) Also clean jsdocs for internal helpers
eslint · Jun 4, 2016 · 7d99162 · 7d99162
1 parent ef739cd
commit 7d99162
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 5 deletions.
diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js
@@ -99,7 +99,6 @@ module.exports = {
          * Determines if a given variable is being exported from a module.
          * @param {Variable} variable - EScope variable object.
          * @returns {boolean} True if the variable is exported, false if not.
-         * @private
          */
         function isExported(variable) {
 
@@ -125,7 +124,6 @@ module.exports = {
          * Determines if a reference is a read operation.
          * @param {Reference} ref - An escope Reference
          * @returns {Boolean} whether the given reference represents a read operation
-         * @private
          */
         function isReadRef(ref) {
             return ref.isRead();
@@ -136,7 +134,6 @@ module.exports = {
          * @param {Reference} ref - The reference to check.
          * @param {ASTNode[]} nodes - The candidate function nodes.
          * @returns {boolean} True if it's a self-reference, false if not.
-         * @private
          */
         function isSelfReference(ref, nodes) {
             var scope = ref.from;
@@ -152,10 +149,33 @@ module.exports = {
             return false;
         }
 
+        /**
+         * Determine if an identifier is used either in for...of or for...in loops.
+         * @param {Reference} ref - The reference to check.
+         * @returns {Boolean} whether reference is used in the for...of or for...in loops
+         */
+        function isForRef(ref) {
+            var target = ref.identifier.parent;
+
+            if (target.type !== "ForInStatement") {
+                return false;
+            }
+
+            // "for (...) { return; }"
+            if (target.body.type === "BlockStatement") {
+                target = target.body.body[0];
+
+            // "for (...) return;"
+            } else {
+                target = target.body;
+            }
+
+            return target.type === "ReturnStatement";
+        }
+
         /**
          * Determines if the variable is used.
          * @param {Variable} variable - The variable to check.
-         * @param {Reference[]} references - The variable references to check.
          * @returns {boolean} True if the variable is used
          */
         function isUsedVariable(variable) {
@@ -167,6 +187,10 @@ module.exports = {
                 isFunctionDefinition = functionNodes.length > 0;
 
             return variable.references.some(function(ref) {
+                if (isForRef(ref)) {
+                    return true;
+                }
+
                 return isReadRef(ref) && !(isFunctionDefinition && isSelfReference(ref, functionNodes));
             });
         }
@@ -176,7 +200,6 @@ module.exports = {
          * @param {Scope} scope - an escope Scope object.
          * @param {Variable[]} unusedVars - an array that saving result.
          * @returns {Variable[]} unused variables of the scope and descendant scopes.
-         * @private
          */
         function collectUnusedVariables(scope, unusedVars) {
             var variables = scope.variables;

diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js
@@ -125,6 +125,10 @@ ruleTester.run("no-unused-vars", rule, {
         { code: "function foo(a, _b) { return a; } foo();", options: [ { args: "after-used", argsIgnorePattern: "^_" } ] },
         { code: "var [ firstItemIgnored, secondItem ] = items;\nconsole.log(secondItem);", parserOptions: { ecmaVersion: 6 }, options: [ { vars: "all", varsIgnorePattern: "[iI]gnored" } ] },
 
+        // for...in loops (See #2342)
+        "(function(obj) { var name; for ( name in obj ) return; })({});",
+        "(function(obj) { var name; for ( name in obj ) { return; } })({});",
+
         // caughtErrors
         {
             code: "try{}catch(err){console.error(err);}",
@@ -191,6 +195,8 @@ ruleTester.run("no-unused-vars", rule, {
         { code: "function foo(a, _b, c) { return a; } foo();", options: [ { args: "after-used", argsIgnorePattern: "^_" } ], errors: [{ message: "'c' is defined but never used", line: 1, column: 21 }] },
         { code: "var [ firstItemIgnored, secondItem ] = items;", parserOptions: { ecmaVersion: 6 }, options: [ { vars: "all", varsIgnorePattern: "[iI]gnored" } ], errors: [{ message: "'secondItem' is defined but never used", line: 1, column: 25 }] },
 
+        { code: "(function(obj) { var name, i = 0; for ( name in obj ) { i++; return; } })({});", errors: [{ message: "'name' is defined but never used", line: 1, column: 22 }] },
+
         // https://github.com/eslint/eslint/issues/3617
         {
             code: "\n/* global foobar, foo, bar */\nfoobar;",