feat!: detect duplicate test cases

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

@@ -463,10 +463,11 @@ In order to aid in the development of high-quality custom rules that are free fr
 
 1. **Suggestion messages must be unique.** Because suggestions are typically displayed in an editor as a dropdown list, it's important that no two suggestions for the same lint problem have the same message. Otherwise, it's impossible to know what any given suggestion will do. This additional check runs automatically.
 1. **Suggestions must generate valid syntax.** In order for rule suggestions to be helpful, they need to be valid syntax. `RuleTester` now parses the output of suggestions using the same language options as the `code` value and throws an error if parsing fails.
+1. **Test cases must be unique.** Identical test cases can cause confusion and be hard to detect manually in a long test file. Duplicates are now automatically detected and can be safely removed.
 
 **To address:** Run your rule tests using `RuleTester` and fix any errors that occur. The changes you'll need to make to satisfy `RuleTester` are compatible with ESLint v8.x.
 
-**Related Issues(s):** [#15735](https://github.com/eslint/eslint/issues/15735), [#16908](https://github.com/eslint/eslint/issues/16908)
+**Related Issues(s):** [#15104](https://github.com/eslint/eslint/issues/15104), [#15735](https://github.com/eslint/eslint/issues/15735), [#16908](https://github.com/eslint/eslint/issues/16908)
 
 ## <a name="flat-eslint"></a> `FlatESLint` is now `ESLint`
 

lib/rule-tester/rule-tester.js

@@ -16,7 +16,8 @@ const
     equal = require("fast-deep-equal"),
     Traverser = require("../shared/traverser"),
     { getRuleOptionsSchema } = require("../config/flat-config-helpers"),
-    { Linter, SourceCodeFixer, interpolate } = require("../linter");
+    { Linter, SourceCodeFixer, interpolate } = require("../linter"),
+    stringify = require("json-stable-stringify-without-jsonify");
 
 const { FlatConfigArray } = require("../config/flat-config-array");
 const { defaultConfig } = require("../config/default-config");
@@ -26,6 +27,7 @@ const ajv = require("../shared/ajv")({ strictDefaults: true });
 const parserSymbol = Symbol.for("eslint.RuleTester.parser");
 const { SourceCode } = require("../source-code");
 const { ConfigArraySymbol } = require("@humanwhocodes/config-array");
+const { isSerializable } = require("../shared/serialization");
 
 //------------------------------------------------------------------------------
 // Typedefs
@@ -499,6 +501,9 @@ class RuleTester {
             linter = this.linter,
             ruleId = `rule-to-test/${ruleName}`;
 
+        const seenValidTestCases = new Set();
+        const seenInvalidTestCases = new Set();
+
         if (!rule || typeof rule !== "object" || typeof rule.create !== "function") {
             throw new TypeError("Rule must be an object with a `create` method");
         }
@@ -794,6 +799,32 @@ class RuleTester {
             }
         }
 
+        /**
+         * Check if this test case is a duplicate of one we have seen before.
+         * @param {Object} item test case object
+         * @param {Set<string>} seenTestCases set of serialized test cases we have seen so far (managed by this function)
+         * @returns {void}
+         * @private
+         */
+        function checkDuplicateTestCase(item, seenTestCases) {
+            if (!isSerializable(item)) {
+
+                /*
+                 * If we can't serialize a test case (because it contains a function, RegExp, etc), skip the check.
+                 * This might happen with properties like: options, plugins, settings, languageOptions.parser, languageOptions.parserOptions.
+                 */
+                return;
+            }
+
+            const serializedTestCase = stringify(item);
+
+            assert(
+                !seenTestCases.has(serializedTestCase),
+                "detected duplicate test case"
+            );
+            seenTestCases.add(serializedTestCase);
+        }
+
         /**
          * Check if the template is valid or not
          * all valid cases go through this
@@ -809,6 +840,8 @@ class RuleTester {
                 assert.ok(typeof item.name === "string", "Optional test case property 'name' must be a string");
             }
 
+            checkDuplicateTestCase(item, seenValidTestCases);
+
             const result = runRuleForItem(item);
             const messages = result.messages;
 
@@ -860,6 +893,8 @@ class RuleTester {
                 assert.fail("Invalid cases must have at least one error");
             }
 
+            checkDuplicateTestCase(item, seenInvalidTestCases);
+
             const ruleHasMetaMessages = hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages");
             const friendlyIDList = ruleHasMetaMessages ? `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]` : null;
 

lib/shared/serialization.js

@@ -0,0 +1,53 @@
+/**
+ * @fileoverview Object serialization utils.
+ * @author Bryan Mishkin
+ */
+
+"use strict";
+
+/**
+ * Check if a value is a primitive or plain object created by the Object constructor.
+ * @param {any} val the value to check
+ * @returns {boolean} true if so
+ * @private
+ */
+function isSerializablePrimitiveOrPlainObject(val) {
+    return (
+        val === null ||
+        typeof val === "string" ||
+         typeof val === "boolean" ||
+         typeof val === "number" ||
+         (typeof val === "object" && val.constructor === Object) ||
+         Array.isArray(val)
+    );
+}
+
+/**
+ * Check if an object is serializable.
+ * Functions or objects like RegExp cannot be serialized by JSON.stringify().
+ * Inspired by: https://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript
+ * @param {any} obj the object
+ * @returns {boolean} true if the object is serializable
+ */
+function isSerializable(obj) {
+    if (!isSerializablePrimitiveOrPlainObject(obj)) {
+        return false;
+    }
+    for (const property in obj) {
+        if (Object.hasOwn(obj, property)) {
+            if (!isSerializablePrimitiveOrPlainObject(obj[property])) {
+                return false;
+            }
+            if (typeof obj[property] === "object") {
+                if (!isSerializable(obj[property])) {
+                    return false;
+                }
+            }
+        }
+    }
+    return true;
+}
+
+module.exports = {
+    isSerializable
+};

tests/lib/rule-tester/rule-tester.js

@@ -2830,6 +2830,200 @@ describe("RuleTester", () => {
 
     });
 
+    describe("duplicate test cases", () => {
+        describe("valid test cases", () => {
+            it("throws with duplicate string test cases", () => {
+                assert.throws(() => {
+                    ruleTester.run("foo", {
+                        meta: {},
+                        create() {
+                            return {};
+                        }
+                    }, {
+                        valid: ["foo", "foo"],
+                        invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }]
+                    });
+                }, "detected duplicate test case");
+            });
+
+            it("throws with duplicate object test cases", () => {
+                assert.throws(() => {
+                    ruleTester.run("foo", {
+                        meta: {},
+                        create() {
+                            return {};
+                        }
+                    }, {
+                        valid: [{ code: "foo" }, { code: "foo" }],
+                        invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }]
+                    });
+                }, "detected duplicate test case");
+            });
+        });
+
+        describe("invalid test cases", () => {
+            it("throws with duplicate object test cases", () => {
+                assert.throws(() => {
+                    ruleTester.run("foo", {
+                        meta: {},
+                        create(context) {
+                            return {
+                                VariableDeclaration(node) {
+                                    context.report(node, "foo bar");
+                                }
+                            };
+                        }
+                    }, {
+                        valid: ["foo"],
+                        invalid: [
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }] },
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }] }
+                        ]
+                    });
+                }, "detected duplicate test case");
+            });
+
+            it("throws with duplicate object test cases when options is a primitive", () => {
+                assert.throws(() => {
+                    ruleTester.run("foo", {
+                        meta: { schema: false },
+                        create(context) {
+                            return {
+                                VariableDeclaration(node) {
+                                    context.report(node, "foo bar");
+                                }
+                            };
+                        }
+                    }, {
+                        valid: ["foo"],
+                        invalid: [
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }], options: ["abc"] },
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }], options: ["abc"] }
+                        ]
+                    });
+                }, "detected duplicate test case");
+            });
+
+            it("throws with duplicate object test cases when options is a nested serializable object", () => {
+                assert.throws(() => {
+                    ruleTester.run("foo", {
+                        meta: { schema: false },
+                        create(context) {
+                            return {
+                                VariableDeclaration(node) {
+                                    context.report(node, "foo bar");
+                                }
+                            };
+                        }
+                    }, {
+                        valid: ["foo"],
+                        invalid: [
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }], options: [{ foo: [{ a: true, b: [1, 2, 3] }] }] },
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }], options: [{ foo: [{ a: true, b: [1, 2, 3] }] }] }
+                        ]
+                    });
+                }, "detected duplicate test case");
+            });
+
+            it("throws with duplicate object test cases even when property order differs", () => {
+                assert.throws(() => {
+                    ruleTester.run("foo", {
+                        meta: {},
+                        create(context) {
+                            return {
+                                VariableDeclaration(node) {
+                                    context.report(node, "foo bar");
+                                }
+                            };
+                        }
+                    }, {
+                        valid: ["foo"],
+                        invalid: [
+                            { code: "const x = 123;", errors: [{ message: "foo bar" }] },
+                            { errors: [{ message: "foo bar" }], code: "const x = 123;" }
+                        ]
+                    });
+                }, "detected duplicate test case");
+            });
+
+            it("ignores duplicate test case when non-serializable property present (settings)", () => {
+                ruleTester.run("foo", {
+                    meta: {},
+                    create(context) {
+                        return {
+                            VariableDeclaration(node) {
+                                context.report(node, "foo bar");
+                            }
+                        };
+                    }
+                }, {
+                    valid: ["foo"],
+                    invalid: [
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], settings: { foo: /abc/u } },
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], settings: { foo: /abc/u } }
+                    ]
+                });
+            });
+
+            it("ignores duplicate test case when non-serializable property present (languageOptions.parserOptions)", () => {
+                ruleTester.run("foo", {
+                    meta: {},
+                    create(context) {
+                        return {
+                            VariableDeclaration(node) {
+                                context.report(node, "foo bar");
+                            }
+                        };
+                    }
+                }, {
+                    valid: ["foo"],
+                    invalid: [
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], languageOptions: { parserOptions: { foo: /abc/u } } },
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], languageOptions: { parserOptions: { foo: /abc/u } } }
+                    ]
+                });
+            });
+
+            it("ignores duplicate test case when non-serializable property present (plugins)", () => {
+                ruleTester.run("foo", {
+                    meta: {},
+                    create(context) {
+                        return {
+                            VariableDeclaration(node) {
+                                context.report(node, "foo bar");
+                            }
+                        };
+                    }
+                }, {
+                    valid: ["foo"],
+                    invalid: [
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], plugins: { foo: /abc/u } },
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], plugins: { foo: /abc/u } }
+                    ]
+                });
+            });
+
+            it("ignores duplicate test case when non-serializable property present (options)", () => {
+                ruleTester.run("foo", {
+                    meta: { schema: false },
+                    create(context) {
+                        return {
+                            VariableDeclaration(node) {
+                                context.report(node, "foo bar");
+                            }
+                        };
+                    }
+                }, {
+                    valid: ["foo"],
+                    invalid: [
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], options: [{ foo: /abc/u }] },
+                        { code: "const x = 123;", errors: [{ message: "foo bar" }], options: [{ foo: /abc/u }] }
+                    ]
+                });
+            });
+        });
+    });
+
     describe("SourceCode forbidden methods", () => {
 
         [

tests/lib/rules/indent.js

@@ -4624,14 +4624,6 @@ ruleTester.run("indent", rule, {
             `,
             options: [0]
         },
-        {
-            code: unIndent`
-              <App
-              \tfoo
-              />
-            `,
-            options: ["tab"]
-        },
         unIndent`
                 <App
                     foo

tests/lib/rules/multiline-comment-style.js

@@ -1262,20 +1262,6 @@ ${"                   "}
             options: ["starred-block"],
             errors: [{ messageId: "expectedBlock", line: 2 }]
         },
-        {
-            code: `
-                // foo
-                //
-                // bar
-            `,
-            output: `
-                /* foo
-${"                   "}
-                   bar */
-            `,
-            options: ["bare-block"],
-            errors: [{ messageId: "expectedBlock", line: 2 }]
-        },
         {
             code: `
                 // foo

tests/lib/rules/newline-after-var.js

@@ -24,8 +24,8 @@ const NO_VAR = "console.log(greet);",
     FOR_LOOP_WITH_VAR = "for(var a = 1; a < 1; a++){\n break;\n}",
     FOR_IN_LOOP_WITH_LET = "for(let a in obj){\n break;\n}",
     FOR_IN_LOOP_WITH_VAR = "for(var a in obj){\n break;\n}",
-    FOR_OF_LOOP_WITH_LET = "for(let a in obj){\n break;\n}",
-    FOR_OF_LOOP_WITH_VAR = "for(var a in obj){\n break;\n}",
+    FOR_OF_LOOP_WITH_LET = "for(let a of obj){\n break;\n}",
+    FOR_OF_LOOP_WITH_VAR = "for(var a of obj){\n break;\n}",
     EXPORT_WITH_LET = "export let a = 1;\nexport let b = 2;",
     EXPORT_WITH_VAR = "export var a = 1;\nexport var b = 2;",
     EXPORT_WITH_CONST = "export const a = 1;\nexport const b = 2;",