Skip to content

Commit

Permalink
feat: add require-meta-docs-recommended rule (#447)
Browse files Browse the repository at this point in the history
* feat: add require-meta-docs-recommended rule

* Apply suggestions from code review

Co-authored-by: Bryan Mishkin <[email protected]>

* Apply rename

* Fix lint complaints

* Added allowNonBoolean opt-in option

* Unit test for unknown value

---------

Co-authored-by: Bryan Mishkin <[email protected]>
  • Loading branch information
JoshuaKGoldberg and bmish authored Apr 25, 2024
1 parent 95b1980 commit 2370b46
Show file tree
Hide file tree
Showing 9 changed files with 397 additions and 44 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ module.exports = [
| [prefer-replace-text](docs/rules/prefer-replace-text.md) | require using `replaceText()` instead of `replaceTextRange()` | | | | |
| [report-message-format](docs/rules/report-message-format.md) | enforce a consistent format for rule report messages | | | | |
| [require-meta-docs-description](docs/rules/require-meta-docs-description.md) | require rules to implement a `meta.docs.description` property with the correct format | | | | |
| [require-meta-docs-recommended](docs/rules/require-meta-docs-recommended.md) | require rules to implement a `meta.docs.recommended` property | | | | |
| [require-meta-docs-url](docs/rules/require-meta-docs-url.md) | require rules to implement a `meta.docs.url` property | | 🔧 | | |
| [require-meta-fixable](docs/rules/require-meta-fixable.md) | require rules to implement a `meta.fixable` property || | | |
| [require-meta-has-suggestions](docs/rules/require-meta-has-suggestions.md) | require suggestable rules to implement a `meta.hasSuggestions` property || 🔧 | | |
Expand Down
75 changes: 75 additions & 0 deletions docs/rules/require-meta-docs-recommended.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Require rules to implement a `meta.docs.recommended` property (`eslint-plugin/require-meta-docs-recommended`)

<!-- end auto-generated rule header -->

Utilizing `meta.docs.recommended` makes it clear from each rule implementation whether a rule is part of the `recommended` config. Some plugins also have scripting for conveniently generating their config based on this flag.

However, this flag may not be appropriate for all plugins:

- Extra scripting/tooling is needed to keep the flags in sync with the config
- The flag may not scale to plugins that have multiple/many configs or don't have a recommended config
- Or some may simply prefer to keep the source of truth solely in the config rather than duplicating config membership data in the rules

By default, this rule enforces a `recommended` property be set to a `boolean` value.

## Rule Details

This rule requires ESLint rules to have a valid `meta.docs.recommended` property.

Examples of **incorrect** code for this rule:

```js
/* eslint eslint-plugin/require-meta-docs-recommended: error */

module.exports = {
meta: {},
create(context) {
/* ... */
},
};
```

Examples of **correct** code for this rule:

```js
/* eslint eslint-plugin/require-meta-docs-recommended: error */

module.exports = {
meta: { recommended: true },
create(context) {
/* ... */
},
};
```

## Options

<!-- begin auto-generated rule options list -->

| Name | Description | Type | Default |
| :---------------- | :--------------------------------------------------- | :------ | :------ |
| `allowNonBoolean` | Whether to allow values of types other than boolean. | Boolean | `false` |

<!-- end auto-generated rule options list -->

### `allowNonBoolean`

Some plugins require `meta.docs.recommended` values but allow value types other than `boolean`.
This option changes the rule to only enforce that the values exist.

Example of **correct** code for this rule with `allowNonBoolean`:

```js
/* eslint eslint-plugin/require-meta-docs-recommended: ["error", { "allowNonBoolean": true }] */

module.exports = {
meta: { recommended: 'strict' },
create(context) {
/* ... */
},
};
```

## Further Reading

- [Rule Structure](https://eslint.org/docs/latest/extend/custom-rules#rule-structure)
31 changes: 16 additions & 15 deletions lib/rules/require-meta-docs-description.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,22 +56,19 @@ module.exports = {
const scope = sourceCode.getScope?.(ast) || context.getScope(); // TODO: just use sourceCode.getScope() when we drop support for ESLint < v9.0.0
const { scopeManager } = sourceCode;

const pattern =
context.options[0] && context.options[0].pattern
? new RegExp(context.options[0].pattern)
: DEFAULT_PATTERN;
const {
docsNode,
metaNode,
metaPropertyNode: descriptionNode,
} = utils.getMetaDocsProperty('description', ruleInfo, scopeManager);

const metaNode = ruleInfo.meta;
const docsNode = utils
.evaluateObjectProperties(metaNode, scopeManager)
.find((p) => p.type === 'Property' && utils.getKeyName(p) === 'docs');

const descriptionNode = utils
.evaluateObjectProperties(docsNode && docsNode.value, scopeManager)
.find(
(p) =>
p.type === 'Property' && utils.getKeyName(p) === 'description',
);
if (!descriptionNode) {
context.report({
node: docsNode || metaNode || ruleInfo.create,
messageId: 'missing',
});
return;
}

if (!descriptionNode) {
context.report({
Expand All @@ -87,6 +84,10 @@ module.exports = {
return;
}

const pattern = context.options[0]?.pattern
? new RegExp(context.options[0].pattern)
: DEFAULT_PATTERN;

if (typeof staticValue.value !== 'string' || staticValue.value === '') {
context.report({
node: descriptionNode.value,
Expand Down
77 changes: 77 additions & 0 deletions lib/rules/require-meta-docs-recommended.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
'use strict';

const { getStaticValue } = require('eslint-utils');
const utils = require('../utils');

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'suggestion',
docs: {
description:
'require rules to implement a `meta.docs.recommended` property',
category: 'Rules',
recommended: false,
url: 'https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/require-meta-docs-recommended.md',
},
fixable: null,
schema: [
{
type: 'object',
properties: {
allowNonBoolean: {
default: false,
description: 'Whether to allow values of types other than boolean.',
type: 'boolean',
},
},
additionalProperties: false,
},
],
messages: {
incorrect: '`meta.docs.recommended` is required to be a boolean.',
missing: '`meta.docs.recommended` is required.',
},
},

create(context) {
const sourceCode = context.sourceCode || context.getSourceCode(); // TODO: just use context.sourceCode when dropping eslint < v9
const ruleInfo = utils.getRuleInfo(sourceCode);
if (!ruleInfo) {
return {};
}

const { scopeManager } = sourceCode;
const {
docsNode,
metaNode,
metaPropertyNode: descriptionNode,
} = utils.getMetaDocsProperty('recommended', ruleInfo, scopeManager);

if (!descriptionNode) {
context.report({
node: docsNode || metaNode || ruleInfo.create,
messageId: 'missing',
});
return {};
}

if (context.options[0]?.allowNonBoolean) {
return {};
}

const staticValue = utils.isUndefinedIdentifier(descriptionNode.value)
? { value: undefined }
: getStaticValue(descriptionNode.value);

if (staticValue && typeof staticValue.value !== 'boolean') {
context.report({
node: descriptionNode.value,
messageId: 'incorrect',
});
return {};
}

return {};
},
};
37 changes: 14 additions & 23 deletions lib/rules/require-meta-docs-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// -----------------------------------------------------------------------------

const path = require('path');
const util = require('../utils');
const utils = require('../utils');
const { getStaticValue } = require('eslint-utils');

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -77,7 +77,7 @@ module.exports = {
}

const sourceCode = context.sourceCode || context.getSourceCode(); // TODO: just use context.sourceCode when dropping eslint < v9
const ruleInfo = util.getRuleInfo(sourceCode);
const ruleInfo = utils.getRuleInfo(sourceCode);
if (!ruleInfo) {
return {};
}
Expand All @@ -87,16 +87,11 @@ module.exports = {
const scope = sourceCode.getScope?.(ast) || context.getScope(); // TODO: just use sourceCode.getScope() when we drop support for ESLint < v9.0.0
const { scopeManager } = sourceCode;

const metaNode = ruleInfo.meta;
const docsPropNode = util
.evaluateObjectProperties(metaNode, scopeManager)
.find((p) => p.type === 'Property' && util.getKeyName(p) === 'docs');
const urlPropNode = util
.evaluateObjectProperties(
docsPropNode && docsPropNode.value,
scopeManager,
)
.find((p) => p.type === 'Property' && util.getKeyName(p) === 'url');
const {
docsNode,
metaNode,
metaPropertyNode: urlPropNode,
} = utils.getMetaDocsProperty('url', ruleInfo, scopeManager);

const staticValue = urlPropNode
? getStaticValue(urlPropNode.value, scope)
Expand All @@ -113,7 +108,7 @@ module.exports = {
context.report({
node:
(urlPropNode && urlPropNode.value) ||
(docsPropNode && docsPropNode.value) ||
(docsNode && docsNode.value) ||
metaNode ||
ruleInfo.create,

Expand All @@ -138,27 +133,23 @@ module.exports = {
if (urlPropNode) {
if (
urlPropNode.value.type === 'Literal' ||
(urlPropNode.value.type === 'Identifier' &&
urlPropNode.value.name === 'undefined')
utils.isUndefinedIdentifier(urlPropNode.value)
) {
return fixer.replaceText(urlPropNode.value, urlString);
}
} else if (
docsPropNode &&
docsPropNode.value.type === 'ObjectExpression'
) {
return util.insertProperty(
} else if (docsNode && docsNode.value.type === 'ObjectExpression') {
return utils.insertProperty(
fixer,
docsPropNode.value,
docsNode.value,
`url: ${urlString}`,
sourceCode,
);
} else if (
!docsPropNode &&
!docsNode &&
metaNode &&
metaNode.type === 'ObjectExpression'
) {
return util.insertProperty(
return utils.insertProperty(
fixer,
metaNode,
`docs: {\nurl: ${urlString}\n}`,
Expand Down
3 changes: 1 addition & 2 deletions lib/rules/require-meta-has-suggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ module.exports = {
fix(fixer) {
if (
hasSuggestionsProperty.value.type === 'Literal' ||
(hasSuggestionsProperty.value.type === 'Identifier' &&
hasSuggestionsProperty.value.name === 'undefined')
utils.isUndefinedIdentifier(hasSuggestionsProperty.value)
) {
return fixer.replaceText(
hasSuggestionsProperty.value,
Expand Down
5 changes: 1 addition & 4 deletions lib/rules/require-meta-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,7 @@ module.exports = {
hasEmptySchema = true;
}

if (
value.type === 'Literal' ||
(value.type === 'Identifier' && value.name === 'undefined')
) {
if (value.type === 'Literal' || utils.isUndefinedIdentifier(value)) {
context.report({ node: value, messageId: 'wrongType' });
}
},
Expand Down
28 changes: 28 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,26 @@ module.exports = {
});
},

getMetaDocsProperty(propertyName, ruleInfo, scopeManager) {
const metaNode = ruleInfo.meta;

const docsNode = module.exports
.evaluateObjectProperties(metaNode, scopeManager)
.find(
(p) => p.type === 'Property' && module.exports.getKeyName(p) === 'docs',
);

const metaPropertyNode = module.exports
.evaluateObjectProperties(docsNode?.value, scopeManager)
.find(
(p) =>
p.type === 'Property' &&
module.exports.getKeyName(p) === propertyName,
);

return { docsNode, metaNode, metaPropertyNode };
},

/**
* Get the `meta.messages` node from a rule.
* @param {RuleInfo} ruleInfo
Expand Down Expand Up @@ -919,6 +939,14 @@ module.exports = {
});
},

/**
* @param {Node} node
* @returns {boolean} Whether the node is an Identifier with name `undefined`.
*/
isUndefinedIdentifier(node) {
return node.type === 'Identifier' && node.name === 'undefined';
},

/**
* Check whether a variable's definition is from a function parameter.
* @param {Node} node - the Identifier node for the variable.
Expand Down
Loading

0 comments on commit 2370b46

Please sign in to comment.