Repository: jshint/jshint
Pull request #3419 (2019-07-28 02:35:13)

Re-implement "comma fixes"
@rwaldron @caitp Writing something truly distinct was a little tough for this small patch, but the small size fortunately also makes originality less critical. I'm hoping that my discovery and correction of a previously-unreported bug lends some weight to the novelty of the new implementation.

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

Coverage remained the same at 100.0% when pulling **15665577892c2ca1746290a8b91874b7c174bde5 on jugglinmike:revert-parser-fixes-5** into **2366c92b852adeb963ecb812bc32b258288663dc on jshint:master**.

caitp: other than the ASI handling question, lgtm

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..5b3bc884 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();
 
@@ -2634,12 +2632,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 +2825,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 +3171,7 @@ var JSHINT = (function() {
     if (!exprs.length) {
       return;
     }
+
     if (exprs.length > 1) {
       ret = Object.create(state.syntax[","]);
       ret.exprs = exprs;
@@ -3220,6 +3218,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 +3233,6 @@ var JSHINT = (function() {
       if (!isNecessary) {
         warning("W126", opening);
       }
-
-      ret.paren = true;
     }
 
     return ret;
@@ -3865,19 +3863,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 +3904,7 @@ var JSHINT = (function() {
     case "|=":
     case "^=":
     case "/=":
-      if (!paren && !state.option.boss) {
-        warning("W084");
-      }
+      warning("W084");
     }
   }
 
@@ -4372,11 +4386,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 +4574,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 +4846,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 +4862,6 @@ var JSHINT = (function() {
     if (!noindent)
       indent += state.option.indent;
 
-    this.cases = [];
-
     for (;;) {
       switch (state.tokens.next.id) {
       case "case":
@@ -4873,7 +4890,8 @@ var JSHINT = (function() {
         }
 
         advance("case");
-        this.cases.push(expression(context, 0));
+        expression(context, 0);
+        seenCase = true;
         increaseComplexityCount();
         g = true;
         advance(":");
@@ -4896,10 +4914,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 +5243,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 +5271,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 +6352,6 @@ var JSHINT = (function() {
     });
 
     functions = [state.funct];
-    urls = [];
     member = {};
     membersOnly = null;
     inblock = false;
@@ -6534,11 +6549,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/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();
 };