Skip to content
Permalink
Browse files
Breaking: Require meta.hasSuggestions for rules with suggestions (#…
…14573)

* 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 <milos.djermanovic@gmail.com>

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

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

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

* update property in Makefile

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
bmish and mdjermanovic committed Aug 5, 2021
1 parent 4c841b8 commit d6a761f9b6582e9f71705161be827ca303ef183f
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 31 deletions.
@@ -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",
@@ -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");
@@ -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) {
// ...
@@ -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);
}
}
@@ -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: {
@@ -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",
@@ -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.",
@@ -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"]
@@ -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') }]
});
}
};
}
};
@@ -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", () => {
@@ -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", () => {

0 comments on commit d6a761f

Please sign in to comment.