fix!: configuration comments with just severity should retain options (…

…#17945)

* fix!: configuration comments with just severity should retain options

Fixes #17381

* update wording in migration guide

docs/src/use/migrate-to-9.0.0.md

@@ -23,6 +23,7 @@ The lists below are ordered roughly by the number of users each change is expect
 * [`eslint:recommended` has been updated](#eslint-recommended)
 * [`--quiet` no longer runs rules set to `"warn"`](#quiet-warn)
 * [Change in behavior when no patterns are passed to CLI](#cli-empty-patterns)
+* [`/* eslint */` comments with only severity now retain options from the config file](#eslint-comment-options)
 * [`no-constructor-return` and `no-sequences` rule schemas are stricter](#stricter-rule-schemas)
 * [New checks in `no-implicit-coercion` by default](#no-implicit-coercion)
 * [Case-sensitive flags in `no-invalid-regexp`](#no-invalid-regexp)
@@ -135,6 +136,46 @@ Prior to ESLint v9.0.0, running the ESLint CLI without any file or directory pat
 
 **Related issue(s):** [#14308](https://github.com/eslint/eslint/issues/14308)
 
+## <a name="eslint-comment-options"></a> `/* eslint */` comments with only severity now retain options from the config file
+
+Prior to ESLint v9.0.0, configuration comments such as `/* eslint curly: "warn" */` or `/* eslint curly: ["warn"] */` would completely override any configuration specified for the rule in the config file, and thus enforce the default options of the rule.
+
+In ESLint v9.0.0, the behavior of configuration comments is aligned with how rule configurations in config files are merged, meaning that a configuration comment with only severity now retains options specified in the config file and just overrides the severity.
+
+For example, if you have the following config file:
+
+```js
+// eslint.config.js
+
+export default [{
+    rules: {
+        curly: ["error", "multi"]
+    }
+}];
+```
+
+and the following configuration comment:
+
+```js
+// my-file.js
+
+/* eslint curly: "warn" */
+```
+
+the resulting configuration for the `curly` rule when linting `my-file.js` will be `curly: ["warn", "multi"]`.
+
+Note that this change only affects cases where the same rule is configured in the config file with options and using a configuration comment without options. In all other cases (e.g. the rule is only configured using a configuration comment), the behavior remains the same as prior to ESLint v9.0.0.
+
+**To address:** We expect that in most cases no change is necessary, as rules configured using configuration comments are typically not already configured in the config file. However, if you need a configuration comment to completely override configuration from the config file and enforce the default options, you'll need to specify at least one option:
+
+```js
+// my-file.js
+
+/* eslint curly: ["warn", "all"] */
+```
+
+**Related issue(s):** [#17381](https://github.com/eslint/eslint/issues/17381)
+
 ## <a name="stricter-rule-schemas"></a> `no-constructor-return` and `no-sequences` rule schemas are stricter
 
 In previous versions of ESLint, `no-constructor-return` and `no-sequences` rules were mistakenly accepting invalid options.

lib/linter/linter.js

@@ -1331,7 +1331,56 @@ class Linter {
             { exportedVariables: commentDirectives.exportedVariables, enabledGlobals: commentDirectives.enabledGlobals }
         );
 
+        /*
+         * Now we determine the final configurations for rules.
+         * First, let all inline rule configurations override those from the config.
+         * Then, check for a special case: if a rule is configured in both places,
+         * inline rule configuration that only has severity should retain options from
+         * the config and just override the severity.
+         *
+         * Example:
+         *
+         *   {
+         *       rules: {
+         *           curly: ["error", "multi"]
+         *       }
+         *   }
+         *
+         *   /* eslint curly: ["warn"] * /
+         *
+         *   Results in:
+         *
+         *   curly: ["warn", "multi"]
+         */
         const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);
+
+        if (config.rules) {
+            for (const [ruleId, ruleInlineConfig] of Object.entries(commentDirectives.configuredRules)) {
+                if (
+
+                    /*
+                     * If inline config for the rule has only severity
+                     */
+                    (!Array.isArray(ruleInlineConfig) || ruleInlineConfig.length === 1) &&
+
+                    /*
+                     * And provided config for the rule has options
+                     */
+                    Object.hasOwn(config.rules, ruleId) &&
+                    (Array.isArray(config.rules[ruleId]) && config.rules[ruleId].length > 1)
+                ) {
+
+                    /*
+                     * Then use severity from the inline config and options from the provided config
+                     */
+                    configuredRules[ruleId] = [
+                        Array.isArray(ruleInlineConfig) ? ruleInlineConfig[0] : ruleInlineConfig, // severity from the inline config
+                        ...config.rules[ruleId].slice(1) // options from the provided config
+                    ];
+                }
+            }
+        }
+
         let lintingProblems;
 
         try {
@@ -1674,7 +1723,7 @@ class Linter {
                                     [ruleId]: ruleOptions
                                 }
                             });
-                            mergedInlineConfig.rules[ruleId] = ruleValue;
+                            mergedInlineConfig.rules[ruleId] = ruleOptions;
                         } catch (err) {
 
                             /*
@@ -1713,7 +1762,58 @@ class Linter {
             )
             : { problems: [], disableDirectives: [] };
 
+        /*
+         * Now we determine the final configurations for rules.
+         * First, let all inline rule configurations override those from the config.
+         * Then, check for a special case: if a rule is configured in both places,
+         * inline rule configuration that only has severity should retain options from
+         * the config and just override the severity.
+         *
+         * Example:
+         *
+         *   {
+         *       rules: {
+         *           curly: ["error", "multi"]
+         *       }
+         *   }
+         *
+         *   /* eslint curly: ["warn"] * /
+         *
+         *   Results in:
+         *
+         *   curly: ["warn", "multi"]
+         *
+         * At this point, all rule configurations are normalized to arrays.
+         */
         const configuredRules = Object.assign({}, config.rules, mergedInlineConfig.rules);
+
+        if (config.rules) {
+            for (const [ruleId, ruleInlineConfig] of Object.entries(mergedInlineConfig.rules)) {
+                if (
+
+                    /*
+                     * If inline config for the rule has only severity
+                     */
+                    ruleInlineConfig.length === 1 &&
+
+                    /*
+                     * And provided config for the rule has options
+                     */
+                    Object.hasOwn(config.rules, ruleId) &&
+                    config.rules[ruleId].length > 1
+                ) {
+
+                    /*
+                     * Then use severity from the inline config and options from the provided config
+                     */
+                    configuredRules[ruleId] = [
+                        ruleInlineConfig[0], // severity from the inline config
+                        ...config.rules[ruleId].slice(1) // options from the provided config
+                    ];
+                }
+            }
+        }
+
         let lintingProblems;
 
         sourceCode.finalize();

tests/lib/linter/linter.js

@@ -1455,6 +1455,90 @@ describe("Linter", () => {
             assert.strictEqual(messages.length, 1);
             assert.strictEqual(suppressedMessages.length, 0);
         });
+
+        describe("when the rule was already configured", () => {
+
+            beforeEach(() => {
+                linter.defineRule("my-rule", {
+                    meta: {
+                        schema: [{
+                            type: "string"
+                        }]
+                    },
+                    create(context) {
+                        const message = context.options[0] ?? "option not provided";
+
+                        return {
+                            Program(node) {
+                                context.report({ node, message });
+                            }
+                        };
+                    }
+                });
+            });
+
+            [
+                "off",
+                "warn",
+                "error",
+                ["off"],
+                ["warn"],
+                ["error"],
+                ["off", "bar"],
+                ["warn", "bar"],
+                ["error", "bar"]
+            ].forEach(ruleConfig => {
+                const config = {
+                    rules: {
+                        "my-rule": ruleConfig
+                    }
+                };
+
+                it(`severity from the /*eslint*/ comment and options from the config should apply when the comment has only severity (original config: ${JSON.stringify(ruleConfig)})`, () => {
+                    const code = "/*eslint my-rule: 'warn' */";
+                    const messages = linter.verify(code, config);
+                    const suppressedMessages = linter.getSuppressedMessages();
+
+                    const expectedMessage = Array.isArray(ruleConfig) && ruleConfig.length > 1
+                        ? ruleConfig[1]
+                        : "option not provided";
+
+                    assert.strictEqual(messages.length, 1);
+                    assert.strictEqual(messages[0].ruleId, "my-rule");
+                    assert.strictEqual(messages[0].severity, 1);
+                    assert.strictEqual(messages[0].message, expectedMessage);
+                    assert.strictEqual(suppressedMessages.length, 0);
+                });
+
+                it(`severity from the /*eslint*/ comment and options from the config should apply when the comment has array with only severity (original config: ${JSON.stringify(ruleConfig)})`, () => {
+                    const code = "/*eslint my-rule: ['warn'] */";
+                    const messages = linter.verify(code, config);
+                    const suppressedMessages = linter.getSuppressedMessages();
+
+                    const expectedMessage = Array.isArray(ruleConfig) && ruleConfig.length > 1
+                        ? ruleConfig[1]
+                        : "option not provided";
+
+                    assert.strictEqual(messages.length, 1);
+                    assert.strictEqual(messages[0].ruleId, "my-rule");
+                    assert.strictEqual(messages[0].severity, 1);
+                    assert.strictEqual(messages[0].message, expectedMessage);
+                    assert.strictEqual(suppressedMessages.length, 0);
+                });
+
+                it(`severity and options from the /*eslint*/ comment should apply when the comment includes options (original config: ${JSON.stringify(ruleConfig)})`, () => {
+                    const code = "/*eslint my-rule: ['warn', 'foo'] */";
+                    const messages = linter.verify(code, config);
+                    const suppressedMessages = linter.getSuppressedMessages();
+
+                    assert.strictEqual(messages.length, 1);
+                    assert.strictEqual(messages[0].ruleId, "my-rule");
+                    assert.strictEqual(messages[0].severity, 1);
+                    assert.strictEqual(messages[0].message, "foo");
+                    assert.strictEqual(suppressedMessages.length, 0);
+                });
+            });
+        });
     });
 
     describe("when evaluating code with invalid comments to enable rules", () => {
@@ -10625,6 +10709,94 @@ describe("Linter with FlatConfigArray", () => {
                         assert.strictEqual(messages.length, 1);
                         assert.strictEqual(suppressedMessages.length, 0);
                     });
+
+                    describe("when the rule was already configured", () => {
+                        const plugin = {
+                            rules: {
+                                "my-rule": {
+                                    meta: {
+                                        schema: [{
+                                            type: "string"
+                                        }]
+                                    },
+                                    create(context) {
+                                        const message = context.options[0] ?? "option not provided";
+
+                                        return {
+                                            Program(node) {
+                                                context.report({ node, message });
+                                            }
+                                        };
+                                    }
+                                }
+                            }
+                        };
+
+                        [
+                            "off",
+                            "warn",
+                            "error",
+                            ["off"],
+                            ["warn"],
+                            ["error"],
+                            ["off", "bar"],
+                            ["warn", "bar"],
+                            ["error", "bar"]
+                        ].forEach(ruleConfig => {
+                            const config = {
+                                plugins: {
+                                    test: plugin
+                                },
+                                rules: {
+                                    "test/my-rule": ruleConfig
+                                }
+                            };
+
+                            it(`severity from the /*eslint*/ comment and options from the config should apply when the comment has only severity (original config: ${JSON.stringify(ruleConfig)})`, () => {
+                                const code = "/*eslint test/my-rule: 'warn' */";
+                                const messages = linter.verify(code, config);
+                                const suppressedMessages = linter.getSuppressedMessages();
+
+                                const expectedMessage = Array.isArray(ruleConfig) && ruleConfig.length > 1
+                                    ? ruleConfig[1]
+                                    : "option not provided";
+
+                                assert.strictEqual(messages.length, 1);
+                                assert.strictEqual(messages[0].ruleId, "test/my-rule");
+                                assert.strictEqual(messages[0].severity, 1);
+                                assert.strictEqual(messages[0].message, expectedMessage);
+                                assert.strictEqual(suppressedMessages.length, 0);
+                            });
+
+                            it(`severity from the /*eslint*/ comment and options from the config should apply when the comment has array with only severity (original config: ${JSON.stringify(ruleConfig)})`, () => {
+                                const code = "/*eslint test/my-rule: ['warn'] */";
+                                const messages = linter.verify(code, config);
+                                const suppressedMessages = linter.getSuppressedMessages();
+
+                                const expectedMessage = Array.isArray(ruleConfig) && ruleConfig.length > 1
+                                    ? ruleConfig[1]
+                                    : "option not provided";
+
+                                assert.strictEqual(messages.length, 1);
+                                assert.strictEqual(messages[0].ruleId, "test/my-rule");
+                                assert.strictEqual(messages[0].severity, 1);
+                                assert.strictEqual(messages[0].message, expectedMessage);
+                                assert.strictEqual(suppressedMessages.length, 0);
+                            });
+
+                            it(`severity and options from the /*eslint*/ comment should apply when the comment includes options (original config: ${JSON.stringify(ruleConfig)})`, () => {
+                                const code = "/*eslint test/my-rule: ['warn', 'foo'] */";
+                                const messages = linter.verify(code, config);
+                                const suppressedMessages = linter.getSuppressedMessages();
+
+                                assert.strictEqual(messages.length, 1);
+                                assert.strictEqual(messages[0].ruleId, "test/my-rule");
+                                assert.strictEqual(messages[0].severity, 1);
+                                assert.strictEqual(messages[0].message, "foo");
+                                assert.strictEqual(suppressedMessages.length, 0);
+                            });
+                        });
+                    });
                 });
 
                 describe("when evaluating code with invalid comments to enable rules", () => {