From 54cf3d393daa217d2c7d7f98a6aa77e08511770b Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Wed, 7 Apr 2021 12:59:46 -0700 Subject: [PATCH] feat: add new rule require-meta-has-suggestions Suggestable ESLint rules should have a `meta.hasSuggestions` property to indicate that they provide [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). This new rule enforces that the `meta.hasSuggestions` property is correctly enabled when a rule provides suggestions, and not enabled when a rule does not provide suggestions. The change to require suggestable rules to have `meta.hasSuggestions` has been accepted and mentioned in the blog post for the upcoming ESLint 8 breaking changes. So we should adopt this change now to help plugin authors ensure they are compatible with ESLint 8 as soon as possible. The old property `meta.docs.suggestion` was unused anyway. https://eslint.org/blog/2021/06/whats-coming-in-eslint-8.0.0#rules-with-suggestions-now-require-the-metahassuggestions-property This is very similar to the existing [eslint-plugin/require-meta-fixable](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-fixable.md) rule enforcing the correct presence of the `meta.fixable` property. --- README.md | 1 + docs/rules/require-meta-has-suggestions.md | 94 +++++++++ lib/rules/require-meta-has-suggestions.js | 79 +++++++ .../lib/rules/require-meta-has-suggestions.js | 194 ++++++++++++++++++ 4 files changed, 368 insertions(+) create mode 100644 docs/rules/require-meta-has-suggestions.md create mode 100644 lib/rules/require-meta-has-suggestions.js create mode 100644 tests/lib/rules/require-meta-has-suggestions.js diff --git a/README.md b/README.md index 34bf6976..06c08950 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,7 @@ Name | ✔️ | 🛠 | Description [require-meta-docs-description](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-docs-description.md) | | | require rules to implement a meta.docs.description property with the correct format [require-meta-docs-url](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-docs-url.md) | | 🛠 | require rules to implement a meta.docs.url property [require-meta-fixable](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-fixable.md) | ✔️ | | require rules to implement a meta.fixable property +[require-meta-has-suggestions](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-has-suggestions.md) | | | require suggestable rules to implement a `meta.hasSuggestions` property [require-meta-schema](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-schema.md) | | 🛠 | require rules to implement a meta.schema property [require-meta-type](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-type.md) | | | require rules to implement a meta.type property [test-case-property-ordering](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/test-case-property-ordering.md) | | 🛠 | require the properties of a test case to be placed in a consistent order diff --git a/docs/rules/require-meta-has-suggestions.md b/docs/rules/require-meta-has-suggestions.md new file mode 100644 index 00000000..c9287e97 --- /dev/null +++ b/docs/rules/require-meta-has-suggestions.md @@ -0,0 +1,94 @@ +# require suggestable rules to implement a `meta.hasSuggestions` property (require-meta-has-suggestions) + +A suggestable ESLint rule should specify the `meta.hasSuggestions` property with a value of `true`. This makes it easier for both humans and tooling to tell whether a rule provides suggestions. [As of ESLint 8](https://eslint.org/blog/2021/06/whats-coming-in-eslint-8.0.0#rules-with-suggestions-now-require-the-metahassuggestions-property), an exception will be thrown if a suggestable rule is missing this property. + +Likewise, rules that do not report suggestions should not enable the `meta.hasSuggestions` property. + +## Rule Details + +This rule aims to require ESLint rules to have a `meta.hasSuggestions` property if necessary. + +The following patterns are considered warnings: + +```js + +/* eslint eslint-plugin/require-meta-has-suggestions: "error" */ + +module.exports = { + meta: {}, // Missing `meta.hasSuggestions`. + create(context) { + context.report({ + node, + message: 'foo', + suggest: [ + { + desc: 'Insert space at the beginning', + fix: fixer => fixer.insertTextBefore(node, " ") + } + ] + }); + } +}; + +``` + +```js + +/* eslint eslint-plugin/require-meta-has-suggestions: "error" */ + +module.exports = { + meta: { hasSuggestions: true }, // Has `meta.hasSuggestions` enabled but never provides suggestions. + create(context) { + context.report({ + node, + message: 'foo' + }); + } +}; + +``` + +The following patterns are not warnings: + +```js + +/* eslint eslint-plugin/require-meta-has-suggestions: "error" */ + +module.exports = { + meta: { hasSuggestions: true }, + create(context) { + context.report({ + node, + message: 'foo', + suggest: [ + { + desc: 'Insert space at the beginning', + fix: fixer => fixer.insertTextBefore(node, " ") + } + ] + }); + } +}; + +``` + +```js + +/* eslint eslint-plugin/require-meta-has-suggestions: "error" */ + +module.exports = { + meta: {}, + create(context) { + context.report({ + node, + message: 'foo' + }); + } +}; + +``` + +## Further Reading + +* [ESLint's suggestion API](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions) +* [ESLint rule basics describing the `meta.hasSuggestions` property](https://eslint.org/docs/developer-guide/working-with-rules#rule-basics) diff --git a/lib/rules/require-meta-has-suggestions.js b/lib/rules/require-meta-has-suggestions.js new file mode 100644 index 00000000..57e7a3f5 --- /dev/null +++ b/lib/rules/require-meta-has-suggestions.js @@ -0,0 +1,79 @@ +'use strict'; + +const utils = require('../utils'); +const { getStaticValue } = require('eslint-utils'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'require suggestable rules to implement a `meta.hasSuggestions` property', + category: 'Rules', + recommended: false, + }, + type: 'problem', + messages: { + shouldBeSuggestable: 'Suggestable rules should specify a `meta.hasSuggestions` property with value `true`.', + shouldNotBeSuggestable: 'Non-suggestable rules should not specify a `meta.hasSuggestions` property with value `true`.', + }, + schema: [], + }, + + create (context) { + const sourceCode = context.getSourceCode(); + const ruleInfo = utils.getRuleInfo(sourceCode); + let contextIdentifiers; + let ruleReportsSuggestions; + + return { + Program (node) { + contextIdentifiers = utils.getContextIdentifiers(context, node); + }, + CallExpression (node) { + if ( + node.callee.type === 'MemberExpression' && + contextIdentifiers.has(node.callee.object) && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'report' && + (node.arguments.length > 4 || ( + node.arguments.length === 1 && + node.arguments[0].type === 'ObjectExpression' + )) + ) { + const suggestProp = node.arguments[0].properties.find(prop => utils.getKeyName(prop) === 'suggest'); + if (suggestProp) { + const staticValue = getStaticValue(suggestProp.value, context.getScope()); + if (!staticValue || (Array.isArray(staticValue.value) && staticValue.value.length > 0)) { + // These are all considered reporting suggestions: + // suggest: [{...}] + // suggest: getSuggestions() + // suggest: MY_SUGGESTIONS + ruleReportsSuggestions = true; + } + } + } + }, + 'Program:exit' () { + const metaNode = ruleInfo && ruleInfo.meta; + const hasSuggestionsProperty = metaNode && metaNode.type === 'ObjectExpression' ? metaNode.properties.find(prop => utils.getKeyName(prop) === 'hasSuggestions') : undefined; + const hasSuggestionsStaticValue = hasSuggestionsProperty && getStaticValue(hasSuggestionsProperty.value, context.getScope()); + + if (ruleReportsSuggestions) { + if (!hasSuggestionsProperty) { + // Rule reports suggestions but is missing the `meta.hasSuggestions` property altogether. + context.report({ node: metaNode ? metaNode : ruleInfo.create, messageId: 'shouldBeSuggestable' }); + } else if (hasSuggestionsStaticValue.value !== true) { + // Rule reports suggestions but does not have `meta.hasSuggestions` property enabled. + context.report({ node: hasSuggestionsProperty.value, messageId: 'shouldBeSuggestable' }); + } + } else if (!ruleReportsSuggestions && hasSuggestionsProperty && hasSuggestionsStaticValue.value === true) { + // Rule does not report suggestions but has the `meta.hasSuggestions` property enabled. + context.report({ node: hasSuggestionsProperty.value, messageId: 'shouldNotBeSuggestable' }); + } + }, + }; + }, +}; diff --git a/tests/lib/rules/require-meta-has-suggestions.js b/tests/lib/rules/require-meta-has-suggestions.js new file mode 100644 index 00000000..f58f9d24 --- /dev/null +++ b/tests/lib/rules/require-meta-has-suggestions.js @@ -0,0 +1,194 @@ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/require-meta-has-suggestions'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } }); +ruleTester.run('require-meta-has-suggestions', rule, { + valid: [ + 'module.exports = context => {};', + // No suggestions reported, no violations reported, no meta object. + ` + module.exports = { + create(context) {} + }; + `, + // No suggestions reported, no violations reported, empty meta object. + ` + module.exports = { + meta: {}, + create(context) {} + }; + `, + // No suggestions reported, violation reported, empty meta object. + ` + module.exports = { + meta: {}, + create(context) { + context.report({node, message}); + } + }; + `, + // No suggestions reported, no suggestion property, non-object style of reporting. + ` + module.exports = { + meta: {}, + create(context) { + context.report(node, message); + } + }; + `, + // No suggestions reported (empty suggest array), no suggestion property. + ` + module.exports = { + meta: {}, + create(context) { + context.report({node, message, suggest:[]}); + } + }; + `, + // No suggestions reported (empty suggest array in variable), no suggestion property. + ` + const SUGGESTIONS = []; + module.exports = { + meta: {}, + create(context) { + context.report({node, message, suggest: SUGGESTIONS}); + } + }; + `, + // No suggestions reported, hasSuggestions property set to false. + ` + module.exports = { + meta: { hasSuggestions: false }, + create(context) { + context.report({node, message}); + } + }; + `, + // No suggestions reported, hasSuggestions property set to false (as variable). + ` + const hasSuggestions = false; + module.exports = { + meta: { hasSuggestions }, + create(context) { + context.report({node, message}); + } + }; + `, + // Provides suggestions, has hasSuggestions property. + ` + module.exports = { + meta: { hasSuggestions: true }, + create(context) { + context.report({node, message, suggest: [{}]}); + } + }; + `, + // Provides suggestions, has hasSuggestions property (as variable). + ` + const hasSuggestions = true; + module.exports = { + meta: { hasSuggestions }, + create(context) { + context.report({node, message, suggest: [{}]}); + } + }; + `, + // Provides *dynamic* suggestions, has hasSuggestions property. + ` + module.exports = { + meta: { hasSuggestions: true }, + create(context) { + context.report({node, message, suggest: getSuggestions()}); + } + }; + `, + // Spread syntax. + { + code: ` + const meta = {}; + module.exports = { + ...meta, + meta: {}, + create(context) { context.report(node, message, data, fix); } + }; + `, + parserOptions: { + ecmaVersion: 9, + }, + }, + ], + + invalid: [ + { + // Reports suggestions, no meta object, violation should be on `create` function. + code: ` + module.exports = { + create(context) { context.report({node, message, suggest: [{}]}); } + }; + `, + errors: [{ messageId: 'shouldBeSuggestable', type: 'FunctionExpression', line: 3, column: 17, endLine: 3, endColumn: 78 }], + }, + { + // Reports suggestions, no hasSuggestions property, violation should be on `meta` object. + code: ` + module.exports = { + meta: {}, + create(context) { context.report({node, message, suggest: [{}]}); } + }; + `, + errors: [{ messageId: 'shouldBeSuggestable', type: 'ObjectExpression', line: 3, column: 17, endLine: 3, endColumn: 19 }], + }, + { + // Reports suggestions (in variable), no hasSuggestions property, violation should be on `meta` object. + code: ` + const SUGGESTIONS = [{}]; + module.exports = { + meta: {}, + create(context) { context.report({node, message, suggest: SUGGESTIONS}); } + }; + `, + errors: [{ messageId: 'shouldBeSuggestable', type: 'ObjectExpression', line: 4, column: 17, endLine: 4, endColumn: 19 }], + }, + { + // Reports suggestions, hasSuggestions property set to false, violation should be on `false` + code: ` + module.exports = { + meta: { hasSuggestions: false }, + create(context) { context.report({node, message, suggest: [{}]}); } + }; + `, + errors: [{ messageId: 'shouldBeSuggestable', type: 'Literal', line: 3, column: 35, endLine: 3, endColumn: 40 }], + }, + { + // Reports suggestions, hasSuggestions property set to false (as variable), violation should be on variable + code: ` + const hasSuggestions = false; + module.exports = { + meta: { hasSuggestions }, + create(context) { context.report({node, message, suggest: [{}]}); } + }; + `, + errors: [{ messageId: 'shouldBeSuggestable', type: 'Identifier', line: 4, column: 19, endLine: 4, endColumn: 33 }], + }, + { + // Does not report suggestions, hasSuggestions property set to true, violation should be on `true` + code: ` + module.exports = { + meta: { hasSuggestions: true }, + create(context) { context.report({node, message}); } + }; + `, + errors: [{ messageId: 'shouldNotBeSuggestable', type: 'Literal', line: 3, column: 35, endLine: 3, endColumn: 39 }], + }, + ], +});