Breaking: Require meta.hasSuggestions for rules with suggestions (#14573)

bmish · mdjermanovic · web-flow · commit d6a761f9b658 · 2021-08-05T12:59:59.000-07:00
* Breaking: Require `meta.hasSuggestions` for rules with suggestions

* Docs: add another note about hasSuggestions

* Update docs/developer-guide/working-with-rules.md

Co-authored-by: Milos Djermanovic &lt;milos.djermanovic@gmail.com&gt;

* Update docs/developer-guide/working-with-rules.md

Co-authored-by: Milos Djermanovic &lt;milos.djermanovic@gmail.com&gt;

* enable eslint-plugin/require-meta-has-suggestions rule internally

* update property in Makefile

Co-authored-by: Milos Djermanovic &lt;milos.djermanovic@gmail.com&gt;
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -73,6 +73,7 @@ module.exports = {
         "eslint-plugin/prefer-replace-text": "error",
         "eslint-plugin/report-message-format": ["error", "[^a-z].*\\.$"],
         "eslint-plugin/require-meta-docs-description": "error",
+        "eslint-plugin/require-meta-has-suggestions": "error",
         "eslint-plugin/require-meta-schema": "error",
         "eslint-plugin/require-meta-type": "error",
         "eslint-plugin/test-case-property-ordering": "error",
diff --git a/Makefile.js b/Makefile.js
@@ -198,7 +198,7 @@ function generateRuleIndexPage() {
                         description: rule.meta.docs.description,
                         recommended: rule.meta.docs.recommended || false,
                         fixable: !!rule.meta.fixable,
-                        hasSuggestions: !!rule.meta.docs.suggestion
+                        hasSuggestions: !!rule.meta.hasSuggestions
                     },
                     category = categoriesData.categories.find(c => c.name === rule.meta.docs.category);
 
@@ -671,7 +671,7 @@ target.gensite = function(prereleaseVersion) {
                 const rule = rules.get(ruleName);
                 const isRecommended = rule && rule.meta.docs.recommended;
                 const isFixable = rule && rule.meta.fixable;
-                const hasSuggestions = rule && rule.meta.docs.suggestion;
+                const hasSuggestions = rule && rule.meta.hasSuggestions;
 
                 // Incorporate the special portion into the documentation content
                 const textSplit = text.split("\n");
diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md
@@ -62,14 +62,17 @@ The source file for a rule exports an object with the following properties.
     * `category` (string) specifies the heading under which the rule is listed in the [rules index](../rules/)
     * `recommended` (boolean) is whether the `"extends": "eslint:recommended"` property in a [configuration file](../user-guide/configuring/configuration-files.md#extending-configuration-files) enables the rule
     * `url` (string) specifies the URL at which the full documentation can be accessed (enabling code editors to provide a helpful link on highlighted rule violations)
-    * `suggestion` (boolean) specifies whether rules can return suggestions (defaults to false if omitted)
 
     In a custom rule or plugin, you can omit `docs` or include any properties that you need in it.
 
 * `fixable` (string) is either `"code"` or `"whitespace"` if the `--fix` option on the [command line](../user-guide/command-line-interface.md#--fix) automatically fixes problems reported by the rule
 
     **Important:** the `fixable` property is mandatory for fixable rules. If this property isn't specified, ESLint will throw an error whenever the rule attempts to produce a fix. Omit the `fixable` property if the rule is not fixable.
 
+* `hasSuggestions` (boolean) specifies whether rules can return suggestions (defaults to `false` if omitted)
+
+     **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` (array) specifies the [options](#options-schemas) so ESLint can prevent invalid [rule configurations](../user-guide/configuring/rules.md#configuring-rules)
 
 * `deprecated` (boolean) indicates whether the rule has been deprecated.  You may omit the `deprecated` property if the rule has not been deprecated.
@@ -376,6 +379,8 @@ context.report({
 {% endraw %}
 ```
 
+**Important:** The `meta.hasSuggestions` property is mandatory for rules that provide suggestions. ESLint will throw an error if a rule attempts to produce a suggestion but does not [export](#rule-basics) this property.
+
 Note: Suggestions will be applied as a stand-alone change, without triggering multipass fixes. Each suggestion should focus on a singular change in the code and should not try to conform to user defined styles. For example, if a suggestion is adding a new statement into the codebase, it should not try to match correct indentation, or conform to user preferences on presence/absence of semicolons. All of those things can be corrected by multipass autofix when the user triggers it.
 
 Best practices for suggestions:
@@ -397,7 +402,8 @@ module.exports = {
             unnecessaryEscape: "Unnecessary escape character: \\{{character}}.",
             removeEscape: "Remove the `\\`. This maintains the current functionality.",
             escapeBackslash: "Replace the `\\` with `\\\\` to include the actual backslash character."
-        }
+        },
+        hasSuggestions: true
     },
     create: function(context) {
         // ...
@@ -438,7 +444,8 @@ module.exports = {
         messages: {
             unnecessaryEscape: "Unnecessary escape character: \\{{character}}.",
             removeEscape: "Remove `\\` before {{character}}.",
-        }
+        },
+        hasSuggestions: true
     },
     create: function(context) {
         // ...
diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -928,6 +928,14 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser
                         if (problem.fix && rule.meta && !rule.meta.fixable) {
                             throw new Error("Fixable rules should export a `meta.fixable` property.");
                         }
+                        if (problem.suggestions && !(rule.meta && rule.meta.hasSuggestions === true)) {
+                            if (rule.meta && rule.meta.docs && typeof rule.meta.docs.suggestion !== "undefined") {
+
+                                // Encourage migration from the former property name.
+                                throw new Error("Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.");
+                            }
+                            throw new Error("Rules with suggestions must set the `meta.hasSuggestions` property to `true`.");
+                        }
                         lintingProblems.push(problem);
                     }
                 }
diff --git a/lib/rules/no-nonoctal-decimal-escape.js b/lib/rules/no-nonoctal-decimal-escape.js
@@ -32,10 +32,11 @@ module.exports = {
             description: "disallow `\\8` and `\\9` escape sequences in string literals",
             category: "Best Practices",
             recommended: true,
-            url: "https://eslint.org/docs/rules/no-nonoctal-decimal-escape",
-            suggestion: true
+            url: "https://eslint.org/docs/rules/no-nonoctal-decimal-escape"
         },
 
+        hasSuggestions: true,
+
         schema: [],
 
         messages: {
diff --git a/lib/rules/no-unsafe-negation.js b/lib/rules/no-unsafe-negation.js
@@ -54,10 +54,11 @@ module.exports = {
             description: "disallow negating the left operand of relational operators",
             category: "Possible Errors",
             recommended: true,
-            url: "https://eslint.org/docs/rules/no-unsafe-negation",
-            suggestion: true
+            url: "https://eslint.org/docs/rules/no-unsafe-negation"
         },
 
+        hasSuggestions: true,
+
         schema: [
             {
                 type: "object",
diff --git a/lib/rules/no-useless-escape.js b/lib/rules/no-useless-escape.js
@@ -85,10 +85,11 @@ module.exports = {
             description: "disallow unnecessary escape characters",
             category: "Best Practices",
             recommended: true,
-            url: "https://eslint.org/docs/rules/no-useless-escape",
-            suggestion: true
+            url: "https://eslint.org/docs/rules/no-useless-escape"
         },
 
+        hasSuggestions: true,
+
         messages: {
             unnecessaryEscape: "Unnecessary escape character: \\{{character}}.",
             removeEscape: "Remove the `\\`. This maintains the current functionality.",
diff --git a/lib/rules/radix.js b/lib/rules/radix.js
@@ -82,10 +82,11 @@ module.exports = {
             description: "enforce the consistent use of the radix argument when using `parseInt()`",
             category: "Best Practices",
             recommended: false,
-            url: "https://eslint.org/docs/rules/radix",
-            suggestion: true
+            url: "https://eslint.org/docs/rules/radix"
         },
 
+        hasSuggestions: true,
+
         schema: [
             {
                 enum: ["always", "as-needed"]
diff --git a/tests/fixtures/testers/rule-tester/suggestions.js b/tests/fixtures/testers/rule-tester/suggestions.js
@@ -1,6 +1,7 @@
 "use strict";
 
 module.exports.basic = {
+    meta: { hasSuggestions: true },
     create(context) {
         return {
             Identifier(node) {
@@ -25,7 +26,8 @@ module.exports.withMessageIds = {
             avoidFoo: "Avoid using identifiers named '{{ name }}'.",
             unused: "An unused key",
             renameFoo: "Rename identifier 'foo' to '{{ newName }}'"
-        }
+        },
+        hasSuggestions: true
     },
     create(context) {
         return {
@@ -57,4 +59,16 @@ module.exports.withMessageIds = {
     }
 };
 
-
+module.exports.withoutHasSuggestionsProperty = {
+    create(context) {
+        return {
+            Identifier(node) {
+                context.report({
+                    node,
+                    message: "some message",
+                    suggest: [{ desc: "some suggestion", fix: fixer => fixer.replaceText(node, 'bar') }]
+                });
+            }
+        };
+    }
+};
diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js
@@ -4801,21 +4801,24 @@ var a = "test2";
 
     describe("suggestions", () => {
         it("provides suggestion information for tools to use", () => {
-            linter.defineRule("rule-with-suggestions", context => ({
-                Program(node) {
-                    context.report({
-                        node,
-                        message: "Incorrect spacing",
-                        suggest: [{
-                            desc: "Insert space at the beginning",
-                            fix: fixer => fixer.insertTextBefore(node, " ")
-                        }, {
-                            desc: "Insert space at the end",
-                            fix: fixer => fixer.insertTextAfter(node, " ")
-                        }]
-                    });
-                }
-            }));
+            linter.defineRule("rule-with-suggestions", {
+                meta: { hasSuggestions: true },
+                create: context => ({
+                    Program(node) {
+                        context.report({
+                            node,
+                            message: "Incorrect spacing",
+                            suggest: [{
+                                desc: "Insert space at the beginning",
+                                fix: fixer => fixer.insertTextBefore(node, " ")
+                            }, {
+                                desc: "Insert space at the end",
+                                fix: fixer => fixer.insertTextAfter(node, " ")
+                            }]
+                        });
+                    }
+                })
+            });
 
             const messages = linter.verify("var a = 1;", { rules: { "rule-with-suggestions": "error" } });
 
@@ -4840,7 +4843,8 @@ var a = "test2";
                     messages: {
                         suggestion1: "Insert space at the beginning",
                         suggestion2: "Insert space at the end"
-                    }
+                    },
+                    hasSuggestions: true
                 },
                 create: context => ({
                     Program(node) {
@@ -4877,6 +4881,44 @@ var a = "test2";
                 }
             }]);
         });
+
+        it("should throw an error if suggestion is passed but `meta.hasSuggestions` property is not enabled", () => {
+            linter.defineRule("rule-with-suggestions", {
+                meta: { docs: {}, schema: [] },
+                create: context => ({
+                    Program(node) {
+                        context.report({
+                            node,
+                            message: "hello world",
+                            suggest: [{ desc: "convert to foo", fix: fixer => fixer.insertTextBefore(node, " ") }]
+                        });
+                    }
+                })
+            });
+
+            assert.throws(() => {
+                linter.verify("0", { rules: { "rule-with-suggestions": "error" } });
+            }, "Rules with suggestions must set the `meta.hasSuggestions` property to `true`.");
+        });
+
+        it("should throw an error if suggestion is passed but `meta.hasSuggestions` property is not enabled and the rule has the obsolete `meta.docs.suggestion` property", () => {
+            linter.defineRule("rule-with-meta-docs-suggestion", {
+                meta: { docs: { suggestion: true }, schema: [] },
+                create: context => ({
+                    Program(node) {
+                        context.report({
+                            node,
+                            message: "hello world",
+                            suggest: [{ desc: "convert to foo", fix: fixer => fixer.insertTextBefore(node, " ") }]
+                        });
+                    }
+                })
+            });
+
+            assert.throws(() => {
+                linter.verify("0", { rules: { "rule-with-meta-docs-suggestion": "error" } });
+            }, "Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.");
+        });
     });
 
     describe("mutability", () => {
diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js
@@ -2279,6 +2279,17 @@ describe("RuleTester", () => {
                 });
             }, /Invalid suggestion property name 'outpt'/u);
         });
+
+        it("should throw an error if a rule that doesn't have `meta.hasSuggestions` enabled produces suggestions", () => {
+            assert.throws(() => {
+                ruleTester.run("suggestions-missing-hasSuggestions-property", require("../../fixtures/testers/rule-tester/suggestions").withoutHasSuggestionsProperty, {
+                    valid: [],
+                    invalid: [
+                        { code: "var foo = bar;", output: "5", errors: 1 }
+                    ]
+                });
+            }, "Rules with suggestions must set the `meta.hasSuggestions` property to `true`.");
+        });
     });
 
     describe("naming test cases", () => {

Original file line number	Diff line number	Diff line change
`@@ -928,6 +928,14 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser`
`928`	`928`	`if (problem.fix && rule.meta && !rule.meta.fixable) {`
`929`	`929`	throw new Error("Fixable rules should export a `meta.fixable` property.");
`930`	`930`	`}`
	`931`	`+ if (problem.suggestions && !(rule.meta && rule.meta.hasSuggestions === true)) {`
	`932`	`+ if (rule.meta && rule.meta.docs && typeof rule.meta.docs.suggestion !== "undefined") {`
	`933`	`+`
	`934`	`+ // Encourage migration from the former property name.`
	`935`	+ throw new Error("Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.");
	`936`	`+ }`
	`937`	+ throw new Error("Rules with suggestions must set the `meta.hasSuggestions` property to `true`.");
	`938`	`+ }`
`931`	`939`	`lintingProblems.push(problem);`
`932`	`940`	`}`
`933`	`941`	`}`