Repository: jshint/jshint (js)

Issue:
Comparison operators `==`/`!=` reported too aggressively
The `eqeqeq` option's description states, "This options prohibits the use of ==
and != in favor of `===` and `!==`." Enabling it will cause JSHint to issue
warning W118 for offending code. However, JSHint will issue a separate warning
(W041) if either side of the comparison is a "falsey" literal value,
*regardless of the value of the `eqeqeq` option*.

**If you are a contributor looking to submit a fix for this bug**, remove W041
altogether. This is a shame because it makes JSHint less strict by default, but
the current behavior is inconsistent and confusing. And while the warning
message for W041 is far more descriptive (and therefore a better choice to keep
around), we can't change W118 in-place outside of a major version release. This
should be relatively straightforward, so I'm marking this issue as a "good
first bug."

**If you are a user struggling with this bug today**, disable W041 in your
project configuration, either with an in-line directive:

```js
// jshint -W041
```

Or within a JSHint configuration file:

```js
{
  "-W041": true
}
```

Pull request description:
[[FIX]] Removed warning message W041
This commit removes the warning message W041 altogether from the jshint
project.

Fixes #3109

Patch:
diff --git a/src/jshint.js b/src/jshint.js
index 09703f95..7c92cd84 100644
--- a/src/jshint.js
+++ b/src/jshint.js
@@ -191,7 +191,10 @@ var JSHINT = (function() {
     }
   }
 
