feat!: Stricter rule test validations

docs/src/integrate/nodejs-api.md

@@ -735,19 +735,19 @@ A test case is an object with the following properties:
 
 In addition to the properties above, invalid test cases can also have the following properties:
 
-* `errors` (number or array, required): Asserts some properties of the errors that the rule is expected to produce when run on this code. If this is a number, asserts the number of errors produced. Otherwise, this should be a list of objects, each containing information about a single reported error. The following properties can be used for an error (all are optional):
-    * `message` (string/regexp): The message for the error
-    * `messageId` (string): The Id for the error. See [testing errors with messageId](#testing-errors-with-messageid) for details
-    * `data` (object): Placeholder data which can be used in combination with `messageId`
+* `errors` (number or array, required): Asserts some properties of the errors that the rule is expected to produce when run on this code. If this is a number, asserts the number of errors produced. Otherwise, this should be a list of objects, each containing information about a single reported error. The following properties can be used for an error (all are optional unless otherwise noted):
+    * `message` (string/regexp): The message for the error. Must provide this or `messageId`
+    * `messageId` (string): The Id for the error. Must provide this or `message`. See [testing errors with messageId](#testing-errors-with-messageid) for details
+    * `data` (object): Placeholder data which can be used in combination with `messageId`. Required if `messageId` is specified and its message uses placeholders
     * `type` (string): The type of the reported AST node
     * `line` (number): The 1-based line number of the reported location
     * `column` (number): The 1-based column number of the reported location
     * `endLine` (number): The 1-based line number of the end of the reported location
     * `endColumn` (number): The 1-based column number of the end of the reported location
-    * `suggestions` (array): An array of objects with suggestion details to check. See [Testing Suggestions](#testing-suggestions) for details
+    * `suggestions` (array): An array of objects with suggestion details to check. Required if the rule produces suggestions. See [Testing Suggestions](#testing-suggestions) for details
 
     If a string is provided as an error instead of an object, the string is used to assert the `message` of the error.
-* `output` (string, required if the rule fixes code): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes.
+* `output` (string, required if the rule fixes code): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes or the fixer did not create a fix.
 
 Any additional properties of a test case will be passed directly to the linter as config options. For example, a test case can have a `languageOptions` property to configure parser behavior:
 
@@ -784,12 +784,12 @@ Please note that `data` in a test case does not assert `data` passed to `context
 
 ### Testing Suggestions
 
-Suggestions can be tested by defining a `suggestions` key on an errors object. The options to check for the suggestions are the following (all are optional):
+Suggestions can be tested by defining a `suggestions` key on an errors object. The options to check for the suggestions are the following:
 
-* `desc` (string): The suggestion `desc` value
-* `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s
-* `data` (object): Placeholder data which can be used in combination with `messageId`
-* `output` (string): A code string representing the result of applying the suggestion fix to the input code
+* `desc` (string): The suggestion `desc` value. Must provide this or `messageId`
+* `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s. Must provide this or `desc`
+* `data` (object): Placeholder data which can be used in combination with `messageId`. Required if `messageId` is specified and its message uses placeholders
+* `output` (string, required): A code string representing the result of applying the suggestion fix to the input code
 
 Example:
 

lib/rule-tester/rule-tester.js

@@ -631,7 +631,11 @@ class RuleTester {
                 configs.push(itemConfig);
             }
 
-            if (item.filename) {
+            if (hasOwnProperty(item, "only")) {
+                assert.ok(typeof item.only === "boolean", "Optional test case property 'only' must be a boolean");
+            }
+            if (hasOwnProperty(item, "filename")) {
+                assert.ok(typeof item.filename === "string", "Optional test case property 'filename' must be a string");
                 filename = item.filename;
             }
 
@@ -964,13 +968,10 @@ class RuleTester {
                                     `Hydrated message "${rehydratedMessage}" does not match "${message.message}"`
                                 );
                             }
+                        } else {
+                            assert.fail("Error must have either a 'messageId' or 'message'.");
                         }
 
-                        assert.ok(
-                            hasOwnProperty(error, "data") ? hasOwnProperty(error, "messageId") : true,
-                            "Error must specify 'messageId' if 'data' is used."
-                        );
-
                         if (error.type) {
                             assert.strictEqual(message.nodeType, error.type, `Error type should be ${error.type}, found ${message.nodeType}`);
                         }
@@ -991,6 +992,7 @@ class RuleTester {
                             assert.strictEqual(message.endColumn, error.endColumn, `Error endColumn should be ${error.endColumn}`);
                         }
 
+                        assert.ok(!message.suggestions || hasOwnProperty(error, "suggestions"), `Error should have suggestions on error with message: "${message.message}`);
                         if (hasOwnProperty(error, "suggestions")) {
 
                             // Support asserting there are no suggestions
@@ -1015,21 +1017,23 @@ class RuleTester {
                                     });
 
                                     const actualSuggestion = message.suggestions[index];
-                                    const suggestionPrefix = `Error Suggestion at index ${index} :`;
+                                    const suggestionPrefix = `Error Suggestion at index ${index}:`;
 
                                     if (hasOwnProperty(expectedSuggestion, "desc")) {
                                         assert.ok(
                                             !hasOwnProperty(expectedSuggestion, "data"),
                                             `${suggestionPrefix} Test should not specify both 'desc' and 'data'.`
                                         );
+                                        assert.ok(
+                                            !hasOwnProperty(expectedSuggestion, "messageId"),
+                                            `${suggestionPrefix} Test should not specify both 'desc' and 'messageId'.`
+                                        );
                                         assert.strictEqual(
                                             actualSuggestion.desc,
                                             expectedSuggestion.desc,
                                             `${suggestionPrefix} desc should be "${expectedSuggestion.desc}" but got "${actualSuggestion.desc}" instead.`
                                         );
-                                    }
-
-                                    if (hasOwnProperty(expectedSuggestion, "messageId")) {
+                                    } else if (hasOwnProperty(expectedSuggestion, "messageId")) {
                                         assert.ok(
                                             ruleHasMetaMessages,
                                             `${suggestionPrefix} Test can not use 'messageId' if rule under test doesn't define 'meta.messages'.`
@@ -1053,29 +1057,32 @@ class RuleTester {
                                                 `${suggestionPrefix} Hydrated test desc "${rehydratedDesc}" does not match received desc "${actualSuggestion.desc}".`
                                             );
                                         }
-                                    } else {
-                                        assert.ok(
-                                            !hasOwnProperty(expectedSuggestion, "data"),
+                                    } else if (hasOwnProperty(expectedSuggestion, "data")) {
+                                        assert.fail(
                                             `${suggestionPrefix} Test must specify 'messageId' if 'data' is used.`
                                         );
+                                    } else {
+                                        assert.fail(
+                                            `${suggestionPrefix} Test must specify either 'messageId' or 'desc'.`
+                                        );
                                     }
 
-                                    if (hasOwnProperty(expectedSuggestion, "output")) {
-                                        const codeWithAppliedSuggestion = SourceCodeFixer.applyFixes(item.code, [actualSuggestion]).output;
+                                    assert.ok(hasOwnProperty(expectedSuggestion, "output"), `${suggestionPrefix} The "output" property is required.`);
+                                    const codeWithAppliedSuggestion = SourceCodeFixer.applyFixes(item.code, [actualSuggestion]).output;
 
-                                        // Verify if suggestion fix makes a syntax error or not.
-                                        const errorMessageInSuggestion =
+                                    // Verify if suggestion fix makes a syntax error or not.
+                                    const errorMessageInSuggestion =
                                             linter.verify(codeWithAppliedSuggestion, result.configs, result.filename).find(m => m.fatal);
 
-                                        assert(!errorMessageInSuggestion, [
-                                            "A fatal parsing error occurred in suggestion fix.",
-                                            `Error: ${errorMessageInSuggestion && errorMessageInSuggestion.message}`,
-                                            "Suggestion output:",
-                                            codeWithAppliedSuggestion
-                                        ].join("\n"));
+                                    assert(!errorMessageInSuggestion, [
+                                        "A fatal parsing error occurred in suggestion fix.",
+                                        `Error: ${errorMessageInSuggestion && errorMessageInSuggestion.message}`,
+                                        "Suggestion output:",
+                                        codeWithAppliedSuggestion
+                                    ].join("\n"));
 
-                                        assert.strictEqual(codeWithAppliedSuggestion, expectedSuggestion.output, `Expected the applied suggestion fix to match the test suggestion output for suggestion at index: ${index} on error with message: "${message.message}"`);
-                                    }
+                                    assert.strictEqual(codeWithAppliedSuggestion, expectedSuggestion.output, `Expected the applied suggestion fix to match the test suggestion output for suggestion at index: ${index} on error with message: "${message.message}"`);
+                                    assert.notStrictEqual(expectedSuggestion.output, item.code, `The output of a suggestion should differ from the original source code for suggestion at index: ${index} on error with message: "${message.message}"`);
                                 });
                             }
                         }
@@ -1096,6 +1103,7 @@ class RuleTester {
                     );
                 } else {
                     assert.strictEqual(result.output, item.output, "Output is incorrect.");
+                    assert.notStrictEqual(item.code, item.output, "Use 'output: null' if the rule does not fix this case instead of copying the code.");
                 }
             } else {
                 assert.strictEqual(

tests/fixtures/testers/rule-tester/suggestions.js

@@ -164,3 +164,23 @@ module.exports.withoutHasSuggestionsProperty = {
         };
     }
 };
+
+module.exports.withFixerWithoutChanges = {
+    meta: { hasSuggestions: true },
+    create(context) {
+        return {
+            Identifier(node) {
+                if (node.name === "foo") {
+                    context.report({
+                        node,
+                        message: "Avoid using identifiers named 'foo'.",
+                        suggest: [{
+                            desc: "Rename identifier 'foo' to 'bar'",
+                            fix: fixer => fixer.replaceText(node, 'foo')
+                        }]
+                    });
+                }
+            }
+        };
+    }
+};