Fix: avoid crashing on malformed configuration comments (fixes #9373) (…

…#9819)

This updates `Linter` to treat malformed configuration comments as linting errors rather than fatal issues that crash the process. Previously, this could cause surprising behavior because it was the only known instance where a problem in the source code (given a valid configuration) could cause an error to be thrown.

For example, this also fixes eslint/archive-website#413.

lib/config/config-validator.js

@@ -98,7 +98,8 @@ function validateRuleSchema(rule, localOptions) {
  * @param {{create: Function}|null} rule The rule that the config is being validated for
  * @param {string} ruleId The rule's unique name.
  * @param {array|number} options The given options for the rule.
- * @param {string} source The name of the configuration source to report in any errors.
+ * @param {string|null} source The name of the configuration source to report in any errors. If null or undefined,
+ * no source is prepended to the message.
  * @returns {void}
  */
 function validateRuleOptions(rule, ruleId, options, source) {
@@ -112,7 +113,13 @@ function validateRuleOptions(rule, ruleId, options, source) {
             validateRuleSchema(rule, Array.isArray(options) ? options.slice(1) : []);
         }
     } catch (err) {
-        throw new Error(`${source}:\n\tConfiguration for rule "${ruleId}" is invalid:\n${err.message}`);
+        const enhancedMessage = `Configuration for rule "${ruleId}" is invalid:\n${err.message}`;
+
+        if (typeof source === "string") {
+            throw new Error(`${source}:\n\t${enhancedMessage}`);
+        } else {
+            throw new Error(enhancedMessage);
+        }
     }
 }
 

lib/linter.js

@@ -340,7 +340,21 @@ function modifyConfigsFromComments(filename, ast, config, ruleMapper) {
                             Object.keys(parseResult.config).forEach(name => {
                                 const ruleValue = parseResult.config[name];
 
-                                validator.validateRuleOptions(ruleMapper(name), name, ruleValue, `${filename} line ${comment.loc.start.line}`);
+                                try {
+                                    validator.validateRuleOptions(ruleMapper(name), name, ruleValue);
+                                } catch (err) {
+                                    problems.push({
+                                        ruleId: name,
+                                        severity: 2,
+                                        source: null,
+                                        message: err.message,
+                                        line: comment.loc.start.line,
+                                        column: comment.loc.start.column + 1,
+                                        endLine: comment.loc.end.line,
+                                        endColumn: comment.loc.end.column + 1,
+                                        nodeType: null
+                                    });
+                                }
                                 commentRules[name] = ruleValue;
                             });
                         } else {

tests/lib/linter.js

@@ -1615,14 +1615,42 @@ describe("Linter", () => {
     });
 
     describe("when evaluating code with invalid comments to enable rules", () => {
-        const code = "/*eslint no-alert:true*/ alert('test');";
-
-        it("should report a violation", () => {
-            const config = { rules: {} };
-
-            const fn = linter.verify.bind(linter, code, config, filename);
+        it("should report a violation when the config is not a valid rule configuration", () => {
+            assert.deepStrictEqual(
+                linter.verify("/*eslint no-alert:true*/ alert('test');", {}),
+                [
+                    {
+                        severity: 2,
+                        ruleId: "no-alert",
+                        message: "Configuration for rule \"no-alert\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed 'true').\n",
+                        line: 1,
+                        column: 1,
+                        endLine: 1,
+                        endColumn: 25,
+                        source: null,
+                        nodeType: null
+                    }
+                ]
+            );
+        });
 
-            assert.throws(fn, "filename.js line 1:\n\tConfiguration for rule \"no-alert\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed 'true').\n");
+        it("should report a violation when the config violates a rule's schema", () => {
+            assert.deepStrictEqual(
+                linter.verify("/* eslint no-alert: [error, {nonExistentPropertyName: true}]*/", {}),
+                [
+                    {
+                        severity: 2,
+                        ruleId: "no-alert",
+                        message: "Configuration for rule \"no-alert\" is invalid:\n\tValue [{\"nonExistentPropertyName\":true}] should NOT have more than 0 items.\n",
+                        line: 1,
+                        column: 1,
+                        endLine: 1,
+                        endColumn: 63,
+                        source: null,
+                        nodeType: null
+                    }
+                ]
+            );
         });
     });