-  function assume() {
+  /**
+   * Apply all linting options according to the status of the `state` object.
+   */
+  function applyOptions() {
     var badESOpt = null;
     processenforceall();
 
@@ -417,12 +420,23 @@ var JSHINT = (function() {
     });
   }
 
-  function doOption() {
-    var nt = state.tokens.next;
-    var body = nt.body.split(",").map(function(s) { return s.trim(); });
-
+  /**
+   * Process an inline linting directive
+   *
+   * @param {Token} directiveToken - the directive-bearing comment token
+   * @param {Token} previous - the token that preceeds the directive
+   */
+  function lintingDirective(directiveToken, previous) {
+    var body = directiveToken.body.split(",")
+      .map(function(s) { return s.trim(); });
     var predef = {};
-    if (nt.type === "globals") {
+
+    if (directiveToken.type === "falls through") {
+      previous.caseFallsThrough = true;
+      return;
+    }
+
+    if (directiveToken.type === "globals") {
       body.forEach(function(g, idx) {
         g = g.split(":");
         var key = (g[0] || "").trim();
@@ -433,7 +447,7 @@ var JSHINT = (function() {
           if (idx > 0 && idx === body.length - 1) {
             return;
           }
-          error("E002", nt);
+          error("E002", directiveToken);
           return;
         }
 
@@ -452,19 +466,19 @@ var JSHINT = (function() {
 
       for (var key in predef) {
         if (_.has(predef, key)) {
-          declared[key] = nt;
+          declared[key] = directiveToken;
         }
       }
     }
 
-    if (nt.type === "exported") {
+    if (directiveToken.type === "exported") {
       body.forEach(function(e, idx) {
         if (!e.length) {
           // Ignore trailing comma
           if (idx > 0 && idx === body.length - 1) {
             return;
           }
-          error("E002", nt);
+          error("E002", directiveToken);
           return;
         }
 
@@ -472,7 +486,7 @@ var JSHINT = (function() {
       });
     }
 
-    if (nt.type === "members") {
+    if (directiveToken.type === "members") {
       membersOnly = membersOnly || {};
 
       body.forEach(function(m) {
@@ -499,13 +513,13 @@ var JSHINT = (function() {
       "indent"
     ];
 
-    if (nt.type === "jshint" || nt.type === "jslint") {
+    if (directiveToken.type === "jshint" || directiveToken.type === "jslint") {
       body.forEach(function(g) {
         g = g.split(":");
         var key = (g[0] || "").trim();
         var val = (g[1] || "").trim();
 
-        if (!checkOption(key, nt)) {
+        if (!checkOption(key, directiveToken)) {
           return;
         }
 
@@ -515,7 +529,7 @@ var JSHINT = (function() {
             val = +val;
 
             if (typeof val !== "number" || !isFinite(val) || val <= 0 || Math.floor(val) !== val) {
-              error("E032", nt, g[1].trim());
+              error("E032", directiveToken, g[1].trim());
               return;
             }
 
@@ -534,7 +548,7 @@ var JSHINT = (function() {
             return void error("E009");
 
           if (val !== "true" && val !== "false")
-            return void error("E002", nt);
+            return void error("E002", directiveToken);
 
           state.option.validthis = (val === "true");
           return;
@@ -551,7 +565,7 @@ var JSHINT = (function() {
             state.option.quotmark = val;
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           return;
         }
@@ -569,7 +583,7 @@ var JSHINT = (function() {
             state.option.shadow = "inner";
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           return;
         }
@@ -587,7 +601,7 @@ var JSHINT = (function() {
             state.option.unused = val;
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           return;
         }
@@ -604,7 +618,7 @@ var JSHINT = (function() {
             state.option.latedef = "nofunc";
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           return;
         }
@@ -612,11 +626,11 @@ var JSHINT = (function() {
         if (key === "ignore") {
           switch (val) {
           case "line":
-            state.ignoredLines[nt.line] = true;
+            state.ignoredLines[directiveToken.line] = true;
             removeIgnoredMessages();
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           return;
         }
@@ -634,7 +648,7 @@ var JSHINT = (function() {
             state.option.strict = val;
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           return;
         }
@@ -644,7 +658,7 @@ var JSHINT = (function() {
            * TODO: Extend this restriction to *all* "environmental" options.
            */
           if (!hasParsedCode(state.funct)) {
-            error("E055", state.tokens.next, "module");
+            error("E055", directiveToken, "module");
           }
         }
 
@@ -653,18 +667,15 @@ var JSHINT = (function() {
           case "3":
           case "5":
           case "6":
-            state.option.moz = false;
-            state.option.esversion = +val;
-            break;
           case "2015":
             state.option.moz = false;
-            state.option.esversion = 6;
+            state.option.esversion = +val;
             break;
           default:
-            error("E002", nt);
+            error("E002", directiveToken);
           }
           if (!hasParsedCode(state.funct)) {
-            error("E055", state.tokens.next, "esversion");
+            error("E055", directiveToken, "esversion");
           }
           return;
         }
@@ -678,7 +689,7 @@ var JSHINT = (function() {
 
         var tn;
         if (val === "true" || val === "false") {
-          if (nt.type === "jslint") {
+          if (directiveToken.type === "jslint") {
             tn = options.renamed[key] || key;
             state.option[tn] = (val === "true");
 
@@ -692,10 +703,10 @@ var JSHINT = (function() {
           return;
         }
 
-        error("E002", nt);
+        error("E002", directiveToken);
       });
 
-      assume();
+      applyOptions();
     }
   }
 
@@ -789,11 +800,7 @@ var JSHINT = (function() {
       }
 
       if (state.tokens.next.isSpecial) {
-        if (state.tokens.next.type === "falls through") {
-          state.tokens.curr.caseFallsThrough = true;
-        } else {
-          doOption();
-        }
+        lintingDirective(state.tokens.next, state.tokens.curr);
       } else {
         if (state.tokens.next.id !== "(endline)") {
           break;
@@ -1208,16 +1215,6 @@ var JSHINT = (function() {
     return x;
   }
 
-  function isPoorRelation(node) {
-    return node &&
-        ((node.type === "(number)" && +node.value === 0) ||
-         (node.type === "(string)" && node.value === "") ||
-         (node.type === "null" && !state.option.eqnull) ||
-        node.type === "true" ||
-        node.type === "false" ||
-        node.type === "undefined");
-  }
-
   var typeofValues = {};
   typeofValues.legacy = [
     // E4X extended the `typeof` operator to return "xml" for the XML and
@@ -2061,12 +2058,6 @@ var JSHINT = (function() {
         this.from = this.character;
         warning("W116", this, "===", "==");
         break;
-      case isPoorRelation(left):
-        warning("W041", this, "===", left.value);
-        break;
-      case isPoorRelation(right):
-        warning("W041", this, "===", right.value);
-        break;
       case isTypoTypeof(right, left, state):
         warning("W122", this, right.value);
         break;
@@ -2092,10 +2083,6 @@ var JSHINT = (function() {
     if (!eqnull && state.option.eqeqeq) {
       this.from = this.character;
       warning("W116", this, "!==", "!=");
-    } else if (isPoorRelation(left)) {
-      warning("W041", this, "!==", left.value);
-    } else if (isPoorRelation(right)) {
-      warning("W041", this, "!==", right.value);
     } else if (isTypoTypeof(right, left, state)) {
       warning("W122", this, right.value);
     } else if (isTypoTypeof(left, right, state)) {
@@ -2743,6 +2730,9 @@ var JSHINT = (function() {
           break;
         }
       } else {
+        if (state.option.trailingcomma && state.inES5()) {
+          warningAt("W140", state.tokens.curr.line, state.tokens.curr.character);
+        }
         break;
       }
     }
@@ -3324,6 +3314,9 @@ var JSHINT = (function() {
             warning("W070", state.tokens.curr);
           }
         } else {
+          if (state.option.trailingcomma && state.inES5()) {
+            warningAt("W140", state.tokens.curr.line, state.tokens.curr.character);
+          }
           break;
         }
       }
@@ -3938,6 +3931,11 @@ var JSHINT = (function() {
     state.condition = true;
     advance("(");
     var expr = expression(0);
+
+    if (!expr) {
+      quit("E041", this);
+    }
+
     checkCondAssignment(expr);
 
     // When the if is within a for-in loop, check if the condition
@@ -5391,7 +5389,7 @@ var JSHINT = (function() {
     }
 
     try {
-      assume();
+      applyOptions();
 
       // combine the passed globals after we've assumed all our options
       combine(predefined, g || {});
diff --git a/src/messages.js b/src/messages.js
index 0c910afa..c910a4a3 100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -121,7 +121,7 @@ var warnings = {
   W039: "'{a}' is not allowed.",
   W040: "If a strict mode function is executed using function invocation, " +
     "its 'this' value will be undefined.",
-  W041: "Use '{a}' to compare with '{b}'.",
+  W041: null,
   W042: "Avoid EOL escaping.",
   W043: "Bad escaping of EOL. Use option multistr if needed.",
   W044: "Bad or unnecessary escaping.", /* TODO(caitp): remove W044 */
@@ -223,7 +223,8 @@ var warnings = {
   W136: "'{a}' must be in function scope.",
   W137: "Empty destructuring.",
   W138: "Regular parameters should not come after default parameters.",
-  W139: "Function expressions should not be used as the second operand to instanceof."
+  W139: "Function expressions should not be used as the second operand to instanceof.",
+  W140: "Missing comma."
 };
 
 var info = {
diff --git a/src/options.js b/src/options.js
index 840457ee..ac0c23a2 100644
--- a/src/options.js
+++ b/src/options.js
@@ -239,7 +239,47 @@ exports.bool = {
      *             warnings/errors in when upgrading between minor versions of
      *             JSHint.
      */
-    enforceall : false
+    enforceall : false,
+
+    /**
+     * This option warns when a comma is not placed after the last element in an
+     * array or object literal. Due to bugs in old versions of IE, trailing
+     * commas used to be discouraged, but since ES5 their semantics were
+     * standardized. (See
+     * [#11.1.4](http://www.ecma-international.org/ecma-262/5.1/#sec-11.1.4) and
+     * [#11.1.5](http://www.ecma-international.org/ecma-262/5.1/#sec-11.1.5).)
+     * Now, they help to prevent the same [visual
+     * ambiguities](http://www.ecma-international.org/ecma-262/5.1/#sec-7.9.2)
+     * that the strict usage of semicolons helps prevent.
+     *
+     * For example, this code might have worked last Tuesday:
+     *
+     *     [
+     *         b + c
+     *     ].forEach(print);
+     *
+     * But if one adds an element to the array and forgets to compensate for the
+     * missing comma, no syntax error is thrown, and a linter cannot determine
+     * if this was a mistake or an intentional function invocation.
+     *
+     *     [
+     *         b + c
+     *         (d + e)
+     *     ].forEach(print);
+     *
+     * If one always appends a list item with a comma, this ambiguity cannot
+     * occur:
+     *
+     *     [
+     *         b + c,
+     *     ].forEach(print);
+     *
+     *     [
+     *         b + c,
+     *         (d + e),
+     *     ].forEach(print);
+     */
+    trailingcomma: false
   },
   relaxing: {
 
diff --git a/src/state.js b/src/state.js
index 11c61a0d..77001962 100644
--- a/src/state.js
+++ b/src/state.js
@@ -113,7 +113,11 @@ var state = {
         return badOpt;
       }
 
-      this.esVersion = this.option.esversion;
+      if (this.option.esversion === 2015) {
+        this.esVersion = 6;
+      } else {
+        this.esVersion = this.option.esversion;
+      }
     } else if (this.option.es3) {
       this.esVersion = 3;
     } else if (this.option.esnext) {


Test patch:
diff --git a/tests/helpers/testhelper.js b/tests/helpers/testhelper.js
index 960d01cd..11fc94ed 100644
--- a/tests/helpers/testhelper.js
+++ b/tests/helpers/testhelper.js
@@ -158,4 +158,4 @@ exports.setup.testRun = function (test, name) {
   };
 
   return helperObj;
-};
+};
\ No newline at end of file
diff --git a/tests/regression/thirdparty.js b/tests/regression/thirdparty.js
index a76995f0..e5cd846f 100644
--- a/tests/regression/thirdparty.js
+++ b/tests/regression/thirdparty.js
@@ -11,8 +11,6 @@ exports["Backbone.js 0.5.3"] = function (test) {
     .addError(784, "Unnecessary grouping operator.")
     .addError(864, "Unnecessary grouping operator.")
     .addError(685, "Missing '()' invoking a constructor.")
-    .addError(764, "Use '===' to compare with '0'.")
-    .addError(859, "Use '!==' to compare with '0'.")
     .test(src, { expr: true, eqnull: true, boss: true, regexdash: true, singleGroups: true });
 
   test.done();
@@ -73,10 +71,6 @@ exports.prototype_1_7 = function (test) {
     .addError(409, "Missing semicolon.")
     .addError(430, "Missing semicolon.")
     .addError(451, "Missing semicolon.")
-    .addError(633, "Use '!==' to compare with 'undefined'.")
-    .addError(737, "Use '===' to compare with ''.")
-    .addError(807, "Use '===' to compare with ''.")
-    .addError(1137, "Use '===' to compare with '0'.")
     .addError(1215, "Missing semicolon.")
     .addError(1224, "Unnecessary semicolon.")
     .addError(1916, "Missing semicolon.")
@@ -102,18 +96,12 @@ exports.prototype_1_7 = function (test) {
     .addError(4322, "'nodeCheck' used out of scope.")
     .addError(4520, "'i' is already defined.")
     .addError(4538, "Expected a 'break' statement before 'case'.")
-    .addError(4547, "Use '===' to compare with '0'.")
-    .addError(4565, "Use '===' to compare with '0'.")
-    .addError(4566, "Use '===' to compare with '0'.")
-    .addError(4568, "Use '===' to compare with '0'.")
     .addError(4656, "'i' is already defined.")
     .addError(4722, "Missing '()' invoking a constructor.")
     .addError(4988, "Missing semicolon.")
     .addError(4988, "Missing semicolon.")
     .addError(5021, "Missing semicolon.")
     .addError(5397, "Missing semicolon.")
-    .addError(5112, "Use '!==' to compare with 'undefined'.")
-    .addError(5140, "Use '!==' to compare with ''.")
     .addError(5224, "'values' is already defined.")
     .addError(5495, "Function declarations should not be placed in blocks. Use a function " +
       "expression or move the statement to the top of the outer function.")
@@ -159,10 +147,8 @@ exports.lodash_0_6_1 = function (test) {
     .addError(963, "Reassignment of 'isFunction', which is is a function. Use 'var' or 'let' to declare bindings that may change.")
     .addError(1122, "'isArr' used out of scope.")
     .addError(1127, "'className' used out of scope.")
-    .addError(1129, "Use '===' to compare with 'true'.")
     .addError(1153, "'isArr' used out of scope.")
     .addError(1159, "'isArr' used out of scope.")
-    .addError(1490, "Use '===' to compare with '0'.")
     .addError(1670, "Missing semicolon.")
     .addError(3374, "If a strict mode function is executed using function invocation, its 'this' value will be undefined.")
     .addError(3377, "Missing '()' invoking a constructor.")
@@ -206,7 +192,6 @@ exports.codemirror3 = function (test) {
     "-W008":  true, // Ignore warnings about leading dots in numbers.
     "-W038":  true, // Ignore scope warnings.
     "-W040":  true, // Ignore possible strict violations.
-    "-W041":  true, // Ignore poor relations warnings.
 
   };
 
diff --git a/tests/unit/options.js b/tests/unit/options.js
index 1b88db4f..92b19235 100644
--- a/tests/unit/options.js
+++ b/tests/unit/options.js
@@ -1414,11 +1414,12 @@ exports.eqnull = function (test) {
   ];
 
   // By default, warn about `== null` comparison
+  /**
+   * This test previously asserted the issuance of warning W041. 
+   * W041 has since been removed, but the test is maintained in 
+   * order to discourage regressions.
+   */
   TestRun(test)
-    .addError(1, "Use '===' to compare with 'null'.")
-    .addError(2, "Use '===' to compare with 'null'.")
-    .addError(3, "Use '!==' to compare with 'null'.")
-    .addError(4, "Use '!==' to compare with 'null'.")
     .test(code, {es3: true});
 
   // But when `eqnull` is true, no questions asked
@@ -1533,8 +1534,12 @@ exports.debug = function (test) {
 exports.eqeqeq = function (test) {
   var src = fs.readFileSync(__dirname + '/fixtures/eqeqeq.js', 'utf8');
 
+  /**
+   * This test previously asserted the issuance of warning W041. 
+   * W041 has since been removed, but the test is maintained in 
+   * order to discourage regressions.
+   */
   TestRun(test)
-    .addError(8, "Use '===' to compare with 'null'.")
     .test(src, {es3: true});
 
   TestRun(test)
@@ -3787,20 +3792,26 @@ exports.esversion = function(test) {
   var es6code = [
     "var a = {",
     "  ['b']: 1",
-    "};"
+    "};",
+    "var b = () => {};"
   ];
 
   TestRun(test, "ES6 syntax as ES3")
     .addError(2, "'computed property names' is only available in ES6 (use 'esversion: 6').")
+    .addError(4, "'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').")
     .test(es6code, { esversion: 3 });
 
   TestRun(test, "ES6 syntax as ES5")
     .addError(2, "'computed property names' is only available in ES6 (use 'esversion: 6').")
+    .addError(4, "'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').")
     .test(es6code); // esversion: 5 (default)
 
   TestRun(test, "ES6 syntax as ES6")
     .test(es6code, { esversion: 6 });
 
+  TestRun(test, "ES6 syntax as ES6 (via option value `2015`)")
+    .test(es5code, { esversion: 2015 });
+
   // Array comprehensions aren't defined in ECMAScript 6,
   // but they can be enabled using the `esnext` option
   var arrayComprehension = [
@@ -3853,7 +3864,7 @@ exports.esversion = function(test) {
   ].concat(es6code);
 
   TestRun(test, "incompatible options specified in-line") // TODO: Remove in JSHint 3
-    .addError(1, "Incompatible values for the 'esversion' and 'esnext' linting options. (25% scanned).")
+    .addError(1, "Incompatible values for the 'esversion' and 'esnext' linting options. (20% scanned).")
     .test(code2);
 
   var code3 = [
@@ -3893,6 +3904,45 @@ exports.esversion = function(test) {
     .addError(4, "get/set are ES5 features.")
     .test(code5);
 
+  test.done();
+};
+
+// Option `trailingcomma` requires a comma after each element in an array or
+// object literal.
+exports.trailingcomma = function (test) {
+  var code = [
+    "var a = [];",
+    "var b = [1];",
+    "var c = [1,];",
+    "var d = [1,2];",
+    "var e = [1,2,];",
+    "var f = {};",
+    "var g = {a: 1};",
+    "var h = {a: 1,};",
+    "var i = {a: 1, b: 2};",
+    "var j = {a: 1, b: 2,};"
+  ];
+
+  TestRun(test, "trailingcomma=true ES6")
+    .addError(2, "Missing comma.")
+    .addError(4, "Missing comma.")
+    .addError(7, "Missing comma.")
+    .addError(9, "Missing comma.")
+    .test(code, { trailingcomma: true, esversion: 6 });
+
+  TestRun(test, "trailingcomma=true ES5")
+    .addError(2, "Missing comma.")
+    .addError(4, "Missing comma.")
+    .addError(7, "Missing comma.")
+    .addError(9, "Missing comma.")
+    .test(code, { trailingcomma: true });
+
+  TestRun(test, "trailingcomma=true ES3")
+    .addError(3, "Extra comma. (it breaks older versions of IE)")
+    .addError(5, "Extra comma. (it breaks older versions of IE)")
+    .addError(8, "Extra comma. (it breaks older versions of IE)")
+    .addError(10, "Extra comma. (it breaks older versions of IE)")
+    .test(code, { trailingcomma: true, es3: true });
 
   test.done();
 };
diff --git a/tests/unit/parser.js b/tests/unit/parser.js
index eeb5cbca..14b96275 100644
--- a/tests/unit/parser.js
+++ b/tests/unit/parser.js
@@ -1039,7 +1039,6 @@ exports["gh-2587"] = function (test) {
   TestRun(test)
     .addError(1, "Expected an identifier and instead saw 'if'.")
     .addError(1, "Unrecoverable syntax error. (100% scanned).")
-    .addError(1, "Use '===' to compare with 'true'.")
     .test([
     "true == if"
   ], {});
@@ -1047,7 +1046,6 @@ exports["gh-2587"] = function (test) {
   TestRun(test)
     .addError(1, "Expected an identifier and instead saw 'if'.")
     .addError(1, "Unrecoverable syntax error. (100% scanned).")
-    .addError(1, "Use '!==' to compare with 'true'.")
     .test([
     "true != if"
   ], {});
@@ -6920,8 +6918,12 @@ exports.ignoreDirective["should be a good option and only accept start, end or l
 exports.ignoreDirective["should allow the linter to skip blocked-out lines to continue finding errors in the rest of the code"] = function (test) {
   var code = fs.readFileSync(__dirname + "/fixtures/gh826.js", "utf8");
 
+  /**
+   * This test previously asserted the issuance of warning W041. 
+   * W041 has since been removed, but the test is maintained in 
+   * order to discourage regressions.
+   */
   TestRun(test)
-    .addError(34, "Use '===' to compare with '0'.")
     .test(code);
 
   test.done();
@@ -7027,7 +7029,6 @@ exports["should be able to ignore a single line with a trailing comment: // jshi
 exports["regression test for GH-1431"] = function (test) {
   // The code is invalid but it should not crash JSHint.
   TestRun(test)
-    .addError(1, "Use '!==' to compare with 'null'.")
     .addError(1, "Expected ';' and instead saw ')'.")
     .addError(1, "Expected ')' and instead saw ';'.")
     .addError(1, "Expected an identifier and instead saw ';'.")
@@ -7211,6 +7212,21 @@ exports["'for of' shouldn't be subject to 'for in' rules"] = function (test) {
   test.done();
 };
 
+// See gh-3099, "TypeError: Cannot read property 'type' of undefined"
+exports["enforcement of `forin` option should be tolerant of invalid syntax"] = function (test) {
+  TestRun(test)
+    .addError(1, "Creating global 'for' variable. Should be 'for (var x ...'.")
+    .addError(2, "Unrecoverable syntax error. (66% scanned).")
+    .addError(3, "Expected an identifier and instead saw '}'.")
+    .test([
+      "for (x in x) {",
+      "  if (",
+      "}"
+    ], { forin: true });
+
+  test.done();
+};
+
 exports["Ignore strings containing braces within array literal declarations"] = function (test) {
   TestRun(test).test("var a = [ '[' ];");
   test.done();