Skip to content

Commit

Permalink
Breaking: Require meta.hasSuggestions for rules with suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish committed May 7, 2021
1 parent ee3a3ea commit f7c476e
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 29 deletions.
11 changes: 8 additions & 3 deletions docs/developer-guide/working-with-rules.md
Expand Up @@ -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
* `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 specified, 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.
Expand Down Expand Up @@ -396,7 +399,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) {
// ...
Expand Down Expand Up @@ -437,7 +441,8 @@ module.exports = {
messages: {
unnecessaryEscape: "Unnecessary escape character: \\{{character}}.",
removeEscape: "Remove `\\` before {{character}}.",
}
},
hasSuggestions: true
},
create: function(context) {
// ...
Expand Down
6 changes: 6 additions & 0 deletions lib/linter/linter.js
Expand Up @@ -922,6 +922,12 @@ 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)) {
throw new Error("Rules with suggestions must set the `meta.hasSuggestions` property to `true`.");
}
if (rule.meta && rule.meta.docs && typeof rule.meta.docs.suggestion !== "undefined") {
throw new Error("Please replace the `meta.docs.suggestion` property with `meta.hasSuggestions`.");
}
lintingProblems.push(problem);
}
}
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/no-nonoctal-decimal-escape.js
Expand Up @@ -32,10 +32,11 @@ module.exports = {
description: "disallow `\\8` and `\\9` escape sequences in string literals",
category: "Best Practices",
recommended: false,
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: {
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/no-unsafe-negation.js
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/no-useless-escape.js
Expand Up @@ -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.",
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/radix.js
Expand Up @@ -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"]
Expand Down
18 changes: 16 additions & 2 deletions tests/fixtures/testers/rule-tester/suggestions.js
@@ -1,6 +1,7 @@
"use strict";

module.exports.basic = {
meta: { hasSuggestions: true },
create(context) {
return {
Identifier(node) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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') }]
});
}
};
}
};
73 changes: 57 additions & 16 deletions tests/lib/linter/linter.js
Expand Up @@ -4680,21 +4680,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" } });

Expand All @@ -4719,7 +4722,8 @@ var a = "test2";
messages: {
suggestion1: "Insert space at the beginning",
suggestion2: "Insert space at the end"
}
},
hasSuggestions: true
},
create: context => ({
Program(node) {
Expand Down Expand Up @@ -4756,6 +4760,43 @@ 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 a rule has the obsolete `meta.docs.suggestion` property (regardless of whether it provides suggestions)", () => {
linter.defineRule("rule-with-meta-docs-suggestion", {
meta: { docs: { suggestion: true }, schema: [] },
create: context => ({
Program(node) {
context.report({
node,
message: "hello world"
});
}
})
});

assert.throws(() => {
linter.verify("0", { rules: { "rule-with-meta-docs-suggestion": "error" } });
}, "Please replace the `meta.docs.suggestion` property with `meta.hasSuggestions`.");
});
});

describe("mutability", () => {
Expand Down
11 changes: 11 additions & 0 deletions tests/lib/rule-tester/rule-tester.js
Expand Up @@ -1813,6 +1813,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", () => {
Expand Down

0 comments on commit f7c476e

Please sign in to comment.