Repository: jshint/jshint
Pull request #3191 (2017-09-16 18:56:32)

[[FIX]] Consistently ignore dot-prefixed dirs
When the Node.js CLI is invoked with a path to an input file, that file
should not be linted if it matches any pattern in a "ignore list"
configuration file.

Prior to this patch, JSHint failed to ignore such files if one of their
parent directories was prefixed with a period character.

Update the CLI's usage of the `minimatch` Node.js library to also
consider such "dot-prefixed" directories when interpreting ignore lists.

Note that this change is backwards-compatible because it limits the
situations in which the CLI reports an error.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/13302269/badge)](https://coveralls.io/builds/13302269)

Coverage remained the same at 98.744% when pulling **30eb670bed09803b24c7882e8f68d9a364dd6e22 on jugglinmike:gh-3187-ignore-dot-prefixed** into **1553e3af6d2453eb52507a2c97971094897794d3 on jshint:master**.

rwaldron: @jugglinmike take a look at appveyor when you have a chance
jugglinmike: @rwaldron I diagnosed the problem shortly after submitting this patch. It's a Node.js version 0.10 compatibility issue in one of our dependencies. I've offered the maintainers a fix here:

https://github.com/maxogden/extract-zip/pull/51

No response yet, though. Sorry for not mentioning that sooner!
rwaldron: Should we hold off until that's resolved?
rwaldron: (Node.js 0.10 is long unsupported by Node.js)
jugglinmike: > Should we hold off until that's resolved?

Yeah, let's give it a few more days. Unfortunately, this is going to fail on
all builds until we resolve the problem or delete the "Windows/Node 0.10" job.

> (Node.js 0.10 is long unsupported by Node.js)

That occurred to me, too. As I wrote in the pull request description, I was
hoping that since supporting that runtime was trivial enough, maybe the
maintainers wouldn't mind taking it on.

Patch:
diff --git a/package.json b/package.json
index 04367250..bc5f6c2d 100644
--- a/package.json
+++ b/package.json
@@ -66,13 +66,16 @@
     "jscs":                           "1.11.x",
     "mock-stdin":                     "0.3.x",
     "nodeunit":                       "0.9.x",
-    "phantom":                        "~4.0.1",
-    "phantomjs-prebuilt":             "~2.1.7",
     "regenerate":                     "1.2.x",
     "sinon":                          "1.12.x",
     "unicode-6.3.0":                  "0.1.x"
   },
 
+  "optionalDependencies": {
+    "phantom":                        "~4.0.1",
+    "phantomjs-prebuilt":             "~2.1.7"
+  },
+
   "license": "(MIT AND JSON)",
 
   "preferGlobal": true,
