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

#17792) * drop support for function-style rules in flat-rule-tester * drop support for function-style rules in rule-tester * drop support for function-style rules in config-rule (eslint-fuzzer) * add rule object checks to rule testers * drop support for function-style rules in flat config and Linter#defineRule * update JSDoc * add rule object checks to Linter * drop support for function-style rules in eslintrc * remove custom-rules-deprecated.md * update flat config getRuleOptionsSchema for schema changes * add back and update custom-rules-deprecated.md * show ruleId in error messages for invalid schema * throw error in Linter if rule has invalid schema * add short description of meta.schema to type errors in flat-rule-tester * throw error for empty object schema in flat-rule-tester * update rule-tester for schema changes * add integration tests for eslintrc schema changes * add more flat-config-array unit tests for schema changes * throw error in Linter in eslintrc mode if rule has invalid schema * update docs for schema changes * use eslintrc v3
eslint · Dec 27, 2023 · fb81b1c · fb81b1c
1 parent 1da0723
commit fb81b1c
Show file tree

Hide file tree

Showing 36 changed files with 1,638 additions and 1,008 deletions.
diff --git a/docs/src/extend/custom-rules-deprecated.md b/docs/src/extend/custom-rules-deprecated.md
diff --git a/docs/src/extend/custom-rules.md b/docs/src/extend/custom-rules.md
@@ -10,8 +10,6 @@ eleventyNavigation:
 
 You can create custom rules to use with ESLint. You might want to create a custom rule if the [core rules](../rules/) do not cover your use case.
 
