feat!: disallow multiple configuration comments for same rule (#18157)

* feat!: disallow multiple configuration comments for same rule

Fixes #18132

* update lint error message

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* update tests

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

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

@@ -25,6 +25,7 @@ The lists below are ordered roughly by the number of users each change is expect
 * [`--output-file` now writes a file to disk even with an empty output](#output-file)
 * [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)
+* [Multiple `/* eslint */` comments for the same rule are now disallowed](#multiple-eslint-comments)
 * [Stricter `/* exported */` parsing](#exported-parsing)
 * [`no-constructor-return` and `no-sequences` rule schemas are stricter](#stricter-rule-schemas)
 * [New checks in `no-implicit-coercion` by default](#no-implicit-coercion)
@@ -190,6 +191,30 @@ Note that this change only affects cases where the same rule is configured in th
 
 **Related issue(s):** [#17381](https://github.com/eslint/eslint/issues/17381)
 
+## <a name="multiple-eslint-comments"></a> Multiple `/* eslint */` comments for the same rule are now disallowed
+
+Prior to ESLint v9.0.0, if the file being linted contained multiple `/* eslint */` configuration comments for the same rule, the last one would be applied, while the others would be silently ignored. For example:
+
+```js
+/* eslint semi: ["error", "always"] */
+/* eslint semi: ["error", "never"] */
+
+foo() // valid, because the configuration is "never"
+```
+
+In ESLint v9.0.0, the first one is applied, while the others are reported as lint errors:
+
+```js
+/* eslint semi: ["error", "always"] */
+/* eslint semi: ["error", "never"] */ // error: Rule "semi" is already configured by another configuration comment in the preceding code. This configuration is ignored.
+
+foo() // error: Missing semicolon
+```
+
+**To address:** Remove duplicate `/* eslint */` comments.
+
+**Related issue(s):** [#18132](https://github.com/eslint/eslint/issues/18132)
+
 ## <a name="exported-parsing"></a> Stricter `/* exported */` parsing
 
 Prior to ESLint v9.0.0, the `/* exported */` directive incorrectly allowed the following syntax:

lib/linter/linter.js

@@ -439,6 +439,14 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
                             return;
                         }
 
+                        if (Object.hasOwn(configuredRules, name)) {
+                            problems.push(createLintingProblem({
+                                message: `Rule "${name}" is already configured by another configuration comment in the preceding code. This configuration is ignored.`,
+                                loc: comment.loc
+                            }));
+                            return;
+                        }
+
                         let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];
 
                         /*
@@ -1706,6 +1714,14 @@ class Linter {
                             return;
                         }
 
+                        if (Object.hasOwn(mergedInlineConfig.rules, ruleId)) {
+                            inlineConfigProblems.push(createLintingProblem({
+                                message: `Rule "${ruleId}" is already configured by another configuration comment in the preceding code. This configuration is ignored.`,
+                                loc: node.loc
+                            }));
+                            return;
+                        }
+
                         try {
 
                             let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];

tests/lib/linter/linter.js

@@ -1877,6 +1877,156 @@ describe("Linter", () => {
         });
     });
 
+    describe("when evaluating code with multiple configuration comments for same rule", () => {
+
+        beforeEach(() => {
+            linter.defineRule("no-foo", {
+                meta: {
+                    schema: [{
+                        enum: ["bar", "baz", "qux"]
+                    }]
+                },
+                create(context) {
+                    const replacement = context.options[0] ?? "default";
+
+                    return {
+                        "Identifier[name='foo']"(node) {
+                            context.report(node, `Replace 'foo' with '${replacement}'.`);
+                        }
+                    };
+                }
+            });
+        });
+
+        it("should apply the first and report an error for the second when there are two", () => {
+            const code = "/*eslint no-foo: ['error', 'bar']*/ /*eslint no-foo: ['error', 'baz']*/ foo;";
+
+            const messages = linter.verify(code);
+            const suppressedMessages = linter.getSuppressedMessages();
+
+            assert.deepStrictEqual(messages, [
+                {
+                    ruleId: null,
+                    severity: 2,
+                    message: "Rule \"no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                    line: 1,
+                    column: 37,
+                    endLine: 1,
+                    endColumn: 72,
+                    nodeType: null
+                },
+                {
+                    ruleId: "no-foo",
+                    severity: 2,
+                    message: "Replace 'foo' with 'bar'.",
+                    line: 1,
+                    column: 73,
+                    endLine: 1,
+                    endColumn: 76,
+                    nodeType: "Identifier"
+                }
+            ]);
+            assert.strictEqual(suppressedMessages.length, 0);
+        });
+
+        it("should apply the first and report an error for each other when there are more than two", () => {
+            const code = "/*eslint no-foo: ['error', 'bar']*/ /*eslint no-foo: ['error', 'baz']*/ /*eslint no-foo: ['error', 'qux']*/ foo;";
+
+            const messages = linter.verify(code);
+            const suppressedMessages = linter.getSuppressedMessages();
+
+            assert.deepStrictEqual(messages, [
+                {
+                    ruleId: null,
+                    severity: 2,
+                    message: "Rule \"no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                    line: 1,
+                    column: 37,
+                    endLine: 1,
+                    endColumn: 72,
+                    nodeType: null
+                },
+                {
+                    ruleId: null,
+                    severity: 2,
+                    message: "Rule \"no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                    line: 1,
+                    column: 73,
+                    endLine: 1,
+                    endColumn: 108,
+                    nodeType: null
+                },
+                {
+                    ruleId: "no-foo",
+                    severity: 2,
+                    message: "Replace 'foo' with 'bar'.",
+                    line: 1,
+                    column: 109,
+                    endLine: 1,
+                    endColumn: 112,
+                    nodeType: "Identifier"
+                }
+            ]);
+            assert.strictEqual(suppressedMessages.length, 0);
+        });
+
+        it("should apply the first and report an error for the second when both just override severity", () => {
+            const code = "/*eslint no-foo: 'warn'*/ /*eslint no-foo: 'error'*/ foo;";
+
+            const messages = linter.verify(code, { rules: { "no-foo": ["error", "bar"] } });
+            const suppressedMessages = linter.getSuppressedMessages();
+
+            assert.deepStrictEqual(messages, [
+                {
+                    ruleId: null,
+                    severity: 2,
+                    message: "Rule \"no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                    line: 1,
+                    column: 27,
+                    endLine: 1,
+                    endColumn: 53,
+                    nodeType: null
+                },
+                {
+                    ruleId: "no-foo",
+                    severity: 1,
+                    message: "Replace 'foo' with 'bar'.",
+                    line: 1,
+                    column: 54,
+                    endLine: 1,
+                    endColumn: 57,
+                    nodeType: "Identifier"
+                }
+            ]);
+            assert.strictEqual(suppressedMessages.length, 0);
+        });
+
+        it("should apply the second if the first has an invalid configuration", () => {
+            const code = "/*eslint no-foo: ['error', 'quux']*/ /*eslint no-foo: ['error', 'bar']*/ foo;";
+
+            const messages = linter.verify(code);
+            const suppressedMessages = linter.getSuppressedMessages();
+
+            assert.strictEqual(messages.length, 2);
+            assert.include(messages[0].message, "Configuration for rule \"no-foo\" is invalid");
+            assert.strictEqual(messages[1].message, "Replace 'foo' with 'bar'.");
+            assert.strictEqual(suppressedMessages.length, 0);
+        });
+
+        it("should apply configurations for other rules that are in the same comment as the duplicate", () => {
+            const code = "/*eslint no-foo: ['error', 'bar']*/ /*eslint no-foo: ['error', 'baz'], no-alert: ['error']*/ foo; alert();";
+
+            const messages = linter.verify(code);
+            const suppressedMessages = linter.getSuppressedMessages();
+
+            assert.strictEqual(messages.length, 3);
+            assert.strictEqual(messages[0].message, "Rule \"no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.");
+            assert.strictEqual(messages[1].message, "Replace 'foo' with 'bar'.");
+            assert.strictEqual(messages[2].ruleId, "no-alert");
+            assert.strictEqual(suppressedMessages.length, 0);
+        });
+    });
+
     describe("when evaluating code with comments to enable and disable all reporting", () => {
         it("should report a violation", () => {
 
@@ -11225,6 +11375,166 @@ describe("Linter with FlatConfigArray", () => {
                     });
                 });
 
+                describe("when evaluating code with multiple configuration comments for same rule", () => {
+
+                    let baseConfig;
+
+                    beforeEach(() => {
+                        baseConfig = {
+                            plugins: {
+                                "test-plugin": {
+                                    rules: {
+                                        "no-foo": {
+                                            meta: {
+                                                schema: [{
+                                                    enum: ["bar", "baz", "qux"]
+                                                }]
+                                            },
+                                            create(context) {
+                                                const replacement = context.options[0] ?? "default";
+
+                                                return {
+                                                    "Identifier[name='foo']"(node) {
+                                                        context.report(node, `Replace 'foo' with '${replacement}'.`);
+                                                    }
+                                                };
+                                            }
+                                        }
+                                    }
+                                }
+                            }
+                        };
+                    });
+
+                    it("should apply the first and report an error for the second when there are two", () => {
+                        const code = "/*eslint test-plugin/no-foo: ['error', 'bar']*/ /*eslint test-plugin/no-foo: ['error', 'baz']*/ foo;";
+
+                        const messages = linter.verify(code, baseConfig);
+                        const suppressedMessages = linter.getSuppressedMessages();
+
+                        assert.deepStrictEqual(messages, [
+                            {
+                                ruleId: null,
+                                severity: 2,
+                                message: "Rule \"test-plugin/no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                                line: 1,
+                                column: 49,
+                                endLine: 1,
+                                endColumn: 96,
+                                nodeType: null
+                            },
+                            {
+                                ruleId: "test-plugin/no-foo",
+                                severity: 2,
+                                message: "Replace 'foo' with 'bar'.",
+                                line: 1,
+                                column: 97,
+                                endLine: 1,
+                                endColumn: 100,
+                                nodeType: "Identifier"
+                            }
+                        ]);
+                        assert.strictEqual(suppressedMessages.length, 0);
+                    });
+
+                    it("should apply the first and report an error for each other when there are more than two", () => {
+                        const code = "/*eslint test-plugin/no-foo: ['error', 'bar']*/ /*eslint test-plugin/no-foo: ['error', 'baz']*/ /*eslint test-plugin/no-foo: ['error', 'qux']*/ foo;";
+
+                        const messages = linter.verify(code, baseConfig);
+                        const suppressedMessages = linter.getSuppressedMessages();
+
+                        assert.deepStrictEqual(messages, [
+                            {
+                                ruleId: null,
+                                severity: 2,
+                                message: "Rule \"test-plugin/no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                                line: 1,
+                                column: 49,
+                                endLine: 1,
+                                endColumn: 96,
+                                nodeType: null
+                            },
+                            {
+                                ruleId: null,
+                                severity: 2,
+                                message: "Rule \"test-plugin/no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                                line: 1,
+                                column: 97,
+                                endLine: 1,
+                                endColumn: 144,
+                                nodeType: null
+                            },
+                            {
+                                ruleId: "test-plugin/no-foo",
+                                severity: 2,
+                                message: "Replace 'foo' with 'bar'.",
+                                line: 1,
+                                column: 145,
+                                endLine: 1,
+                                endColumn: 148,
+                                nodeType: "Identifier"
+                            }
+                        ]);
+                        assert.strictEqual(suppressedMessages.length, 0);
+                    });
+
+                    it("should apply the first and report an error for the second when both just override severity", () => {
+                        const code = "/*eslint test-plugin/no-foo: 'warn'*/ /*eslint test-plugin/no-foo: 'error'*/ foo;";
+
+                        const messages = linter.verify(code, { ...baseConfig, rules: { "test-plugin/no-foo": ["error", "bar"] } });
+                        const suppressedMessages = linter.getSuppressedMessages();
+
+                        assert.deepStrictEqual(messages, [
+                            {
+                                ruleId: null,
+                                severity: 2,
+                                message: "Rule \"test-plugin/no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.",
+                                line: 1,
+                                column: 39,
+                                endLine: 1,
+                                endColumn: 77,
+                                nodeType: null
+                            },
+                            {
+                                ruleId: "test-plugin/no-foo",
+                                severity: 1,
+                                message: "Replace 'foo' with 'bar'.",
+                                line: 1,
+                                column: 78,
+                                endLine: 1,
+                                endColumn: 81,
+                                nodeType: "Identifier"
+                            }
+                        ]);
+                        assert.strictEqual(suppressedMessages.length, 0);
+                    });
+
+                    it("should apply the second if the first has an invalid configuration", () => {
+                        const code = "/*eslint test-plugin/no-foo: ['error', 'quux']*/ /*eslint test-plugin/no-foo: ['error', 'bar']*/ foo;";
+
+                        const messages = linter.verify(code, baseConfig);
+                        const suppressedMessages = linter.getSuppressedMessages();
+
+                        assert.strictEqual(messages.length, 2);
+                        assert.include(messages[0].message, "Inline configuration for rule \"test-plugin/no-foo\" is invalid");
+                        assert.strictEqual(messages[1].message, "Replace 'foo' with 'bar'.");
+                        assert.strictEqual(suppressedMessages.length, 0);
+                    });
+
+                    it("should apply configurations for other rules that are in the same comment as the duplicate", () => {
+                        const code = "/*eslint test-plugin/no-foo: ['error', 'bar']*/ /*eslint test-plugin/no-foo: ['error', 'baz'], no-alert: ['error']*/ foo; alert();";
+
+                        const messages = linter.verify(code, baseConfig);
+                        const suppressedMessages = linter.getSuppressedMessages();
+
+                        assert.strictEqual(messages.length, 3);
+                        assert.strictEqual(messages[0].message, "Rule \"test-plugin/no-foo\" is already configured by another configuration comment in the preceding code. This configuration is ignored.");
+                        assert.strictEqual(messages[1].message, "Replace 'foo' with 'bar'.");
+                        assert.strictEqual(messages[2].ruleId, "no-alert");
+                        assert.strictEqual(suppressedMessages.length, 0);
+                    });
+                });
+
                 describe("when evaluating code with comments to enable and disable all reporting", () => {
                     it("should report a violation", () => {