diff --git a/README.md b/README.md index 5af4e7b2..a189e245 100644 --- a/README.md +++ b/README.md @@ -68,34 +68,35 @@ export default [ ### 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-meta-replaced-by](docs/rules/no-meta-replaced-by.md) | disallow using the `meta.replacedBy` rule property | ✅ | | | | -| [no-meta-schema-default](docs/rules/no-meta-schema-default.md) | disallow rules `meta.schema` properties to include defaults | ✅ | | | | -| [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-default-options](docs/rules/require-meta-default-options.md) | require only rules with options to implement a `meta.defaultOptions` property | ✅ | 🔧 | | | -| [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 | ✅ | | | | +| 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-matching-violation-suggest-message-ids](docs/rules/no-matching-violation-suggest-message-ids.md) | require suggestions to have different `messageId` than their parent report | | | | | +| [no-meta-replaced-by](docs/rules/no-meta-replaced-by.md) | disallow using the `meta.replacedBy` rule property | ✅ | | | | +| [no-meta-schema-default](docs/rules/no-meta-schema-default.md) | disallow rules `meta.schema` properties to include defaults | ✅ | | | | +| [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-default-options](docs/rules/require-meta-default-options.md) | require only rules with options to implement a `meta.defaultOptions` property | ✅ | 🔧 | | | +| [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 diff --git a/docs/rules/no-matching-violation-suggest-message-ids.md b/docs/rules/no-matching-violation-suggest-message-ids.md new file mode 100644 index 00000000..68a1ce65 --- /dev/null +++ b/docs/rules/no-matching-violation-suggest-message-ids.md @@ -0,0 +1,69 @@ +# Require suggestions to have different `messageId` than their parent report (`eslint-plugin/no-matching-violation-suggest-message-ids`) + + + +When providing fix suggestions to a reported problem, it's important to have an actionable `messageId` for each suggestion rather than reusing the same `messageId` as the main report. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```js +/* eslint eslint-plugin/no-matching-violation-suggest-message-ids: error */ + +module.exports = { + meta: { messages: { notAllowed: '`DebuggerStatement`s are not allowed' } }, + + create(context) { + return { + DebuggerStatement(node) { + context.report({ + node, + messageId: 'notAllowed', + suggest: [{ messageId: 'notAllowed' }], + }); + }, + }; + }, +}; +``` + +Examples of **correct** code for this rule: + +```js +/* eslint eslint-plugin/no-matching-violation-suggest-message-ids: error */ + +module.exports = { + meta: { + messages: { + notAllowed: '`DebuggerStatement`s are not allowed', + remove: 'Remove the debugger statement', + }, + }, + + create(context) { + return { + DebuggerStatement(node) { + context.report({ + node, + messageId: 'notAllowed', + suggest: [ + { + messageId: 'remove', + fix: (fixer) => fixer.remove(node), + }, + ], + }); + }, + }; + }, +}; +``` + +## Further Reading + +- [ESLint rule docs: Providing Suggestions](https://eslint.org/docs/latest/extend/custom-rules#providing-suggestions) +- [ESLint rule docs: Suggestion `messageId`s](https://eslint.org/docs/latest/extend/custom-rules#suggestion-messageids) +- [no-missing-message-ids](./no-missing-message-ids.md) rule +- [no-unused-message-ids](./no-unused-message-ids.md) rule +- [prefer-message-ids](./prefer-message-ids.md) rule diff --git a/lib/index.ts b/lib/index.ts index 11ba765e..d8af58de 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -12,6 +12,7 @@ import metaPropertyOrdering from './rules/meta-property-ordering.ts'; import noDeprecatedContextMethods from './rules/no-deprecated-context-methods.ts'; import noDeprecatedReportApi from './rules/no-deprecated-report-api.ts'; import noIdenticalTests from './rules/no-identical-tests.ts'; +import noMatchingViolationSuggestMessageIds from './rules/no-matching-violation-suggest-message-ids.ts'; import noMetaReplacedBy from './rules/no-meta-replaced-by.ts'; import noMetaSchemaDefault from './rules/no-meta-schema-default.ts'; import noMissingMessageIds from './rules/no-missing-message-ids.ts'; @@ -92,6 +93,8 @@ const allRules = { 'no-deprecated-context-methods': noDeprecatedContextMethods, 'no-deprecated-report-api': noDeprecatedReportApi, 'no-identical-tests': noIdenticalTests, + 'no-matching-violation-suggest-message-ids': + noMatchingViolationSuggestMessageIds, 'no-meta-replaced-by': noMetaReplacedBy, 'no-meta-schema-default': noMetaSchemaDefault, 'no-missing-message-ids': noMissingMessageIds, diff --git a/lib/rules/no-matching-violation-suggest-message-ids.ts b/lib/rules/no-matching-violation-suggest-message-ids.ts new file mode 100644 index 00000000..d1ddd319 --- /dev/null +++ b/lib/rules/no-matching-violation-suggest-message-ids.ts @@ -0,0 +1,127 @@ +import type { Rule, Scope } from 'eslint'; +import type { Node, CallExpression, MemberExpression } from 'estree'; + +import { + collectReportViolationAndSuggestionData, + findPossibleVariableValues, + getContextIdentifiers, + getReportInfo, + getRuleInfo, + isStringLiteral, +} from '../utils.ts'; +import type { StringLiteral, ViolationAndSuppressionData } from '../types.ts'; + +const rule: Rule.RuleModule = { + meta: { + type: 'suggestion', + docs: { + recommended: false, + description: + 'require suggestions to have different `messageId` than their parent report', + category: 'Rules', + url: 'https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/no-matching-violation-suggest-message-ids.md', + }, + schema: [], + messages: { + matchingMessageId: + 'Suggestion messageId "{{messageId}}" should not match its parent report messageId.', + }, + }, + + create(context) { + const sourceCode = context.sourceCode; + const { scopeManager } = sourceCode; + const ruleInfo = getRuleInfo(sourceCode); + if (!ruleInfo) { + return {}; + } + + const contextIdentifiers: Set = getContextIdentifiers( + scopeManager, + sourceCode.ast, + ); + + return { + 'CallExpression:has(>MemberExpression[property.name="report"])'( + _node: Rule.Node, + ) { + const node = _node as CallExpression & { callee: MemberExpression }; + + if (!contextIdentifiers.has(node.callee.object)) { + return; + } + + const reportInfo = getReportInfo(node, context); + + if (!reportInfo) { + return; + } + + const [report, ...suggestions] = + collectReportViolationAndSuggestionData(reportInfo).map((obj) => ({ + ...obj, + literalMessageIds: messageIdToLiteralValues( + obj.messageId, + scopeManager, + ), + })); + + if (report.literalMessageIds.length === 0 || suggestions.length === 0) { + return; + } + + const reportContainsMessageId = (messageId: StringLiteral) => { + return report.literalMessageIds.some( + (reportMessageId) => messageId.value === reportMessageId.value, + ); + }; + + suggestions.forEach((suggestion) => { + const matchingMessageId = suggestion.literalMessageIds.find( + reportContainsMessageId, + ); + + if (matchingMessageId) { + context.report({ + messageId: 'matchingMessageId', + node: matchingMessageId, + data: { + messageId: matchingMessageId.value, + }, + }); + } + }); + }, + }; + }, +}; + +function messageIdToLiteralValues( + messageId: ViolationAndSuppressionData['messageId'], + scopeManager: Scope.ScopeManager, +): StringLiteral[] { + if (!messageId) { + return []; + } + + if (isStringLiteral(messageId)) { + return [messageId]; + } + + if (messageId.type === 'ConditionalExpression') { + return [ + ...messageIdToLiteralValues(messageId.consequent, scopeManager), + ...messageIdToLiteralValues(messageId.alternate, scopeManager), + ]; + } + + if (messageId.type === 'Identifier') { + return findPossibleVariableValues(messageId, scopeManager).filter( + isStringLiteral, + ); + } + + return []; +} + +export default rule; diff --git a/lib/types.ts b/lib/types.ts index 72c9899a..960c2e4b 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -6,6 +6,7 @@ import type { Expression, FunctionDeclaration, FunctionExpression, + Literal, MaybeNamedClassDeclaration, MaybeNamedFunctionDeclaration, Node, @@ -81,3 +82,5 @@ export type MetaDocsProperty = { metaNode: Node | undefined; metaPropertyNode: Property | undefined; }; + +export type StringLiteral = Literal & { value: string }; diff --git a/lib/utils.ts b/lib/utils.ts index 5d901f3c..442ce24e 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -29,6 +29,7 @@ import type { MetaDocsProperty, PartialRuleInfo, RuleInfo, + StringLiteral, TestInfo, ViolationAndSuppressionData, } from './types.ts'; @@ -1115,6 +1116,10 @@ export function isUndefinedIdentifier(node: Node): boolean { return node.type === 'Identifier' && node.name === 'undefined'; } +export function isStringLiteral(node: Node): node is StringLiteral { + return node.type === 'Literal' && typeof node.value === 'string'; +} + /** * Check whether a variable's definition is from a function parameter. * @param node - the Identifier node for the variable. diff --git a/tests/lib/rules/no-matching-violation-suggest-message-ids.ts b/tests/lib/rules/no-matching-violation-suggest-message-ids.ts new file mode 100644 index 00000000..51201cd1 --- /dev/null +++ b/tests/lib/rules/no-matching-violation-suggest-message-ids.ts @@ -0,0 +1,252 @@ +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +import rule from '../../../lib/rules/no-matching-violation-suggest-message-ids.ts'; +import { RuleTester } from 'eslint'; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + languageOptions: { sourceType: 'commonjs' }, +}); + +ruleTester.run('no-matching-violation-suggest-message-ids', rule, { + valid: [ + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + }); + } + };`, + name: 'no suggestions', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation', suggestion: 'suggestion' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + suggest: [{ messageId: 'suggestion' }], + }); + } + };`, + name: 'different messageIds', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation' } }, + + create(context) { + notContext.report({ + node, + messageId: 'violation', + suggest: [{ messageId: 'violation' }], + }); + } + };`, + name: 'report function not from context', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation' } }, + + create(context) { + context.notReport({ + node, + messageId: 'violation', + suggest: [{ messageId: 'violation' }], + }); + } + };`, + name: 'context function other than report', + }, + { + code: ` + const suggestionMessageId = 'suggestion'; + + module.exports = { + meta: { messages: { violation: 'violation', suggestion: 'suggestion' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + suggest: [{ messageId: suggestionMessageId }], + }); + } + };`, + name: 'suggestion messageId from external variable', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation' } }, + + create(context) {} + }; + + context.report({ + messageId: 'violation', + suggest: [{ messageId: 'violation' }], + });`, + name: 'report outside of a rule', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation', otherViolation: 'other violation', suggestion: 'suggestion' } }, + + create(context) { + context.report({ + node, + messageId: Math.random() > 0.5 ? 'violation' : 'otherViolation', + suggest: [{ + messageId: 'suggestion', + }], + }); + } + };`, + name: 'conditional violation messageId', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation', suggestion: 'suggestion', otherSuggestion: 'other suggestion' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + suggest: [{ + messageId: Math.random() > 0.5 ? 'suggestion' : 'otherSuggestion', + }], + }); + } + };`, + name: 'conditional suggestion messageId', + }, + ], + + invalid: [ + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + suggest: [{ messageId: 'violation' }], + }); + } + };`, + errors: [ + { + messageId: 'matchingMessageId', + data: { messageId: 'violation' }, + type: 'Literal', + line: 9, + column: 38, + endColumn: 49, + }, + ], + name: 'matching messageId', + }, + { + code: ` + const suggestionMessageId = 'violation'; + + module.exports = { + meta: { messages: { violation: 'violation' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + suggest: [{ messageId: suggestionMessageId }], + }); + } + };`, + errors: [ + { + messageId: 'matchingMessageId', + data: { messageId: 'violation' }, + type: 'Literal', + line: 2, + column: 37, + endColumn: 48, + }, + ], + name: 'suggestion messageId from external variable', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation', otherViolation: 'other violation' } }, + + create(context) { + context.report({ + node, + messageId: Math.random() > 0.5 ? 'violation' : 'otherViolation', + suggest: [{ + messageId: 'violation', + }], + }); + } + };`, + errors: [ + { + messageId: 'matchingMessageId', + data: { messageId: 'violation' }, + type: 'Literal', + line: 10, + column: 28, + endColumn: 39, + }, + ], + name: 'conditional violation messageId', + }, + { + code: ` + module.exports = { + meta: { messages: { violation: 'violation', suggestion: 'suggestion' } }, + + create(context) { + context.report({ + node, + messageId: 'violation', + suggest: [{ + messageId: Math.random() > 0.5 ? 'suggestion' : 'violation', + }], + }); + } + };`, + errors: [ + { + messageId: 'matchingMessageId', + data: { messageId: 'violation' }, + type: 'Literal', + line: 10, + column: 65, + endColumn: 76, + }, + ], + name: 'conditional suggestion messageId', + }, + ], +});