Skip to content

Commit

Permalink
feat: add require-meta-schema-description rule (#490)
Browse files Browse the repository at this point in the history
* feat: add require-meta-schema-description rule

* Update lib/rules/require-meta-schema-description.js

Co-authored-by: Brad Zacher <[email protected]>

* Added tests too

* recommended: false

* Fill in test coverage

* npm run update:eslint-docs

---------

Co-authored-by: Brad Zacher <[email protected]>
  • Loading branch information
JoshuaKGoldberg and bradzacher authored Oct 24, 2024
1 parent a3b3c05 commit 875200b
Show file tree
Hide file tree
Showing 12 changed files with 665 additions and 69 deletions.
49 changes: 25 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,30 +80,31 @@ module.exports = [

### Rules

| Name                          | Description | 💼 | 🔧 | 💡 | 💭 |
| :--------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------- | :-- | :-- | :-- | :-- |
| [fixer-return](docs/rules/fixer-return.md) | require fixer functions to return a fix || | | |
| [meta-property-ordering](docs/rules/meta-property-ordering.md) | enforce the order of meta properties | | 🔧 | | |
| [no-deprecated-context-methods](docs/rules/no-deprecated-context-methods.md) | disallow usage of deprecated methods on rule context objects || 🔧 | | |
| [no-deprecated-report-api](docs/rules/no-deprecated-report-api.md) | disallow the version of `context.report()` with multiple arguments || 🔧 | | |
| [no-missing-message-ids](docs/rules/no-missing-message-ids.md) | disallow `messageId`s that are missing from `meta.messages` || | | |
| [no-missing-placeholders](docs/rules/no-missing-placeholders.md) | disallow missing placeholders in rule report messages || | | |
| [no-property-in-node](docs/rules/no-property-in-node.md) | disallow using `in` to narrow node types instead of looking at properties | | | | 💭 |
| [no-unused-message-ids](docs/rules/no-unused-message-ids.md) | disallow unused `messageId`s in `meta.messages` || | | |
| [no-unused-placeholders](docs/rules/no-unused-placeholders.md) | disallow unused placeholders in rule report messages || | | |
| [no-useless-token-range](docs/rules/no-useless-token-range.md) | disallow unnecessary calls to `sourceCode.getFirstToken()` and `sourceCode.getLastToken()` || 🔧 | | |
| [prefer-message-ids](docs/rules/prefer-message-ids.md) | require using `messageId` instead of `message` or `desc` to report rule violations || | | |
| [prefer-object-rule](docs/rules/prefer-object-rule.md) | disallow function-style rules || 🔧 | | |
| [prefer-placeholders](docs/rules/prefer-placeholders.md) | require using placeholders for dynamic report messages | | | | |
| [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 || 🔧 | | |
| [require-meta-schema](docs/rules/require-meta-schema.md) | require rules to implement a `meta.schema` property || | 💡 | |
| [require-meta-type](docs/rules/require-meta-type.md) | require rules to implement a `meta.type` property || | | |
| Name                            | Description | 💼 | 🔧 | 💡 | 💭 |
| :------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------- | :-- | :-- | :-- | :-- |
| [fixer-return](docs/rules/fixer-return.md) | require fixer functions to return a fix || | | |
| [meta-property-ordering](docs/rules/meta-property-ordering.md) | enforce the order of meta properties | | 🔧 | | |
| [no-deprecated-context-methods](docs/rules/no-deprecated-context-methods.md) | disallow usage of deprecated methods on rule context objects || 🔧 | | |
| [no-deprecated-report-api](docs/rules/no-deprecated-report-api.md) | disallow the version of `context.report()` with multiple arguments || 🔧 | | |
| [no-missing-message-ids](docs/rules/no-missing-message-ids.md) | disallow `messageId`s that are missing from `meta.messages` || | | |
| [no-missing-placeholders](docs/rules/no-missing-placeholders.md) | disallow missing placeholders in rule report messages || | | |
| [no-property-in-node](docs/rules/no-property-in-node.md) | disallow using `in` to narrow node types instead of looking at properties | | | | 💭 |
| [no-unused-message-ids](docs/rules/no-unused-message-ids.md) | disallow unused `messageId`s in `meta.messages` || | | |
| [no-unused-placeholders](docs/rules/no-unused-placeholders.md) | disallow unused placeholders in rule report messages || | | |
| [no-useless-token-range](docs/rules/no-useless-token-range.md) | disallow unnecessary calls to `sourceCode.getFirstToken()` and `sourceCode.getLastToken()` || 🔧 | | |
| [prefer-message-ids](docs/rules/prefer-message-ids.md) | require using `messageId` instead of `message` or `desc` to report rule violations || | | |
| [prefer-object-rule](docs/rules/prefer-object-rule.md) | disallow function-style rules || 🔧 | | |
| [prefer-placeholders](docs/rules/prefer-placeholders.md) | require using placeholders for dynamic report messages | | | | |
| [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 || 🔧 | | |
| [require-meta-schema](docs/rules/require-meta-schema.md) | require rules to implement a `meta.schema` property || | 💡 | |
| [require-meta-schema-description](docs/rules/require-meta-schema-description.md) | require rules `meta.schema` properties to include descriptions | | | | |
| [require-meta-type](docs/rules/require-meta-type.md) | require rules to implement a `meta.type` property || | | |

### Tests

Expand Down
88 changes: 88 additions & 0 deletions docs/rules/require-meta-schema-description.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Require rules `meta.schema` properties to include descriptions (`eslint-plugin/require-meta-schema-description`)

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

Defining a description in the schema for each rule option helps explain that option to users.
It also allows documentation generators such as [`eslint-doc-generator`](https://github.com/bmish/eslint-doc-generator) to generate more informative documentation for users.

## Rule Details

This rule requires that if a rule option has a property in the rule's `meta.schema`, it should have a `description`.

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

```js
/* eslint eslint-plugin/require-meta-schema-description: error */

module.exports = {
meta: {
schema: [
{
elements: { type: 'string' },
type: 'array',
},
],
},
create() {},
};

module.exports = {
meta: {
schema: [
{
properties: {
something: {
type: 'string',
},
},
type: 'object',
},
],
},
create() {},
};
```

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

```js
/* eslint eslint-plugin/require-meta-schema-description: error */

module.exports = {
meta: {
schema: [
{
description: 'Elements to allow.',
elements: { type: 'string' },
type: 'array',
},
],
},
create() {},
};

module.exports = {
meta: {
schema: [
{
oneOf: [
{
description: 'Elements to allow.',
elements: { type: 'string' },
type: 'array',
},
],
},
],
},
create() {},
};
```

## When Not To Use It

If your rule options are very simple and well-named, and your rule isn't used by developers outside of your immediate team, you may not need this rule.

## Further Reading

- [working-with-rules#options-schemas](https://eslint.org/docs/developer-guide/working-with-rules#options-schemas)
2 changes: 2 additions & 0 deletions lib/rules/consistent-output.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ module.exports = {
schema: [
{
type: 'string',
description:
"Whether to enforce having output assertions 'always' or to be 'consistent' when some cases have them.",
enum: ['always', 'consistent'],
default: 'consistent',
},
Expand Down
1 change: 1 addition & 0 deletions lib/rules/meta-property-ordering.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = {
schema: [
{
type: 'array',
description: 'What order to enforce for meta properties.',
elements: { type: 'string' },
},
],
Expand Down
7 changes: 6 additions & 1 deletion lib/rules/report-message-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ module.exports = {
url: 'https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/report-message-format.md',
},
fixable: null,
schema: [{ type: 'string' }],
schema: [
{
description: 'Format that all report messages must match.',
type: 'string',
},
],
messages: {
noMatch: "Report message does not match the pattern '{{pattern}}'.",
},
Expand Down
8 changes: 0 additions & 8 deletions lib/rules/require-meta-docs-description.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,6 @@ module.exports = {
return;
}

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

const staticValue = getStaticValue(descriptionNode.value, scope);
if (!staticValue) {
// Ignore non-static values since we can't determine what they look like.
Expand Down
109 changes: 109 additions & 0 deletions lib/rules/require-meta-schema-description.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
'use strict';

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

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'suggestion',
docs: {
description:
'require rules `meta.schema` properties to include descriptions',
category: 'Rules',
recommended: false,
url: 'https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/require-meta-schema-description.md',
},
schema: [],
messages: {
missingDescription: 'Schema option is missing an ajv description.',
},
},

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

const schemaNode = utils.getMetaSchemaNode(ruleInfo.meta, scopeManager);
if (!schemaNode) {
return {};
}

const schemaProperty = utils.getMetaSchemaNodeProperty(
schemaNode,
scopeManager,
);
if (schemaProperty?.type !== 'ArrayExpression') {
return {};
}

for (const element of schemaProperty.elements) {
checkSchemaElement(element, true);
}

return {};

function checkSchemaElement(node, isRoot) {
if (node.type !== 'ObjectExpression') {
return;
}

let hadChildren = false;
let hadDescription = false;

for (const { key, value } of node.properties) {
const staticKey =
key.type === 'Identifier' ? { value: key.name } : getStaticValue(key);
if (!staticKey?.value) {
continue;
}

switch (key.name ?? key.value) {
case 'description': {
hadDescription = true;
break;
}

case 'oneOf': {
hadChildren = true;

if (value.type === 'ArrayExpression') {
for (const element of value.elements) {
checkSchemaElement(element, isRoot);
}
}

break;
}

case 'properties': {
hadChildren = true;

for (const property of value.properties) {
if (property.value?.type === 'ObjectExpression') {
checkSchemaElement(property.value);
}
}

break;
}
}
}

if (!hadDescription && !(hadChildren && isRoot)) {
context.report({
messageId: 'missingDescription',
node,
});
}
}
},
};
51 changes: 16 additions & 35 deletions lib/rules/require-meta-schema.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

const { findVariable } = require('@eslint-community/eslint-utils');
const utils = require('../utils');

// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -52,7 +51,6 @@ module.exports = {

let contextIdentifiers;
const metaNode = ruleInfo.meta;
let schemaNode;

// Options
const requireSchemaPropertyWhenOptionless =
Expand All @@ -62,52 +60,35 @@ module.exports = {
let hasEmptySchema = false;
let isUsingOptions = false;

const schemaNode = utils.getMetaSchemaNode(metaNode, scopeManager);
const schemaProperty = utils.getMetaSchemaNodeProperty(
schemaNode,
scopeManager,
);

return {
Program(ast) {
contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast);

schemaNode = utils
.evaluateObjectProperties(metaNode, scopeManager)
.find(
(p) => p.type === 'Property' && utils.getKeyName(p) === 'schema',
);

if (!schemaNode) {
if (!schemaProperty) {
return;
}

let { value } = schemaNode;
if (value.type === 'Identifier' && value.name !== 'undefined') {
const variable = findVariable(
scopeManager.acquire(value) || scopeManager.globalScope,
value,
);

// If we can't find the declarator, we have to assume it's in correct type
if (
!variable ||
!variable.defs ||
!variable.defs[0] ||
!variable.defs[0].node ||
variable.defs[0].node.type !== 'VariableDeclarator' ||
!variable.defs[0].node.init
) {
return;
}

value = variable.defs[0].node.init;
}

if (
(value.type === 'ArrayExpression' && value.elements.length === 0) ||
(value.type === 'ObjectExpression' && value.properties.length === 0)
(schemaProperty.type === 'ArrayExpression' &&
schemaProperty.elements.length === 0) ||
(schemaProperty.type === 'ObjectExpression' &&
schemaProperty.properties.length === 0)
) {
// Schema is explicitly defined as having no options.
hasEmptySchema = true;
}

if (value.type === 'Literal' || utils.isUndefinedIdentifier(value)) {
context.report({ node: value, messageId: 'wrongType' });
if (
schemaProperty.type === 'Literal' ||
utils.isUndefinedIdentifier(schemaProperty)
) {
context.report({ node: schemaProperty, messageId: 'wrongType' });
}
},

Expand Down
1 change: 1 addition & 0 deletions lib/rules/test-case-property-ordering.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports = {
schema: [
{
type: 'array',
description: 'What order to enforce for test case properties.',
elements: { type: 'string' },
},
],
Expand Down
Loading

0 comments on commit 875200b

Please sign in to comment.