Repository: eslint/eslint
Pull request #9234 (2017-09-05 17:32:57)

Chore: upgrade eslint-plugin-eslint-plugin@1.0.0
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[x] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

## v1.0.0 (2017-08-31)

* Fix: some rules crashing if tests array has missing elements (fixes [#35](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/35)). ([#36](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/36)) ([e3a14e1](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/e3a14e11d86836b1ab63d19c689327fea8f8a4bc))
* Breaking: update ESLint peerDependency to >=4.1.0 ([#33](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/33)) ([7cabcdc](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/7cabcdcae32ae67fc3769804aad5e5d8882b3baa))
* Chore: use local variable. ([#34](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/34)) ([e04a4bc](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/e04a4bc15963efdff6309549e34c51cfa2c2ea74))
* Update: add autofixing to test-case-property-ordering. (fixes [#31](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/31)) ([#32](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/32)) ([23f9010](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/23f90106a5c8b3cf1a785989d6c9ec798da339a9))

**Is there anything you'd like reviewers to focus on?**




eslintbot: LGTM

Patch:
diff --git a/lib/linter.js b/lib/linter.js
index 1a99bd171..76487c53e 100755
--- a/lib/linter.js
+++ b/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 rule = this.rules.get(ruleId);
             let reportTranslator = null;
             const ruleContext = Object.freeze(
                 Object.assign(
@@ -961,7 +902,7 @@ class Linter {
                             }
                             const problem = reportTranslator.apply(null, arguments);
 
-                            if (problem.fix && ruleCreator.meta && !ruleCreator.meta.fixable) {
+                            if (problem.fix && rule.meta && !rule.meta.fixable) {
                                 throw new Error("Fixable rules should export a `meta.fixable` property.");
                             }
                             problems.push(problem);
@@ -989,16 +930,15 @@ class Linter {
             );
 
             try {
-                const rule = ruleCreator.create
-                    ? ruleCreator.create(ruleContext)
-                    : ruleCreator(ruleContext);
+                const ruleListeners = rule.create(ruleContext);
 
                 // add all the selectors from the rule as listeners
-                Object.keys(rule).forEach(selector => {
+                Object.keys(ruleListeners).forEach(selector => {
                     emitter.on(
-                        selector, timing.enabled
-                            ? timing.time(ruleId, rule[selector])
-                            : rule[selector]
+                        selector,
+                        timing.enabled
+                            ? timing.time(ruleId, ruleListeners[selector])
+                            : ruleListeners[selector]
                     );
                 });
             } catch (ex) {
diff --git a/lib/rules.js b/lib/rules.js
index 893104f65..040f9db50 100644
--- a/lib/rules.js
+++ b/lib/rules.js
@@ -9,7 +9,48 @@
 // 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
+                });
+            }
+        })
+    };
+});
+
+/**
+ * Normalizes a rule module to the new-style API
+ * @param {(Function|{create: Function})} rule A rule object, which can either be a function
+ * ("old-style") or an object with a `create` method ("new-style")
+ * @returns {{create: Function}} A new-style rule.
+ */
+function normalizeRule(rule) {
+    return typeof rule === "function" ? Object.assign({ create: rule }, rule) : rule;
+}
 
 //------------------------------------------------------------------------------
 // Public Interface
@@ -29,7 +70,7 @@ class Rules {
      * @returns {void}
      */
     define(ruleId, ruleModule) {
-        this._rules[ruleId] = ruleModule;
+        this._rules[ruleId] = normalizeRule(ruleModule);
     }
 
     /**
@@ -66,11 +107,15 @@ class Rules {
     /**
      * Access rule handler by id (file name).
      * @param {string} ruleId Rule id (file name).
-     * @returns {Function} Rule handler.
+     * @returns {{create: Function, schema: JsonSchema[]}}
+     * A rule. This is normalized to always have the new-style shape with a `create` method.
      */
     get(ruleId) {
+        if (!Object.prototype.hasOwnProperty.call(this._rules, ruleId)) {
+            return createMissingRule(ruleId);
+        }
         if (typeof this._rules[ruleId] === "string") {
-            return require(this._rules[ruleId]);
+            return normalizeRule(require(this._rules[ruleId]));
         }
         return this._rules[ruleId];
 
diff --git a/package.json b/package.json
index 5bcfc2803..dd79d8d27 100644
--- a/package.json
+++ b/package.json
@@ -85,7 +85,7 @@
     "coveralls": "^2.13.1",
     "dateformat": "^2.0.0",
     "ejs": "^2.5.6",
-    "eslint-plugin-eslint-plugin": "^0.8.0",
+    "eslint-plugin-eslint-plugin": "^1.0.0",
     "eslint-plugin-node": "^5.1.0",
     "eslint-release": "^0.10.1",
     "eslump": "1.6.0",


Test patch:
diff --git a/lib/testers/rule-tester.js b/lib/testers/rule-tester.js
index 5b58e5452..c16f51c6d 100644
--- a/lib/testers/rule-tester.js
+++ b/lib/testers/rule-tester.js
@@ -350,16 +350,6 @@ class RuleTester {
                 linter.rules.get = function(ruleId) {
                     const originalRule = originalGet.call(linter.rules, ruleId);
 
-                    if (typeof originalRule === "function") {
-                        return function(context) {
-                            Object.freeze(context);
-                            freezeDeeply(context.options);
-                            freezeDeeply(context.settings);
-                            freezeDeeply(context.parserOptions);
-
-                            return originalRule(context);
-                        };
-                    }
                     return {
                         meta: originalRule.meta,
                         create(context) {
diff --git a/tests/lib/config/plugins.js b/tests/lib/config/plugins.js
index 4b165ea4e..97c5637d3 100644
--- a/tests/lib/config/plugins.js
+++ b/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"));
             });
         });
     });
diff --git a/tests/lib/rules.js b/tests/lib/rules.js
index efae6f0e1..06f87b680 100644
--- a/tests/lib/rules.js
+++ b/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");
         });
@@ -50,6 +50,62 @@ describe("rules", () => {
             rules.define(ruleId, {});
             assert.ok(rules.get(ruleId));
         });
+
+        it("should return the rule as an object with a create() method if the rule was defined as a function", () => {
+
+            /**
+             * A rule that does nothing
+             * @returns {void}
+             */
+            function rule() {}
+            rule.schema = [];
+            rules.define("foo", rule);
+            assert.deepEqual(rules.get("foo"), { create: rule, schema: [] });
+        });
+
+        it("should return the rule as-is if it was defined as an object with a create() method", () => {
+            const rule = { create() {} };
+
+            rules.define("foo", rule);
+            assert.strictEqual(rules.get("foo"), rule);
+        });
+    });
+
+    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", () => {
@@ -64,7 +120,7 @@ describe("rules", () => {
             rules.importPlugin(customPlugin, pluginName);
 
             assert.isDefined(rules.get("custom-plugin/custom-rule"));
-            assert.equal(rules.get("custom-plugin/custom-rule"), customPlugin.rules["custom-rule"]);
+            assert.equal(rules.get("custom-plugin/custom-rule").create, customPlugin.rules["custom-rule"]);
         });
 
         it("should return custom rules as part of getAllLoadedRules", () => {
@@ -72,7 +128,7 @@ describe("rules", () => {
 
             const allRules = rules.getAllLoadedRules();
 
-            assert.equal(allRules.get("custom-plugin/custom-rule"), customPlugin.rules["custom-rule"]);
+            assert.equal(allRules.get("custom-plugin/custom-rule").create, customPlugin.rules["custom-rule"]);
         });
     });