Repository: jshint/jshint
Pull request #3209 (2017-11-12 20:43:53)

[[TEST]] Refactor negative syntax tests for "rest"
Validate JSHint's behavior for various invalid applications of the
"rest" operator in isolation. In the previous factoring, the independent
syntax tests interacted and caused JSHint to report errors that differ
from those it would report when encountering each form in isolation.
Although JSHint makes a best-effort approach to recovering from invalid
syntax, this behavior is not formalized and should therefor not be
asserted by unit tests.

This factoring avoids the undesirable interaction, allowing the tests to
document JSHint's behavior under more predictable conditions. It also
highlights a previously-existing bug in the errors reported (as
documented via in-line code comments).

---

@rwaldron Mind taking a look? This is pretty minor, but it will help me while rebasing the `v2.10.0` branch (this commit in particular 8b1bbf38cfe4690e810db6c7bbc591575abffbfe).

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

Coverage remained the same at 98.743% when pulling **05db8e2981fad94ea12220aaa8e4d64c5ca673b5 on jugglinmike:refactor-rest-tests** into **a11d6310c5448304d56be081dc02b4c2a1ff7377 on jshint:master**.

rwaldron: > In the previous factoring, the independent syntax tests interacted and caused JSHint to report errors that differ from those it would report when encountering each form in isolation.

This is excellent!

Travis is failing in Node.js 0.10.x and 0.12.x because a nested dependency is using `const` :\



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..886e6f2f 100644
--- a/src/lex.js
+++ b/src/lex.js
@@ -1514,7 +1514,7 @@ Lexer.prototype = {
     this.from = this.char;
 
     // Move to the next non-space character.
-    while (/\s/.test(this.peek())) {
+    while (reg.whitespace.test(this.peek())) {
       this.from += 1;
       this.skip();
     }
@@ -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/reg.js b/src/reg.js
index f51eb127..9fc9bb65 100644
--- a/src/reg.js
+++ b/src/reg.js
@@ -36,3 +36,11 @@ exports.fallsThrough = /^\s*falls?\sthrough\s*$/;
 // very conservative rule (eg: only one space between the start of the comment and the first character)
 // to relax the maxlen option
 exports.maxlenException = /^(?:(?:\/\/|\/\*|\*) ?)?[^ ]+$/;
+
+// Node.js releases prior to version 8 include a version of the V8 engine which
+// incorrectly interprets the character class escape `\s`. The following
+// regular expression may be replaced with `/\s/` when JSHint removes support
+// for Node.js versions prior to 8.
+// Source:
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
+exports.whitespace = /[ \f\n\r\t\v\u00a0\u1680\u2000-\u200a\u2028\u2029\u202f\u205f\u3000\ufeff]/;
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/test262/expectations.txt b/tests/test262/expectations.txt
index 4d629dba..786d418b 100644
--- a/tests/test262/expectations.txt
+++ b/tests/test262/expectations.txt
@@ -288,7 +288,6 @@ language/identifiers/vals-rus-alpha-upper-via-escape-hex.js
 language/rest-parameters/array-pattern.js
 language/rest-parameters/no-alias-arguments.js
 language/rest-parameters/object-pattern.js
-language/white-space/mongolian-vowel-separator.js
 language/arguments-object/10.5-7-b-2-s.js
 language/arguments-object/10.5-7-b-3-s.js
 language/arguments-object/10.6-10-c-ii-2.js
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..e4ee8832 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
@@ -7667,41 +7668,49 @@ exports.commaAfterRestParameter = function (test) {
 
 
 exports.extraRestOperator = function (test) {
-  var code = [
-    'function fn([a, b, ......c]) { }',
-    'function fn2([......c]) { }',
-    'function fn3(a, b, ......) { }',
-    'function fn4(......) { }',
-    'var [......a] = [1, 2, 3];',
-    'var [a, b, ... ...c] = [1, 2, 3];',
-    'var arrow = (......a) => a;',
-    'var arrow2 = (a, b, ......c) => c;',
-    'var arrow3 = ([......a]) => a;',
-    'var arrow4 = ([a, b, ......c]) => c;',
-    'fn([1, 2, 3]);',
-    'fn2([1, 2, 3]);',
-    'fn3(1, 2, 3);',
-    'fn4(1, 2, 3);',
-    'arrow(1, 2, 3);',
-    'arrow2(1, 2, 3);',
-    'arrow3([1, 2, 3]);',
-    'arrow4([1, 2, 3]);',
-  ];
+  TestRun(test)
+    .addError(1, 23, "Unexpected '...'.")
+    .test('function fn([a, b, ......c]) { }', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 18, "Unexpected '...'.")
+    .test('function fn2([......c]) { }', { esnext: true });
 
   TestRun(test)
     .addError(1, 23, "Unexpected '...'.")
-    .addError(2, 18, "Unexpected '...'.")
-    .addError(3, 23, "Unexpected '...'.")
-    .addError(3, 23, "Unexpected ')'.")
-    .addError(4, 17, "Unexpected '...'.")
-    .addError(4, 17, "Unexpected ')'.")
-    .addError(5, 9, "Unexpected '...'.")
-    .addError(6, 16, "Unexpected '...'.")
-    .addError(7, 17, "Unexpected '...'.")
-    .addError(8, 24, "Unexpected '...'.")
-    .addError(9, 19, "Unexpected '...'.")
-    .addError(10, 25, "Unexpected '...'.")
-    .test(code, { esnext: true });
+    // The reported column number for this parsing error is incorrect.
+    .addError(1, 23, "Unexpected ')'.")
+    .test('function fn3(a, b, ......) { }', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 17, "Unexpected '...'.")
+    // The reported column number for this parsing error is incorrect.
+    .addError(1, 17, "Unexpected ')'.")
+    .test('function fn4(......) { }', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 9, "Unexpected '...'.")
+    .test('var [......a] = [1, 2, 3];', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 16, "Unexpected '...'.")
+    .test('var [a, b, ... ...c] = [1, 2, 3];', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 17, "Unexpected '...'.")
+    .test('var arrow = (......a) => a;', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 24, "Unexpected '...'.")
+    .test('var arrow2 = (a, b, ......c) => c;', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 19, "Unexpected '...'.")
+    .test('var arrow3 = ([......a]) => a;', { esnext: true });
+
+  TestRun(test)
+    .addError(1, 25, "Unexpected '...'.")
+    .test('var arrow4 = ([a, b, ......c]) => c;', { esnext: true });
 
   test.done();
 };