[[FIX]] Report loopfunc for all function types

Previously only function expressions caused loopfunc, meaning arrow
functions that captured variables were not warned.
Fixes jshint#2153

src/jshint.js

@@ -2939,7 +2939,7 @@ var JSHINT = (function() {
    *                                           the body of member functions.
    */
   function doFunction(options) {
-    var f, name, statement, classExprBinding, isGenerator, isArrow;
+    var f, token, name, statement, classExprBinding, isGenerator, isArrow, ignoreLoopFunc;
     var oldOption = state.option;
     var oldIgnored = state.ignored;
 
@@ -2949,6 +2949,7 @@ var JSHINT = (function() {
       classExprBinding = options.classExprBinding;
       isGenerator = options.type === "generator";
       isArrow = options.type === "arrow";
+      ignoreLoopFunc = options.ignoreLoopFunc;
     }
 
     state.option = Object.create(state.option);
@@ -2962,7 +2963,8 @@ var JSHINT = (function() {
     });
 
     f = state.funct;
-    state.tokens.curr.funct = state.funct;
+    token = state.tokens.curr;
+    token.funct = state.funct;
 
     functions.push(state.funct);
 
@@ -3024,6 +3026,15 @@ var JSHINT = (function() {
 
     state.funct = state.funct["(context)"];
 
+    if (!ignoreLoopFunc && !state.option.loopfunc && state.funct["(loopage)"]) {
+      // If the function we just parsed accesses any non-local variables
+      // trigger a warning. Otherwise, the function is safe even within
+      // a loop.
+      if (f["(isCapturing)"]) {
+        warning("W083", token);
+      }
+    }
+
     return f;
   }
 
@@ -3769,7 +3780,6 @@ var JSHINT = (function() {
     }
     if (inblock) {
       warning("W082", state.tokens.curr);
-
     }
     var i = optionalidentifier();
 
@@ -3786,7 +3796,8 @@ var JSHINT = (function() {
     doFunction({
       name: i,
       statement: this,
-      type: generator ? "generator" : null
+      type: generator ? "generator" : null,
+      ignoreLoopFunc: inblock // a declaration may already have warned
     });
     if (state.tokens.next.id === "(" && state.tokens.next.line === state.tokens.curr.line) {
       error("E039");
@@ -3806,16 +3817,7 @@ var JSHINT = (function() {
     }
 
     var i = optionalidentifier();
-    var fn = doFunction({ name: i, type: generator ? "generator" : null });
-
-    if (!state.option.loopfunc && state.funct["(loopage)"]) {
-      // If the function we just parsed accesses any non-local variables
-      // trigger a warning. Otherwise, the function is safe even within
-      // a loop.
-      if (fn["(isCapturing)"]) {
-        warning("W083");
-      }
-    }
+    doFunction({ name: i, type: generator ? "generator" : null });
     return this;
   });
 

tests/unit/fixtures/loopfunc.js

@@ -31,5 +31,13 @@ while (true) {
 }
 
 for (i = 0; i < 5; i++) {
-  c = function(a,b,i) { return i; };
+  c = function (a,b,i) { return i; };
+}
+
+for (i = 0; i < 5; i++) {
+  c = function (a,b,i) { function d() { return i; } return d(); };
+}
+
+for (i = 0; i < 5; i++) {
+  c = function (a,b,c) { function d() { return i; } return d(); };
 }

tests/unit/options.js

@@ -1200,6 +1200,7 @@ exports.loopfunc = function (test) {
     .addError(25, "Don't make functions within a loop.")
     .addError(12, "Function declarations should not be placed in blocks. Use a function " +
             "expression or move the statement to the top of the outer function.")
+    .addError(42, "Don't make functions within a loop.")
     .test(src, {es3: true});
 
   // When loopfunc is true, only function declaration should fail.
@@ -1209,6 +1210,34 @@ exports.loopfunc = function (test) {
             "expression or move the statement to the top of the outer function.")
     .test(src, { es3: true, loopfunc: true });
 
+  var es6LoopFuncSrc = [
+    "for (var i = 0; i < 5; i++) {",
+    "  var y = w => i;",
+    "}",
+    "for (i = 0; i < 5; i++) {",
+    "  var z = () => i;",
+    "}",
+    "for (i = 0; i < 5; i++) {",
+    "  y = i => i;", // not capturing
+    "}",
+    "for (i = 0; i < 5; i++) {",
+    "  y = { a() { return i; } };",
+    "}",
+    "for (i = 0; i < 5; i++) {",
+    "  y = class { constructor() { this.i = i; }};",
+    "}",
+    "for (i = 0; i < 5; i++) {",
+    "  y = { a() { return () => i; } };",
+    "}"
+  ];
+  TestRun(test)
+    .addError(2, "Don't make functions within a loop.")
+    .addError(5, "Don't make functions within a loop.")
+    .addError(11, "Don't make functions within a loop.")
+    .addError(14, "Don't make functions within a loop.")
+    .addError(17, "Don't make functions within a loop.")
+    .test(es6LoopFuncSrc, {esnext: true});
+
   test.done();
 };