-**Note:** This page covers the most recent rule format for ESLint >= 3.0.0. There is also a [deprecated rule format](./custom-rules-deprecated).
-
 Here's the basic format of a custom rule:
 
 ```js
@@ -60,7 +58,7 @@ The source file for a rule exports an object with the following properties. Both
 
   **Important:** the `hasSuggestions` property is mandatory for rules that provide suggestions. If this property isn't set to `true`, ESLint will throw an error whenever the rule attempts to produce a suggestion. Omit the `hasSuggestions` property if the rule does not provide suggestions.
 
-* `schema`: (`object | array`) Specifies the [options](#options-schemas) so ESLint can prevent invalid [rule configurations](../use/configure/rules).
+* `schema`: (`object | array | false`) Specifies the [options](#options-schemas) so ESLint can prevent invalid [rule configurations](../use/configure/rules). Mandatory when the rule has options.
 
 * `deprecated`: (`boolean`) Indicates whether the rule has been deprecated.  You may omit the `deprecated` property if the rule has not been deprecated.
 
@@ -482,6 +480,13 @@ The `quotes` rule in this example has one option, `"double"` (the `error` is the
 
 ```js
 module.exports = {
+    meta: {
+        schema: [
+            {
+                enum: ["single", "double", "backtick"]
+            }
+        ]
+    },
     create: function(context) {
         var isDouble = (context.options[0] === "double");
 
@@ -494,6 +499,8 @@ Since `context.options` is just an array, you can use it to determine how many o
 
 When using options, make sure that your rule has some logical defaults in case the options are not provided.
 
+Rules with options must specify a [schema](#options-schemas).
+
 ### Accessing the Source Code
 
 The `SourceCode` object is the main object for getting more information about the source code being linted. You can retrieve the `SourceCode` object at any time by using the `context.sourceCode` property:
@@ -612,9 +619,11 @@ You can also access comments through many of `sourceCode`'s methods using the `i
 
 ### Options Schemas
 
-Rules may specify a `schema` property, which is a [JSON Schema](https://json-schema.org/) format description of a rule's options which will be used by ESLint to validate configuration options and prevent invalid or unexpected inputs before they are passed to the rule in `context.options`.
+Rules with options must specify a `meta.schema` property, which is a [JSON Schema](https://json-schema.org/) format description of a rule's options which will be used by ESLint to validate configuration options and prevent invalid or unexpected inputs before they are passed to the rule in `context.options`.
+
+If your rule has options, it is strongly recommended that you specify a schema for options validation. However, it is possible to opt-out of options validation by setting `schema: false`, but doing so is discouraged as it increases the chance of bugs and mistakes.
 
-Note: Prior to ESLint v9.0.0, rules without a schema are passed their options directly from the config without any validation. In ESLint v9.0.0 and later, rules without schemas will throw errors when options are passed. See the [Require schemas and object-style rules](https://github.com/eslint/rfcs/blob/main/designs/2021-schema-object-rules/README.md) RFC for further details.
+For rules that don't specify a `meta.schema` property, ESLint throws errors when any options are passed. If your rule doesn't have options, do not set `schema: false`, but simply omit the schema property or use `schema: []`, both of which prevent any options from being passed.
 
 When validating a rule's config, there are five steps:
 

diff --git a/lib/config/flat-config-helpers.js b/lib/config/flat-config-helpers.js
@@ -5,6 +5,23 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Typedefs
+//------------------------------------------------------------------------------
+
+/** @typedef {import("../shared/types").Rule} Rule */
+
+//------------------------------------------------------------------------------
+// Private Members
+//------------------------------------------------------------------------------
+
+// JSON schema that disallows passing any options
+const noOptionsSchema = Object.freeze({
+    type: "array",
+    minItems: 0,
+    maxItems: 0
+});
+
 //-----------------------------------------------------------------------------
 // Functions
 //-----------------------------------------------------------------------------
@@ -52,32 +69,39 @@ function getRuleFromConfig(ruleId, config) {
     const { pluginName, ruleName } = parseRuleId(ruleId);
 
     const plugin = config.plugins && config.plugins[pluginName];
-    let rule = plugin && plugin.rules && plugin.rules[ruleName];
-
-
-    // normalize function rules into objects
-    if (rule && typeof rule === "function") {
-        rule = {
-            create: rule
-        };
-    }
+    const rule = plugin && plugin.rules && plugin.rules[ruleName];
 
     return rule;
 }
 
 /**
  * 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 `meta.schema` is `false`.
  */
 function getRuleOptionsSchema(rule) {
 
-    if (!rule) {
+    if (!rule.meta) {
+        return { ...noOptionsSchema }; // default if `meta.schema` is not specified
+    }
+
+    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;
     }
 
-    const schema = rule.schema || rule.meta && rule.meta.schema;
+    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 {
@@ -87,16 +111,13 @@ function getRuleOptionsSchema(rule) {
                 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;
 }
 
 

diff --git a/lib/config/rule-validator.js b/lib/config/rule-validator.js
@@ -66,6 +66,25 @@ function throwRuleNotFoundError({ pluginName, ruleName }, config) {
     throw new TypeError(errorMessage);
 }
 
+/**
+ * The error type when a rule has an invalid `meta.schema`.
+ */
+class InvalidRuleOptionsSchemaError extends Error {
+
+    /**
+     * Creates a new instance.
+     * @param {string} ruleId Id of the rule that has an invalid `meta.schema`.
+     * @param {Error} processingError Error caught while processing the `meta.schema`.
+     */
+    constructor(ruleId, processingError) {
+        super(
+            `Error while processing options validation schema of rule '${ruleId}': ${processingError.message}`,
+            { cause: processingError }
+        );
+        this.code = "ESLINT_INVALID_RULE_OPTIONS_SCHEMA";
+    }
+}
+
 //-----------------------------------------------------------------------------
 // Exports
 //-----------------------------------------------------------------------------
@@ -130,10 +149,14 @@ class RuleValidator {
 
             // Precompile and cache validator the first time
             if (!this.validators.has(rule)) {
-                const schema = getRuleOptionsSchema(rule);
-
-                if (schema) {
-                    this.validators.set(rule, ajv.compile(schema));
+                try {
+                    const schema = getRuleOptionsSchema(rule);
+
+                    if (schema) {
+                        this.validators.set(rule, ajv.compile(schema));
+                    }
+                } catch (err) {
+                    throw new InvalidRuleOptionsSchemaError(ruleId, err);
                 }
             }
 

diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -439,6 +439,15 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
                         try {
                             validator.validateRuleOptions(rule, name, ruleValue);
                         } catch (err) {
+
+                            /*
+                             * If the rule has invalid `meta.schema`, throw the error because
+                             * this is not an invalid inline configuration but an invalid rule.
+                             */
+                            if (err.code === "ESLINT_INVALID_RULE_OPTIONS_SCHEMA") {
+                                throw err;
+                            }
+
                             problems.push(createLintingProblem({
                                 ruleId: name,
                                 message: err.message,
@@ -885,12 +894,18 @@ function parse(text, languageOptions, filePath) {
 
 /**
  * Runs a rule, and gets its listeners
- * @param {Rule} rule A normalized rule with a `create` method
+ * @param {Rule} rule A rule object
  * @param {Context} ruleContext The context that should be passed to the rule
+ * @throws {TypeError} If `rule` is not an object with a `create` method
  * @throws {any} Any error during the rule's `create`
  * @returns {Object} A map of selector listeners provided by the rule
  */
 function createRuleListeners(rule, ruleContext) {
+
+    if (!rule || typeof rule !== "object" || typeof rule.create !== "function") {
+        throw new TypeError(`Error while loading rule '${ruleContext.id}': Rule must be an object with a \`create\` method`);
+    }
+
     try {
         return rule.create(ruleContext);
     } catch (ex) {
@@ -1648,6 +1663,14 @@ class Linter {
                             mergedInlineConfig.rules[ruleId] = ruleValue;
                         } catch (err) {
 
+                            /*
+                             * If the rule has invalid `meta.schema`, throw the error because
+                             * this is not an invalid inline configuration but an invalid rule.
+                             */
+                            if (err.code === "ESLINT_INVALID_RULE_OPTIONS_SCHEMA") {
+                                throw err;
+                            }
+
                             let baseMessage = err.message.slice(
                                 err.message.startsWith("Key \"rules\":")
                                     ? err.message.indexOf(":", 12) + 1
@@ -1941,17 +1964,17 @@ class Linter {
     /**
      * Defines a new linting rule.
      * @param {string} ruleId A unique rule identifier
-     * @param {Function | Rule} ruleModule Function from context to object mapping AST node types to event handlers
+     * @param {Rule} rule A rule object
      * @returns {void}
      */
-    defineRule(ruleId, ruleModule) {
+    defineRule(ruleId, rule) {
         assertEslintrcConfig(this);
-        internalSlotsMap.get(this).ruleMap.define(ruleId, ruleModule);
+        internalSlotsMap.get(this).ruleMap.define(ruleId, rule);
     }
 
     /**
      * Defines many new linting rules.
-     * @param {Record<string, Function | Rule>} rulesToDefine map from unique rule identifier to rule
+     * @param {Record<string, Rule>} rulesToDefine map from unique rule identifier to rule
      * @returns {void}
      */
     defineRules(rulesToDefine) {

diff --git a/lib/linter/rules.js b/lib/linter/rules.js
@@ -13,18 +13,10 @@
 const builtInRules = require("../rules");
 
 //------------------------------------------------------------------------------
-// Helpers
+// Typedefs
 //------------------------------------------------------------------------------
 
-/**
- * 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;
-}
+/** @typedef {import("../shared/types").Rule} Rule */
 
 //------------------------------------------------------------------------------
 // Public Interface
@@ -41,18 +33,17 @@ class Rules {
     /**
      * Registers a rule module for rule id in storage.
      * @param {string} ruleId Rule id (file name).
-     * @param {Function} ruleModule Rule handler.
+     * @param {Rule} rule Rule object.
      * @returns {void}
      */
-    define(ruleId, ruleModule) {
-        this._rules[ruleId] = normalizeRule(ruleModule);
+    define(ruleId, rule) {
+        this._rules[ruleId] = rule;
     }
 
     /**
      * Access rule handler by id (file name).
      * @param {string} ruleId Rule id (file name).
-     * @returns {{create: Function, schema: JsonSchema[]}}
-     * A rule. This is normalized to always have the new-style shape with a `create` method.
+     * @returns {Rule} Rule object.
      */
     get(ruleId) {
         if (typeof this._rules[ruleId] === "string") {

diff --git a/lib/rule-tester/flat-rule-tester.js b/lib/rule-tester/flat-rule-tester.js
@@ -317,6 +317,14 @@ function throwForbiddenMethodError(methodName, prototype) {
     };
 }
 
+const metaSchemaDescription = `
+\t- If the rule has options, set \`meta.schema\` to an array or non-empty object to enable options validation.
+\t- If the rule doesn't have options, omit \`meta.schema\` to enforce that no options can be passed to the rule.
+\t- You can also set \`meta.schema\` to \`false\` to opt-out of options validation (not recommended).
+
+\thttps://eslint.org/docs/latest/extend/custom-rules#options-schemas
+`;
+
 //------------------------------------------------------------------------------
 // Public Interface
 //------------------------------------------------------------------------------
@@ -490,13 +498,13 @@ class FlatRuleTester {
     /**
      * Adds a new rule test to execute.
      * @param {string} ruleName The name of the rule to run.
-     * @param {Function | Rule} rule The rule to test.
+     * @param {Rule} rule The rule to test.
      * @param {{
      *   valid: (ValidTestCase | string)[],
      *   invalid: InvalidTestCase[]
      * }} test The collection of tests to run.
-     * @throws {TypeError|Error} If non-object `test`, or if a required
-     * scenario of the given type is missing.
+     * @throws {TypeError|Error} If `rule` is not an object with a `create` method,
+     * or if non-object `test`, or if a required scenario of the given type is missing.
      * @returns {void}
      */
     run(ruleName, rule, test) {
@@ -507,6 +515,10 @@ class FlatRuleTester {
             linter = this.linter,
             ruleId = `rule-to-test/${ruleName}`;
 
+        if (!rule || typeof rule !== "object" || typeof rule.create !== "function") {
+            throw new TypeError("Rule must be an object with a `create` method");
+        }
+
         if (!test || typeof test !== "object") {
             throw new TypeError(`Test Scenarios for rule ${ruleName} : Could not find test scenario object`);
         }
@@ -560,7 +572,7 @@ class FlatRuleTester {
 
                                     // freezeDeeply(context.languageOptions);
 
-                                    return (typeof rule === "function" ? rule : rule.create)(context);
+                                    return rule.create(context);
                                 }
                             })
                         }
@@ -652,7 +664,31 @@ class FlatRuleTester {
                 }
             });
 
-            const schema = getRuleOptionsSchema(rule);
+            let schema;
+
+            try {
+                schema = getRuleOptionsSchema(rule);
+            } catch (err) {
+                err.message += metaSchemaDescription;
+                throw err;
+            }
+
+            /*
+             * Check and throw an error if the schema is an empty object (`schema:{}`), because such schema
+             * doesn't validate or enforce anything and is therefore considered a possible error. If the intent
+             * was to skip options validation, `schema:false` should be set instead (explicit opt-out).
+             *
+             * For this purpose, a schema object is considered empty if it doesn't have any own enumerable string-keyed
+             * properties. While `ajv.compile()` does use enumerable properties from the prototype chain as well,
+             * it caches compiled schemas by serializing only own enumerable properties, so it's generally not a good idea
+             * to use inherited properties in schemas because schemas that differ only in inherited properties would end up
+             * having the same cache entry that would be correct for only one of them.
+             *
+             * At this point, `schema` can only be an object or `null`.
+             */
+            if (schema && Object.keys(schema).length === 0) {
+                throw new Error(`\`schema: {}\` is a no-op${metaSchemaDescription}`);
+            }
 
             /*
              * Setup AST getters.