Breaking: throw an error if disables a non-existent rule in a configu…

…ration comment (fixes#9505)

lib/config/config-validator.js

@@ -125,6 +125,12 @@ function validateRuleOptions(rule, ruleId, options, source = null) {
     if (!rule) {
         return;
     }
+
+    // https://github.com/eslint/eslint/issues/9505
+    if (rule && rule.meta && rule.meta.type === "internal") {
+        throw new Error(`Definition for rule '${ruleId}' was not found`);
+    }
+
     try {
         const severity = validateRuleSeverity(options);
 
@@ -332,5 +338,7 @@ module.exports = {
     validate,
     validateConfigArray,
     validateConfigSchema,
-    validateRuleOptions
+    validateRuleOptions,
+    validateRules,
+    validateRuleSeverity
 };

lib/linter.js

@@ -160,6 +160,7 @@ function createDisableDirectives(type, loc, value) {
     return directiveRules.map(ruleId => ({ type, line: loc.line, column: loc.column + 1, ruleId }));
 }
 
+
 /**
  * Parses comments in file to extract file-specific config of rules, globals
  * and environments and merges them with global config; also code blocks
@@ -243,9 +244,39 @@ function getDirectiveComments(filename, ast, ruleMapper) {
                     }
                     break;
 
-                case "eslint-disable":
-                    disableDirectives.push(...createDisableDirectives("disable", comment.loc.start, directiveValue));
+                case "eslint-disable": {
+                    const directiveRules = createDisableDirectives("disable", comment.loc.start, directiveValue);
+
+                    // validate disable directives
+                    directiveRules.forEach(directive => {
+                        const ruleId = directive.ruleId;
+
+                        if (ruleId !== null) {
+                            const options = directive.type === "disable" ? [0] : [2];
+
+                            try {
+                                validator.validateRuleOptions(ruleMapper(ruleId), ruleId, options);
+                            } catch (err) {
+                                problems.push({
+                                    ruleId,
+                                    severity: 2,
+                                    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
+                                });
+
+                                // do not apply the config, if found invalid options.
+                                return;
+                            }
+                        }
+
+                        disableDirectives.push(directive);
+                    });
                     break;
+                }
 
                 case "eslint-enable":
                     disableDirectives.push(...createDisableDirectives("enable", comment.loc.start, directiveValue));
@@ -271,8 +302,13 @@ function getDirectiveComments(filename, ast, ruleMapper) {
                                     endColumn: comment.loc.end.column + 1,
                                     nodeType: null
                                 });
+
+                                // do not apply the config, if found invalid options.
+                                return;
                             }
+
                             configuredRules[name] = ruleValue;
+
                         });
                     } else {
                         problems.push(parseResult.error);
@@ -699,12 +735,13 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser
 
     Object.keys(configuredRules).forEach(ruleId => {
         const severity = ConfigOps.getRuleSeverity(configuredRules[ruleId]);
+        const rule = ruleMapper(ruleId);
 
-        if (severity === 0) {
+        if (severity === 0 || !rule) {
             return;
         }
 
-        const rule = ruleMapper(ruleId);
+
         const messageIds = rule.meta && rule.meta.messages;
         let reportTranslator = null;
         const ruleContext = Object.freeze(
@@ -946,13 +983,13 @@ class Linter {
             configuredGlobals,
             { exportedVariables: commentDirectives.exportedVariables, enabledGlobals: commentDirectives.enabledGlobals }
         );
+        let lintingProblems = [];
 
         const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);
 
-        let lintingProblems;
 
         try {
-            lintingProblems = runRules(
+            lintingProblems = lintingProblems.concat(runRules(
                 sourceCode,
                 configuredRules,
                 ruleId => getRule(slots, ruleId),
@@ -961,7 +998,7 @@ class Linter {
                 settings,
                 options.filename,
                 options.disableFixes
-            );
+            ));
         } catch (err) {
             err.message += `\nOccurred while linting ${options.filename}`;
             debug("An error occurred while traversing");

lib/rules.js

@@ -31,6 +31,9 @@ const createMissingRule = lodash.memoize(ruleId => {
         : `Definition for rule '${ruleId}' was not found`;
 
     return {
+        meta: {
+            type: "internal" // only used in eslint core.
+        },
         create: context => ({
             Program() {
                 context.report({

tests/lib/config/config-validator.js

@@ -443,18 +443,6 @@ describe("Validator", () => {
             assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '[ \"error\" ]').\n");
         });
 
-        it("should only check warning level for nonexistent rules", () => {
-            const fn = validator.validateRuleOptions.bind(null, ruleMapper("non-existent-rule"), "non-existent-rule", [3, "foobar"], "tests");
-
-            assert.throws(fn, "tests:\n\tConfiguration for rule \"non-existent-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '3').\n");
-        });
-
-        it("should only check warning level for plugin rules", () => {
-            const fn = validator.validateRuleOptions.bind(null, ruleMapper("plugin/rule"), "plugin/rule", 3, "tests");
-
-            assert.throws(fn, "tests:\n\tConfiguration for rule \"plugin/rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '3').\n");
-        });
-
         it("should throw for incorrect configuration values", () => {
             const fn = validator.validateRuleOptions.bind(null, ruleMapper("mock-rule"), "mock-rule", [2, "frist"], "tests");
 

tests/lib/linter.js

@@ -1699,6 +1699,24 @@ describe("Linter", () => {
         });
     });
 
+    describe("when evaluating code with comments to disable rules", () => {
+
+
+        it("should report an error when disabling a non-existent rule", () => {
+            const code = "/*eslint foo:0*/ ;";
+            const messages = linter.verify(code, {}, filename);
+
+            assert.strictEqual(messages.length, 1);
+        });
+
+        it("should report an error when disabling a non-existent rule", () => {
+            const code = "/*eslint-disable foo*/ ;";
+            const messages = linter.verify(code, {}, filename);
+
+            assert.strictEqual(messages.length, 1);
+        });
+    });
+
     describe("when evaluating code with comments to enable multiple rules", () => {
         const code = "/*eslint no-alert:1 no-console:1*/ alert('test'); console.log('test');";