diff --git a/src/cli.js b/src/cli.js
index 0a5258b8..7933ca67 100644
--- a/src/cli.js
+++ b/src/cli.js
@@ -230,7 +230,7 @@ function loadIgnores(params) {
  */
 function isIgnored(fp, patterns) {
   return patterns.some(function(ip) {
-    if (minimatch(path.resolve(fp), ip, { nocase: true })) {
+    if (minimatch(path.resolve(fp), ip, { nocase: true, dot: true })) {
       return true;
     }
 
diff --git a/src/jshint.js b/src/jshint.js
index 463bd129..fb4a18f2 100644
--- a/src/jshint.js
+++ b/src/jshint.js
@@ -3059,8 +3059,8 @@ var JSHINT = (function() {
       // 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);
+      if (f["(outerMutables)"]) {
+        warning("W083", token, f["(outerMutables)"].join(", "));
       }
     }
 
diff --git a/src/lex.js b/src/lex.js
index ade8c102..b620420d 100644
--- a/src/lex.js
+++ b/src/lex.js
@@ -1703,7 +1703,7 @@ Lexer.prototype = {
 
       if (type === "(identifier)") {
         if (value === "return" || value === "case" || value === "yield" ||
-            value === "typeof" || value === "instanceof") {
+            value === "typeof" || value === "instanceof" || value === "void") {
           this.prereg = true;
         }
 
diff --git a/src/messages.js b/src/messages.js
index 665e71da..f9824f0f 100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -168,7 +168,7 @@ var warnings = {
     "Use a function expression or move the statement to the top of " +
     "the outer function.",
   W083: "Functions declared within loops referencing an outer scoped " +
-    "variable may lead to confusing semantics.",
+    "variable may lead to confusing semantics. ({a})",
   W084: "Expected a conditional expression and instead saw an assignment.",
   W085: "Don't use 'with'.",
   W086: "Expected a 'break' statement before '{a}'.",
diff --git a/src/scope-manager.js b/src/scope-manager.js
index ff566430..ac11e976 100644
--- a/src/scope-manager.js
+++ b/src/scope-manager.js
@@ -315,7 +315,10 @@ var scopeManager = function(state, predefined, exported, declared) {
           isImmutable = labelType === "const" ||
             (labelType === null && _scopeStack[0]["(predefined)"][usedLabelName] === false);
           if (isUnstackingFunctionOuter && !isImmutable) {
-            state.funct["(isCapturing)"] = true;
+            if (!state.funct["(outerMutables)"]) {
+              state.funct["(outerMutables)"] = [];
+            }
+            state.funct["(outerMutables)"].push(usedLabelName);
           }
 
           // not exiting the global scope, so copy the usage down in case its an out of scope usage


Test patch:
diff --git a/tests/browser.js b/tests/browser.js
index 2a2a1376..3eb5693f 100644
--- a/tests/browser.js
+++ b/tests/browser.js
@@ -1,13 +1,24 @@
 "use strict";
 
-var phantom = require("phantom");
+var phantom, phantomJsPrebuilt;
+try {
+  phantom = require("phantom");
+  phantomJsPrebuilt = require("phantomjs-prebuilt");
+} catch (err) {
+  throw new Error(
+    "Unable to run tests in PhantomJS because the required dependencies are " +
+    "not available. Please note that JSHint does not support development " +
+    "using versions of Node.js which are no longer maintained."
+  );
+}
+
 var createTestServer = require("./helpers/browser/server");
 var options = {
   /**
    * The `phantom` module provides a Node.js API for the PhantomJS binary,
    * while the `phantomjs` module includes the binary itself.
    */
-  phantomPath: require("phantomjs-prebuilt").path
+  phantomPath: phantomJsPrebuilt.path
 };
 var port = process.env.NODE_PORT || 8045;
 var ph;
diff --git a/tests/cli.js b/tests/cli.js
index 573df403..f3e40703 100644
--- a/tests/cli.js
+++ b/tests/cli.js
@@ -658,6 +658,30 @@ exports.group = {
     test.done();
   },
 
+  // See gh-3187
+  testIgnoreWithDot: function (test) {
+    var dir = __dirname + "/../examples/";
+    this.sinon.stub(process, "cwd").returns(dir);
+
+    this.sinon.stub(shjs, "cat")
+      .withArgs(sinon.match(/file\.js$/)).returns("This is not Javascript.")
+      .withArgs(sinon.match(/\.jshintignore$/)).returns("**/ignored-dir/**");
+    this.sinon.stub(shjs, "test")
+      .withArgs("-e", sinon.match(/file\.js$/)).returns(true)
+      .withArgs("-e", sinon.match(/\.jshintignore$/)).returns(true);
+
+    cli.interpret([
+      "node", "jshint", "ignored-dir/.dot-prefixed/file.js",
+      "ignored-dir/not-dot-prefixed/file.js"
+    ]);
+
+    process.cwd.returns(__dirname + "/../");
+
+    test.equal(cli.exit.args[0][0], 0, "All matching files are ignored, regardless of dot-prefixed directories.");
+
+    test.done();
+  },
+
   testExcludePath: function (test) {
     var run = this.sinon.stub(cli, "run");
     var dir = __dirname + "/../examples/";
diff --git a/tests/unit/options.js b/tests/unit/options.js
index 20ec7071..788f35ab 100644
--- a/tests/unit/options.js
+++ b/tests/unit/options.js
@@ -1390,13 +1390,13 @@ exports.loopfunc = function (test) {
 
   // By default, not functions are allowed inside loops
   TestRun(test)
-    .addError(4, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(8, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(20, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(25, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(4, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (v)")
+    .addError(8, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (v)")
+    .addError(20, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (nonExistent)")
+    .addError(25, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (p)")
     .addError(12, 5, "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, 7, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(42, 7, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
     .test(src, {es3: true});
 
   // When loopfunc is true, only function declaration should fail.
@@ -1427,11 +1427,11 @@ exports.loopfunc = function (test) {
     "}"
   ];
   TestRun(test)
-    .addError(2, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(5, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(11, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(14, 15, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(17, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(2, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
+    .addError(5, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
+    .addError(11, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
+    .addError(14, 15, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
+    .addError(17, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
     .test(es6LoopFuncSrc, {esnext: true});
 
   // functions declared in the expressions that loop should warn
@@ -1442,13 +1442,13 @@ exports.loopfunc = function (test) {
     "for(var c = function(){return j;};;){c();}"];
 
   TestRun(test)
-    .addError(1, 25, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(3, 16, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(1, 25, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
+    .addError(3, 16, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (j)")
     .test(src2, { es3: true, loopfunc: false, boss: true });
 
   TestRun(test, "Allows closing over immutable bindings (ES5)")
-    .addError(6, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(7, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(6, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (outerVar)")
+    .addError(7, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (innerVar)")
     .test([
         "var outerVar;",
         "",
@@ -1463,8 +1463,8 @@ exports.loopfunc = function (test) {
       ]);
 
   TestRun(test, "Allows closing over immutable bindings (globals)")
-    .addError(8, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(15, 10, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(8, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (mutableGlobal)")
+    .addError(15, 10, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (immutableGlobal)")
     .test([
         "/* globals immutableGlobal: false, mutableGlobal: true */",
         "while (false) {",
@@ -1486,10 +1486,10 @@ exports.loopfunc = function (test) {
       ]);
 
   TestRun(test, "Allows closing over immutable bindings (ES2015)")
-    .addError(10, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(11, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(18, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
-    .addError(19, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(10, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (outerLet)")
+    .addError(11, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (innerLet)")
+    .addError(18, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (OuterClass)")
+    .addError(19, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (InnerClass)")
     .test([
         "let outerLet;",
         "const outerConst = 0;",
@@ -1514,6 +1514,17 @@ exports.loopfunc = function (test) {
         "}"
       ], { esversion: 2015 });
 
+  TestRun(test, "W083 lists multiple outer scope variables")
+    .addError(3, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (a, b)")
+    .test([
+        "var a, b;",
+        "for (;;) {",
+        "  var f = function() {",
+        "    return a + b;",
+        "  };",
+        "}"
+      ]);
+
   test.done();
 };
 
diff --git a/tests/unit/parser.js b/tests/unit/parser.js
index 55f602fb..b3158bf6 100644
--- a/tests/unit/parser.js
+++ b/tests/unit/parser.js
@@ -557,6 +557,7 @@ exports.regexp = function (test) {
     "var x = w - /s/;",
     "var y = typeof /[a-z]/;", // GH-657
     "var z = /a/ instanceof /a/.constructor;", // GH-2773
+    "void /./;",
     "var v = /dsdg;"
   ];
 
@@ -576,8 +577,8 @@ exports.regexp = function (test) {
     .addError(17, 9, "Invalid regular expression.")
     .addError(20, 9, "Invalid regular expression.")
     .addError(21, 9, "Invalid regular expression.")
-    .addError(28, 9, "Unclosed regular expression.")
-    .addError(28, 9, "Unrecoverable syntax error. (100% scanned).");
+    .addError(29, 9, "Unclosed regular expression.")
+    .addError(29, 9, "Unrecoverable syntax error. (100% scanned).");
 
   run.test(code, {es3: true});
   run.test(code, {}); // es5