Repository: eslint/eslint
Pull request #9236 (2017-09-05 18:08:21)

Chore: always normalize rules to new API in rules.js
<!--
    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)**

This updates `rules.js` to always normalize rules to the new-style API when retrieving them from storage. This avoids the need for other modules, such as `Linter`, to deal with casework for multiple different rule formats.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


eslintbot: LGTM

Patch:
diff --git a/lib/linter.js b/lib/linter.js
index 933181406..76487c53e 100755
--- a/lib/linter.js
+++ b/lib/linter.js
@@ -877,7 +877,7 @@ class Linter {
                 return;
             }
 
-            const ruleCreator = this.rules.get(ruleId);
+            const rule = this.rules.get(ruleId);
             let reportTranslator = null;
             const ruleContext = Object.freeze(
                 Object.assign(
@@ -902,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);
@@ -930,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 c9ae9e5c2..040f9db50 100644
--- a/lib/rules.js
+++ b/lib/rules.js
@@ -42,6 +42,16 @@ const createMissingRule = lodash.memoize(ruleId => {
     };
 });
 
+/**
+ * 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
 //------------------------------------------------------------------------------
@@ -60,7 +70,7 @@ class Rules {
      * @returns {void}
      */
     define(ruleId, ruleModule) {
-        this._rules[ruleId] = ruleModule;
+        this._rules[ruleId] = normalizeRule(ruleModule);
     }
 
     /**
@@ -97,14 +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];
 


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/rules.js b/tests/lib/rules.js
index 82ef3b5d3..06f87b680 100644
--- a/tests/lib/rules.js
+++ b/tests/lib/rules.js
@@ -50,6 +50,25 @@ 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", () => {
@@ -101,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", () => {
@@ -109,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"]);
         });
     });