Chore: move logic for handling missing rules to rules.js (#9235)

lib/linter.js

@@ -14,7 +14,6 @@ const EventEmitter = require("events").EventEmitter,
     levn = require("levn"),
     blankScriptAST = require("../conf/blank-script.json"),
     defaultConfig = require("../conf/default-config-options.js"),
-    replacements = require("../conf/replacements.json"),
     CodePathAnalyzer = require("./code-path-analysis/code-path-analyzer"),
     ConfigOps = require("./config/config-ops"),
     validator = require("./config/config-validator"),
@@ -518,53 +517,6 @@ function prepareConfig(config, envContext) {
     return preparedConfig;
 }
 
-/**
- * Provide a stub rule with a given message
- * @param  {string} message The message to be displayed for the rule
- * @returns {Function}      Stub rule function
- */
-function createStubRule(message) {
-
-    /**
-     * Creates a fake rule object
-     * @param {Object} context context object for each rule
-     * @returns {Object} collection of node to listen on
-     */
-    function createRuleModule(context) {
-        return {
-            Program() {
-                context.report({
-                    loc: { line: 1, column: 0 },
-                    message
-                });
-            }
-        };
-    }
-
-    if (message) {
-        return createRuleModule;
-    }
-
-    /* istanbul ignore next */
-    throw new Error("No message passed to stub rule");
-
-}
-
-/**
- * Provide a rule replacement message
- * @param  {string} ruleId Name of the rule
- * @returns {string}       Message detailing rule replacement
- */
-function getRuleReplacementMessage(ruleId) {
-    if (ruleId in replacements.rules) {
-        const newRules = replacements.rules[ruleId];
-
-        return `Rule '${ruleId}' was removed and replaced by: ${newRules.join(", ")}`;
-    }
-
-    return null;
-}
-
 const eslintEnvPattern = /\/\*\s*eslint-env\s(.+?)\*\//g;
 
 /**
@@ -924,19 +876,8 @@ class Linter {
             if (severity === 0) {
                 return;
             }
-            let ruleCreator = this.rules.get(ruleId);
-
-            if (!ruleCreator) {
-                const replacementMsg = getRuleReplacementMessage(ruleId);
-
-                if (replacementMsg) {
-                    ruleCreator = createStubRule(replacementMsg);
-                } else {
-                    ruleCreator = createStubRule(`Definition for rule '${ruleId}' was not found`);
-                }
-                this.rules.define(ruleId, ruleCreator);
-            }
 
+            const ruleCreator = this.rules.get(ruleId);
             let reportTranslator = null;
             const ruleContext = Object.freeze(
                 Object.assign(

lib/rules.js

@@ -9,7 +9,38 @@
 // Requirements
 //------------------------------------------------------------------------------
 
+const lodash = require("lodash");
 const loadRules = require("./load-rules");
+const ruleReplacements = require("../conf/replacements").rules;
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+/**
+ * Creates a stub rule that gets used when a rule with a given ID is not found.
+ * @param {string} ruleId The ID of the missing rule
+ * @returns {{create: function(RuleContext): Object}} A rule that reports an error at the first location
+ * in the program. The report has the message `Definition for rule '${ruleId}' was not found` if the rule is unknown,
+ * or `Rule '${ruleId}' was removed and replaced by: ${replacements.join(", ")}` if the rule is known to have been
+ * replaced.
+ */
+const createMissingRule = lodash.memoize(ruleId => {
+    const message = Object.prototype.hasOwnProperty.call(ruleReplacements, ruleId)
+        ? `Rule '${ruleId}' was removed and replaced by: ${ruleReplacements[ruleId].join(", ")}`
+        : `Definition for rule '${ruleId}' was not found`;
+
+    return {
+        create: context => ({
+            Program() {
+                context.report({
+                    loc: { line: 1, column: 0 },
+                    message
+                });
+            }
+        })
+    };
+});
 
 //------------------------------------------------------------------------------
 // Public Interface
@@ -69,6 +100,9 @@ class Rules {
      * @returns {Function} Rule handler.
      */
     get(ruleId) {
+        if (!Object.prototype.hasOwnProperty.call(this._rules, ruleId)) {
+            return createMissingRule(ruleId);
+        }
         if (typeof this._rules[ruleId] === "string") {
             return require(this._rules[ruleId]);
         }

tests/lib/config/plugins.js

@@ -180,7 +180,7 @@ describe("Plugins", () => {
                 };
                 StubbedPlugins.load("@scope/eslint-plugin-example");
 
-                assert.equal(rules.get("example/foo"), null);
+                assert.isFalse(rules.getAllLoadedRules().has("example/foo"));
             });
         });
     });

tests/lib/rules.js

@@ -10,7 +10,8 @@
 //------------------------------------------------------------------------------
 
 const assert = require("chai").assert,
-    Rules = require("../../lib/rules");
+    Rules = require("../../lib/rules"),
+    Linter = require("../../lib/linter");
 
 //------------------------------------------------------------------------------
 // Tests
@@ -26,7 +27,7 @@ describe("rules", () => {
     describe("when given an invalid rules directory", () => {
         const code = "invaliddir";
 
-        it("should log an error and exit", () => {
+        it("should throw an error", () => {
             assert.throws(() => {
                 rules.load(code);
             });
@@ -36,8 +37,7 @@ describe("rules", () => {
     describe("when given a valid rules directory", () => {
         const code = "tests/fixtures/rules";
 
-        it("should load rules and not log an error or exit", () => {
-            assert.equal(typeof rules.get("fixture-rule"), "undefined");
+        it("should load rules and not throw an error", () => {
             rules.load(code, process.cwd());
             assert.equal(typeof rules.get("fixture-rule"), "object");
         });
@@ -52,6 +52,43 @@ describe("rules", () => {
         });
     });
 
+    describe("when a rule is not found", () => {
+        it("should return a stub rule that reports an error if the rule is unknown", () => {
+            const stubRule = rules.get("not-defined");
+            const linter = new Linter();
+
+            linter.defineRule("test-rule", stubRule);
+
+            const problems = linter.verify("foo", { rules: { "test-rule": "error" } });
+
+            assert.lengthOf(problems, 1);
+            assert.strictEqual(problems[0].message, "Definition for rule 'not-defined' was not found");
+            assert.strictEqual(problems[0].line, 1);
+            assert.strictEqual(problems[0].column, 1);
+            assert.typeOf(problems[0].endLine, "undefined");
+            assert.typeOf(problems[0].endColumn, "undefined");
+        });
+
+        it("should return a stub rule that lists replacements if a rule is known to have been replaced", () => {
+            const stubRule = rules.get("no-arrow-condition");
+            const linter = new Linter();
+
+            linter.defineRule("test-rule", stubRule);
+
+            const problems = linter.verify("foo", { rules: { "test-rule": "error" } });
+
+            assert.lengthOf(problems, 1);
+            assert.strictEqual(
+                problems[0].message,
+                "Rule 'no-arrow-condition' was removed and replaced by: no-confusing-arrow, no-constant-condition"
+            );
+            assert.strictEqual(problems[0].line, 1);
+            assert.strictEqual(problems[0].column, 1);
+            assert.typeOf(problems[0].endLine, "undefined");
+            assert.typeOf(problems[0].endColumn, "undefined");
+        });
+    });
+
     describe("when importing plugin rules", () => {
         const customPlugin = {
                 rules: {