feat!: Set default schema: [], drop support for function-style rules (

#139) * drop support for function-style rules * implement schema changes * add unit tests for ConfigValidator#getRuleOptionsSchema * add regression tests for ConfigValidator#validateRuleOptions * add tests for error code * update fixture rules to use object-style
eslint · Dec 26, 2023 · a6c240d · a6c240d
1 parent 01db002
commit a6c240d
Show file tree

Hide file tree

Showing 8 changed files with 374 additions and 76 deletions.
diff --git a/lib/config-array/config-array.js b/lib/config-array/config-array.js
@@ -319,31 +319,18 @@ function createConfig(instance, indices) {
  * @param {string} pluginId The plugin ID for prefix.
  * @param {Record<string,T>} defs The definitions to collect.
  * @param {Map<string, U>} map The map to output.
- * @param {function(T): U} [normalize] The normalize function for each value.
  * @returns {void}
  */
-function collect(pluginId, defs, map, normalize) {
+function collect(pluginId, defs, map) {
     if (defs) {
         const prefix = pluginId && `${pluginId}/`;
 
         for (const [key, value] of Object.entries(defs)) {
-            map.set(
-                `${prefix}${key}`,
-                normalize ? normalize(value) : value
-            );
+            map.set(`${prefix}${key}`, value);
         }
     }
 }
 
-/**
- * Normalize a rule definition.
- * @param {Function|Rule} rule The rule definition to normalize.
- * @returns {Rule} The normalized rule definition.
- */
-function normalizePluginRule(rule) {
-    return typeof rule === "function" ? { create: rule } : rule;
-}
-
 /**
  * Delete the mutation methods from a given map.
  * @param {Map<any, any>} map The map object to delete.
@@ -385,7 +372,7 @@ function initPluginMemberMaps(elements, slots) {
 
             collect(pluginId, plugin.environments, slots.envMap);
             collect(pluginId, plugin.processors, slots.processorMap);
-            collect(pluginId, plugin.rules, slots.ruleMap, normalizePluginRule);
+            collect(pluginId, plugin.rules, slots.ruleMap);
         }
     }
 

diff --git a/lib/shared/config-validator.js b/lib/shared/config-validator.js
@@ -5,6 +5,12 @@
 
 /* eslint class-methods-use-this: "off" */
 
+//------------------------------------------------------------------------------
+// Typedefs
+//------------------------------------------------------------------------------
+
+/** @typedef {import("../shared/types").Rule} Rule */
+
 //------------------------------------------------------------------------------
 // Requirements
 //------------------------------------------------------------------------------
@@ -33,6 +39,13 @@ const severityMap = {
 
 const validated = new WeakSet();
 
+// JSON schema that disallows passing any options
+const noOptionsSchema = Object.freeze({
+    type: "array",
+    minItems: 0,
+    maxItems: 0
+});
+
 //-----------------------------------------------------------------------------
 // Exports
 //-----------------------------------------------------------------------------
@@ -44,17 +57,36 @@ export default class ConfigValidator {
 
     /**
      * Gets a complete options schema for a rule.
-     * @param {{create: Function, schema: (Array|null)}} rule A new-style rule object
-     * @returns {Object} JSON Schema for the rule's options.
+     * @param {Rule} rule A rule object
+     * @throws {TypeError} If `meta.schema` is specified but is not an array, object or `false`.
+     * @returns {Object|null} JSON Schema for the rule's options.
+     *      `null` if rule wasn't passed or its `meta.schema` is `false`.
      */
     getRuleOptionsSchema(rule) {
         if (!rule) {
             return null;
         }
 
-        const schema = rule.schema || rule.meta && rule.meta.schema;
+        if (!rule.meta) {
+            return { ...noOptionsSchema }; // default if `meta.schema` is not specified
+        }
 
-        // Given a tuple of schemas, insert warning level at the beginning
+        const schema = rule.meta.schema;
+
+        if (typeof schema === "undefined") {
+            return { ...noOptionsSchema }; // default if `meta.schema` is not specified
+        }
+
+        // `schema:false` is an allowed explicit opt-out of options validation for the rule
+        if (schema === false) {
+            return null;
+        }
+
+        if (typeof schema !== "object" || schema === null) {
+            throw new TypeError("Rule's `meta.schema` must be an array or object");
+        }
+
+        // ESLint-specific array form needs to be converted into a valid JSON Schema definition
         if (Array.isArray(schema)) {
             if (schema.length) {
                 return {
@@ -64,16 +96,13 @@ export default class ConfigValidator {
                     maxItems: schema.length
                 };
             }
-            return {
-                type: "array",
-                minItems: 0,
-                maxItems: 0
-            };
 
+            // `schema:[]` is an explicit way to specify that the rule does not accept any options
+            return { ...noOptionsSchema };
         }
 
-        // Given a full schema, leave it alone
-        return schema || null;
+        // `schema:<object>` is assumed to be a valid JSON Schema definition
+        return schema;
     }
 
     /**
@@ -101,10 +130,18 @@ export default class ConfigValidator {
      */
     validateRuleSchema(rule, localOptions) {
         if (!ruleValidators.has(rule)) {
-            const schema = this.getRuleOptionsSchema(rule);
+            try {
+                const schema = this.getRuleOptionsSchema(rule);
+
+                if (schema) {
+                    ruleValidators.set(rule, ajv.compile(schema));
+                }
+            } catch (err) {
+                const errorWithCode = new Error(err.message, { cause: err });
 
-            if (schema) {
-                ruleValidators.set(rule, ajv.compile(schema));
+                errorWithCode.code = "ESLINT_INVALID_RULE_OPTIONS_SCHEMA";
+
+                throw errorWithCode;
             }
         }
 
@@ -137,13 +174,21 @@ export default class ConfigValidator {
                 this.validateRuleSchema(rule, Array.isArray(options) ? options.slice(1) : []);
             }
         } catch (err) {
-            const enhancedMessage = `Configuration for rule "${ruleId}" is invalid:\n${err.message}`;
+            let enhancedMessage = err.code === "ESLINT_INVALID_RULE_OPTIONS_SCHEMA"
+                ? `Error while processing options validation schema of rule '${ruleId}': ${err.message}`
+                : `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);
+                enhancedMessage = `${source}:\n\t${enhancedMessage}`;
             }
+
+            const enhancedError = new Error(enhancedMessage, { cause: err });
+
+            if (err.code) {
+                enhancedError.code = err.code;
+            }
+
+            throw enhancedError;
         }
     }
 

diff --git a/tests/fixtures/rules/custom-rule.cjs b/tests/fixtures/rules/custom-rule.cjs
@@ -1,15 +1,17 @@
-module.exports = function(context) {
+"use strict";
 
-    "use strict";
+module.exports = {
+    meta: {
+        schema: []
+    },
 
-    return {
-        "Identifier": function(node) {
-            if (node.name === "foo") {
-                context.report(node, "Identifier cannot be named 'foo'.");
+    create(context) {
+        return {
+            "Identifier": function(node) {
+                if (node.name === "foo") {
+                    context.report(node, "Identifier cannot be named 'foo'.");
+                }
             }
-        }
-    };
-
+        };
+    }
 };
-
-module.exports.schema = [];
diff --git a/tests/fixtures/rules/dir1/no-strings.cjs b/tests/fixtures/rules/dir1/no-strings.cjs
@@ -1,14 +1,15 @@
 "use strict";
 
-module.exports = function(context) {
+module.exports = {
+    create(context) {
+        return {
 
-    return {
+            "Literal": function(node) {
+                if (typeof node.value === 'string') {
+                    context.report(node, "String!");
+                }
 
-        "Literal": function(node) {
-            if (typeof node.value === 'string') {
-                context.report(node, "String!");
             }
-
-        }
-    };
+        };
+    }
 };
diff --git a/tests/fixtures/rules/dir2/no-literals.cjs b/tests/fixtures/rules/dir2/no-literals.cjs
@@ -1,11 +1,12 @@
 "use strict";
 
-module.exports = function(context) {
+module.exports = {
+    create (context) {
+        return {
 
-    return {
-
-        "Literal": function(node) {
-            context.report(node, "Literal!");
-        }
-    };
+            "Literal": function(node) {
+                context.report(node, "Literal!");
+            }
+        };
+    }
 };
diff --git a/tests/fixtures/rules/make-syntax-error-rule.cjs b/tests/fixtures/rules/make-syntax-error-rule.cjs
@@ -1,14 +1,18 @@
-module.exports = function(context) {
-    return {
-        Program: function(node) {
-            context.report({
-                node: node,
-                message: "ERROR",
-                fix: function(fixer) {
-                    return fixer.insertTextAfter(node, "this is a syntax error.");
-                }
-            });
-        }
-    };
+module.exports = {
+    meta: {
+        schema: []
+    },
+    create(context) {
+        return {
+            Program: function(node) {
+                context.report({
+                    node: node,
+                    message: "ERROR",
+                    fix: function(fixer) {
+                        return fixer.insertTextAfter(node, "this is a syntax error.");
+                    }
+                });
+            }
+        };
+    }
 };
-module.exports.schema = [];
diff --git a/tests/fixtures/rules/wrong/custom-rule.cjs b/tests/fixtures/rules/wrong/custom-rule.cjs
@@ -1,5 +1,7 @@
-module.exports = function() {
+"use strict";
 
-    "use strict";
-    return (null).something;
+module.exports = {
+    create() {
+        return (null).something;
+    }
 };