Repository: jshint/jshint
Pull request #3424 (2019-08-18 01:08:08)

[[FIX]] Validate lone arrow function parameter
Previously, the identifier parsing logic returned early for a lone arrow
function parameter in order to avoid incorrectly extending the
environment record in which the associated arrow function was defined.
This unintentionally circumvented logic which enforced restrictions on
identifier names.

Refactor the parsing logic to enforce the restriction in all cases, but
only alter the environment record when the identifier is not a lone
arrow function parameter.

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

Coverage remained the same at 100.0% when pulling **129dde3589c205fd4b37ea24be432fe199984fda on jugglinmike:validate-arrow-lone-param** into **2366c92b852adeb963ecb812bc32b258288663dc on jshint:master**.


Patch:
diff --git a/.travis.yml b/.travis.yml
index 5d9805cf..825ee0c3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -9,16 +9,26 @@ before_install:
     fi
 script:
   - npm run pretest
-  - npm run test-node
-  - if [[ $(node --version) == v10* ]] ; then npm run test-262; fi
-  - if [[ $(node --version) == v10* ]] ; then npm run test-browser; fi
-node_js:
-  - "0.10"
-  - "0.12"
-  - 4
-  - 6
-  - 8
-  - 10
+  - npm run $CMD
+cache:
+  npm: false
 matrix:
+  include:
+    - node_js: "0.10"
+      env: CMD=test-node
+    - node_js: "0.12"
+      env: CMD=test-node
+    - node_js: 4
+      env: CMD=test-node
+    - node_js: 6
+      env: CMD=test-node
+    - node_js: 8
+      env: CMD=test-node
+    - node_js: 10
+      env: CMD=test-node
+    - node_js: 10
+      env: CMD=test-262
+    - node_js: 10
+      env: CMD=test-browser
   fast_finish: true
 sudo: false
