Fix: failed to parse /*eslint comments by colon (fixes #6224) (#6258)

It came to use Optionator to parse the comments by the same way as
parsing CLI option. But Optionator cannot parse commaless notation, so
there is a fallback for that.

Author: mysticatea

lib/config/config-ops.js

@@ -23,7 +23,8 @@ var RULE_SEVERITY_STRINGS = ["off", "warn", "error"],
     RULE_SEVERITY = RULE_SEVERITY_STRINGS.reduce(function(map, value, index) {
         map[value] = index;
         return map;
-    }, {});
+    }, {}),
+    VALID_SEVERITIES = [0, 1, 2, "off", "warn", "error"];
 
 //------------------------------------------------------------------------------
 // Public Interface
@@ -248,6 +249,30 @@ module.exports = {
         }
 
         return (typeof severity === "number" && severity === 2);
-    }
+    },
 
+    /**
+     * Checks whether a given config has valid severity or not.
+     * @param {number|string|Array} ruleConfig - The configuration for an individual rule.
+     * @returns {boolean} `true` if the configuration has valid severity.
+     */
+    isValidSeverity: function(ruleConfig) {
+        var severity = Array.isArray(ruleConfig) ? ruleConfig[0] : ruleConfig;
+
+        if (typeof severity === "string") {
+            severity = severity.toLowerCase();
+        }
+        return VALID_SEVERITIES.indexOf(severity) !== -1;
+    },
+
+    /**
+     * Checks whether every rule of a given config has valid severity or not.
+     * @param {object} config - The configuration for rules.
+     * @returns {boolean} `true` if the configuration has valid severity.
+     */
+    isEverySeverityValid: function(config) {
+        return Object.keys(config).every(function(ruleId) {
+            return this.isValidSeverity(config[ruleId]);
+        }, this);
+    }
 };

lib/eslint.js

@@ -9,25 +9,25 @@
 // Requirements
 //------------------------------------------------------------------------------
 
-var lodash = require("lodash"),
-    Traverser = require("./util/traverser"),
+var assert = require("assert"),
+    EventEmitter = require("events").EventEmitter,
     escope = require("escope"),
-    Environments = require("./config/environments"),
+    levn = require("levn"),
+    lodash = require("lodash"),
     blankScriptAST = require("../conf/blank-script.json"),
-    rules = require("./rules"),
-    RuleContext = require("./rule-context"),
-    timing = require("./timing"),
-    SourceCode = require("./util/source-code"),
-    NodeEventGenerator = require("./util/node-event-generator"),
-    CommentEventGenerator = require("./util/comment-event-generator"),
-    EventEmitter = require("events").EventEmitter,
+    DEFAULT_PARSER = require("../conf/eslint.json").parser,
+    replacements = require("../conf/replacements.json"),
+    CodePathAnalyzer = require("./code-path-analysis/code-path-analyzer"),
     ConfigOps = require("./config/config-ops"),
     validator = require("./config/config-validator"),
-    replacements = require("../conf/replacements.json"),
-    assert = require("assert"),
-    CodePathAnalyzer = require("./code-path-analysis/code-path-analyzer");
-
-var DEFAULT_PARSER = require("../conf/eslint.json").parser;
+    Environments = require("./config/environments"),
+    CommentEventGenerator = require("./util/comment-event-generator"),
+    NodeEventGenerator = require("./util/node-event-generator"),
+    SourceCode = require("./util/source-code"),
+    Traverser = require("./util/traverser"),
+    RuleContext = require("./rule-context"),
+    rules = require("./rules"),
+    timing = require("./timing");
 
 //------------------------------------------------------------------------------
 // Helpers
@@ -80,6 +80,25 @@ function parseBooleanConfig(string, comment) {
 function parseJsonConfig(string, location, messages) {
     var items = {};
 
+    // Parses a JSON-like comment by the same way as parsing CLI option.
+    try {
+        items = levn.parse("Object", string) || {};
+
+        // Some tests say that it should ignore invalid comments such as `/*eslint no-alert:abc*/`.
+        // Also, commaless notations have invalid severity:
+        //     "no-alert: 2 no-console: 2" --> {"no-alert": "2 no-console: 2"}
+        // Should ignore that case as well.
+        if (ConfigOps.isEverySeverityValid(items)) {
+            return items;
+        }
+    } catch (ex) {
+
+        // ignore to parse the string by a fallback.
+    }
+
+    // Optionator cannot parse commaless notations.
+    // But we are supporting that. So this is a fallback for that.
+    items = {};
     string = string.replace(/([a-zA-Z0-9\-\/]+):/g, "\"$1\":").replace(/(\]|[0-9])\s+(?=")/, "$1,");
     try {
         items = JSON.parse("{" + string + "}");

package.json

@@ -55,6 +55,7 @@
     "is-resolvable": "^1.0.0",
     "js-yaml": "^3.5.1",
     "json-stable-stringify": "^1.0.0",
+    "levn": "^0.3.0",
     "lodash": "^4.0.0",
     "mkdirp": "^0.5.0",
     "optionator": "^0.8.1",

tests/lib/eslint.js

@@ -2515,6 +2515,19 @@ describe("eslint", function() {
         });
     });
 
+    describe("when evaluating code with comments which have colon in its value", function() {
+        var code = "/* eslint max-len: [2, 100, 2, {ignoreUrls: true, ignorePattern: \"data:image\\/|\\s*require\\s*\\(|^\\s*loader\\.lazy|-\\*-\"}] */\nalert('test');";
+
+        it("should not parse errors, should report a violation", function() {
+            var messages = eslint.verify(code, {}, filename);
+
+            assert.equal(messages.length, 1);
+            assert.equal(messages[0].ruleId, "max-len");
+            assert.equal(messages[0].message, "Line 1 exceeds the maximum line length of 100.");
+            assert.include(messages[0].nodeType, "Program");
+        });
+    });
+
     describe("when evaluating a file with a shebang", function() {
         var code = "#!bin/program\n\nvar foo;;";