diff --git a/src/jshint.js b/src/jshint.js
index baf95692..75f05e07 100644
--- a/src/jshint.js
+++ b/src/jshint.js
@@ -86,8 +86,6 @@ var JSHINT = (function() {
     membersOnly,
     predefined,    // Global variables defined by option
 
-    urls,
-
     extraModules = [],
     emitter = new events.EventEmitter();
 
@@ -2241,7 +2239,6 @@ var JSHINT = (function() {
 
     nud: function(context) {
       var v = this.value;
-
       // If this identifier is the lone parameter to a shorthand "fat arrow"
       // function definition, i.e.
       //
@@ -2250,15 +2247,14 @@ var JSHINT = (function() {
       // ...it should not be considered as a variable in the current scope. It
       // will be added to the scope of the new function when the next token is
       // parsed, so it can be safely ignored for now.
-      if (state.tokens.next.id === "=>") {
-        return this;
-      }
+      var isLoneArrowParam = state.tokens.next.id !== "=>";
 
       if (isReserved(context, this)) {
         warning("W024", this, v);
-      } else if (!state.funct["(comparray)"].check(v)) {
+      } else if (isLoneArrowParam && !state.funct["(comparray)"].check(v)) {
         state.funct["(scope)"].block.use(v, state.tokens.curr);
       }
+
       return this;
     },
 
@@ -2634,12 +2630,10 @@ var JSHINT = (function() {
     this.right = expression(context, 150);
 
     if (!this.right) { // '!' followed by nothing? Give up.
-      /* istanbul ignore next */
       quit("E041", this);
     }
 
     if (bang[this.right.id] === true) {
-      /* istanbul ignore next */
       warning("W018", this, "!");
     }
     return this;
@@ -2829,7 +2823,8 @@ var JSHINT = (function() {
       inGenerator = false;
       context &= ~prodParams.preAsync;
 
-      if (state.tokens.next.value === "static") {
+      if (state.tokens.next.value === "static" &&
+        !checkPunctuator(peek(), "(")) {
         isStatic = true;
         advance();
       }
@@ -3174,6 +3169,7 @@ var JSHINT = (function() {
     if (!exprs.length) {
       return;
     }
+
     if (exprs.length > 1) {
       ret = Object.create(state.syntax[","]);
       ret.exprs = exprs;
@@ -3220,6 +3216,8 @@ var JSHINT = (function() {
     }
 
     if (ret) {
+      ret.paren = true;
+
       // The operator may be necessary to override the default binding power of
       // neighboring operators (whenever there is an operator in use within the
       // first expression *or* the current group contains multiple expressions)
@@ -3233,8 +3231,6 @@ var JSHINT = (function() {
       if (!isNecessary) {
         warning("W126", opening);
       }
-
-      ret.paren = true;
     }
 
     return ret;
@@ -3865,19 +3861,37 @@ var JSHINT = (function() {
     state.funct["(metrics)"].ComplexityCount += 1;
   }
 
-  // Parse assignments that were found instead of conditionals.
-  // For example: if (a = 1) { ... }
-
+  /**
+   * Detect and warn about assignment that occurs within conditional clauses,
+   * e.g.
+   *
+   *     if (a = 1) { ... }
+   *
+   * This check is disabled in response to parenthesized expressions so that
+   * users may opt out of the warning on a case-by-case bases, e.g.
+   *
+   *     if ((a = 1)) { ... }
+   *
+   * @param {object} expr - the parsed expression within the conditional clause
+   */
   function checkCondAssignment(expr) {
-    var id, paren;
-    if (expr) {
+    if (!expr || expr.paren || state.option.boss) {
+      return;
+    }
+
+    var id = expr.id;
+
+    // If the expression is composed of multiple sub-expressions via a comma
+    // operator, then check the final sub-expression.
+    if (id === ",") {
+      expr = expr.exprs[expr.exprs.length - 1];
       id = expr.id;
-      paren = expr.paren;
-      if (id === "," && (expr = expr.exprs[expr.exprs.length - 1])) {
-        id = expr.id;
-        paren = paren || expr.paren;
+
+      if (expr.paren) {
+        return;
       }
     }
+
     switch (id) {
     case "=":
     case "+=":
@@ -3888,9 +3902,7 @@ var JSHINT = (function() {
     case "|=":
     case "^=":
     case "/=":
-      if (!paren && !state.option.boss) {
-        warning("W084");
-      }
+      warning("W084");
     }
   }
 
@@ -4372,11 +4384,13 @@ var JSHINT = (function() {
         }
         var id = state.tokens.prev;
         value = expression(context, 10);
-        if (value && value.identifier && value.value === "undefined") {
-          warning("W080", id, id.value);
-        }
-        if (!lone) {
-          destructuringPatternMatch(names, value);
+        if (value) {
+          if (value.identifier && value.value === "undefined") {
+            warning("W080", id, id.value);
+          }
+          if (!lone) {
+            destructuringPatternMatch(names, value);
+          }
         }
       }
 
@@ -4558,12 +4572,14 @@ var JSHINT = (function() {
         }
         id = state.tokens.prev;
         value = expression(context, 10);
-        if (value && !state.funct["(loopage)"] && value.identifier &&
-          value.value === "undefined") {
-          warning("W080", id, id.value);
-        }
-        if (!lone) {
-          destructuringPatternMatch(names, value);
+        if (value) {
+          if (!state.funct["(loopage)"] && value.identifier &&
+            value.value === "undefined") {
+            warning("W080", id, id.value);
+          }
+          if (!lone) {
+            destructuringPatternMatch(names, value);
+          }
         }
       }
 
@@ -4828,6 +4844,7 @@ var JSHINT = (function() {
     var t = state.tokens.next;
     var g = false;
     var noindent = false;
+    var seenCase = false;
 
     state.funct["(breakage)"] += 1;
     advance("(");
@@ -4843,8 +4860,6 @@ var JSHINT = (function() {
     if (!noindent)
       indent += state.option.indent;
 
-    this.cases = [];
-
     for (;;) {
       switch (state.tokens.next.id) {
       case "case":
@@ -4873,7 +4888,8 @@ var JSHINT = (function() {
         }
 
         advance("case");
-        this.cases.push(expression(context, 0));
+        expression(context, 0);
+        seenCase = true;
         increaseComplexityCount();
         g = true;
         advance(":");
@@ -4896,10 +4912,8 @@ var JSHINT = (function() {
         default:
           // Do not display a warning if 'default' is the first statement or if
           // there is a special /* falls through */ comment.
-          if (this.cases.length) {
-            if (!state.tokens.curr.caseFallsThrough) {
-              warning("W086", state.tokens.curr, "default");
-            }
+          if (seenCase && !state.tokens.curr.caseFallsThrough) {
+            warning("W086", state.tokens.curr, "default");
           }
         }
 
@@ -5227,7 +5241,7 @@ var JSHINT = (function() {
     if (!state.option.asi)
       nolinebreak(this);
 
-    if (state.tokens.next.id !== ";" && !state.tokens.next.reach &&
+    if (state.tokens.next.identifier &&
         state.tokens.curr.line === startLine(state.tokens.next)) {
       if (!state.funct["(scope)"].funct.hasLabel(v)) {
         warning("W090", state.tokens.next, v);
@@ -5255,7 +5269,7 @@ var JSHINT = (function() {
     if (!state.option.asi)
       nolinebreak(this);
 
-    if (state.tokens.next.id !== ";" && !state.tokens.next.reach) {
+    if (state.tokens.next.identifier) {
       if (state.tokens.curr.line === startLine(state.tokens.next)) {
         if (!state.funct["(scope)"].funct.hasLabel(v)) {
           warning("W090", state.tokens.next, v);
@@ -6336,7 +6350,6 @@ var JSHINT = (function() {
     });
 
     functions = [state.funct];
-    urls = [];
     member = {};
     membersOnly = null;
     inblock = false;
@@ -6534,11 +6547,6 @@ var JSHINT = (function() {
       data.implieds = impliedGlobals;
     }
 
-    if (urls.length > 0) {
-      /* istanbul ignore next */
-      data.urls = urls;
-    }
-
     globals = state.funct["(scope)"].getUsedOrDefinedGlobals();
     if (globals.length > 0) {
       data.globals = globals;


Test patch:
diff --git a/tests/test262/expectations.txt b/tests/test262/expectations.txt
index 0bf999d0..7abfde6a 100644
--- a/tests/test262/expectations.txt
+++ b/tests/test262/expectations.txt
@@ -267,9 +267,6 @@ test/language/expressions/arrow-function/scope-param-rest-elem-var-close.js(defa
 test/language/expressions/arrow-function/scope-param-rest-elem-var-open.js(default)
 test/language/expressions/arrow-function/syntax/arrowparameters-bindingidentifier-yield.js(default)
 test/language/expressions/arrow-function/syntax/arrowparameters-cover-formalparameters-yield.js(default)
-test/language/expressions/arrow-function/syntax/early-errors/arrowparameters-bindingidentifier-identifier-futurereservedword.js(default)
-test/language/expressions/arrow-function/syntax/early-errors/arrowparameters-bindingidentifier-identifier-futurereservedword.js(strict mode)
-test/language/expressions/arrow-function/syntax/early-errors/arrowparameters-bindingidentifier-identifier-strict-futurereservedword.js(strict mode)
 test/language/expressions/arrow-function/syntax/early-errors/asi-restriction-invalid-parenless-parameters-expression-body.js(default)
 test/language/expressions/arrow-function/syntax/early-errors/asi-restriction-invalid-parenless-parameters-expression-body.js(strict mode)
 test/language/expressions/arrow-function/syntax/early-errors/asi-restriction-invalid-parenless-parameters.js(default)
diff --git a/tests/unit/parser.js b/tests/unit/parser.js
index b3fd37b4..9ba5099b 100644
--- a/tests/unit/parser.js
+++ b/tests/unit/parser.js
@@ -280,6 +280,53 @@ exports.relations = function (test) {
   run.test(code, {esnext: true});
   run.test(code, {moz: true});
 
+  TestRun(test, "No suitable expression following logical NOT.")
+    .addError(1, 7, "Expected an identifier and instead saw ';'.")
+    .addError(1, 6, "Unrecoverable syntax error. (100% scanned).")
+    .test("void !;");
+
+  TestRun(test, "Logical NOT in combination with 'infix' operators.")
+    .addError(3, 6, "Confusing use of '!'.")
+    .addError(4, 6, "Confusing use of '!'.")
+    .addError(5, 6, "Confusing use of '!'.")
+    .addError(6, 6, "Confusing use of '!'.")
+    .addError(7, 6, "Confusing use of '!'.")
+    .addError(8, 6, "Confusing use of '!'.")
+    .addError(9, 6, "Confusing use of '!'.")
+    .addError(10, 6, "Confusing use of '!'.")
+    .addError(11, 6, "Confusing use of '!'.")
+    .addError(12, 6, "Confusing use of '!'.")
+    .addError(13, 6, "Confusing use of '!'.")
+    .addError(14, 6, "Confusing use of '!'.")
+    .addError(15, 6, "Confusing use of '!'.")
+    .test([
+      "void !'-';",
+      "void !'+';",
+      "void !(0 < 0);",
+      "void !(0 <= 0);",
+      "void !(0 == 0);",
+      "void !(0 === 0);",
+      "void !(0 !== 0);",
+      "void !(0 != 0);",
+      "void !(0 > 0);",
+      "void !(0 >= 0);",
+      "void !(0 + 0);",
+      "void !(0 - 0);",
+      "void !(0 * 0);",
+      "void !(0 / 0);",
+      "void !(0 % 0);",
+    ]);
+
+  TestRun(test, "Logical NOT in combination with other unary operators.")
+    .addError(3, 6, "Confusing use of '!'.")
+    .addError(4, 6, "Confusing use of '!'.")
+    .test([
+      "void !'-';",
+      "void !'+';",
+      "void !+0;",
+      "void !-0;"
+    ]);
+
   test.done();
 };
 
@@ -2493,6 +2540,25 @@ exports["destructuring assignment of valid simple assignment targets"] = functio
   test.done();
 };
 
+exports["regression test for GH-3408"] = function (test) {
+  TestRun(test, "var statement")
+    .addError(1, 9, "Expected an identifier and instead saw ';'.")
+    .addError(1, 10, "Missing semicolon.")
+    .test("var [x]=;", { esversion: 6 });
+
+  TestRun(test, "let statement")
+    .addError(1, 9, "Expected an identifier and instead saw ';'.")
+    .addError(1, 10, "Missing semicolon.")
+    .test("let [x]=;", { esversion: 6 });
+
+  TestRun(test, "const statement")
+    .addError(1, 11, "Expected an identifier and instead saw ';'.")
+    .addError(1, 12, "Missing semicolon.")
+    .test("const [x]=;", { esversion: 6 });
+
+  test.done();
+};
+
 exports["non-identifier PropertyNames in object destructuring"] = function (test) {
   var code = [
     "var { ['x' + 2]: a = 3, 0: b } = { x2: 1, 0: 2 };",
@@ -6825,6 +6891,28 @@ exports["class and method naming"] = function (test) {
       "};"
     ], {esversion: 6});
 
+  TestRun(test, "valid uses of name `static`")
+    .test([
+      "void class {",
+      "  static() {}",
+      "  static static() {}",
+      "  static ['static']() {}",
+      "};",
+      "void class {",
+      "  * static() { yield; }",
+      "  static * static() { yield; }",
+      "  static * ['static']() { yield; }",
+      "};",
+      "void class {",
+      "  get static() {}",
+      "  set static(x) {}",
+      "  static get static() {}",
+      "  static set static(x) {}",
+      "  static get ['static']() {}",
+      "  static set ['static'](x) {}",
+      "};"
+    ], {esversion: 6});
+
   TestRun(test, "invalid use of name `prototype`: static method")
     .addError(2, 10, "A static class method cannot be named 'prototype'.")
     .test([
@@ -7420,10 +7508,20 @@ exports["test for GH-1105"] = function (test) {
     "}"
   ];
 
-  var run = TestRun(test)
-    .addError(2, 22, "Missing semicolon.");
+  TestRun(test)
+    .addError(2, 22, "Missing semicolon.")
+    .test(code);
+
+  code = [
+    "while (true) {",
+    "    if (true) { continue }",
+    "}"
+  ];
+
+  TestRun(test)
+    .addError(2, 25, "Missing semicolon.")
+    .test(code);
 
-  run.test(code);
   test.done();
 };