feat: add new rule no-matching-violation-suggest-message-ids#567
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new ESLint rule no-matching-violation-suggest-message-ids that enforces suggestions to have different messageIds than their parent report violations. This helps ensure that suggestion messages are actionable and distinct from the main violation message.
Key Changes:
- Introduced the new rule implementation with support for static and dynamic messageId detection
- Added comprehensive test coverage for various scenarios including conditional expressions and external variables
- Extended utility functions to support string literal type checking
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/rules/no-matching-violation-suggest-message-ids.ts | Implements the core rule logic for detecting matching messageIds between violations and suggestions |
| tests/lib/rules/no-matching-violation-suggest-message-ids.ts | Provides comprehensive test cases covering valid and invalid scenarios |
| lib/utils.ts | Adds isStringLiteral utility function for type checking |
| lib/types.ts | Defines StringLiteral type for type-safe string literal handling |
| lib/index.ts | Registers the new rule in the plugin's rule registry |
| docs/rules/no-matching-violation-suggest-message-ids.md | Documents the rule with examples and usage guidance |
| README.md | Updates the rules table to include the new rule |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
aladdin-add
left a comment
aladdin-add
left a comment
There was a problem hiding this comment.
Looks really good, just a minor suggestion, thanks! 👍
| let contextIdentifiers: Set<Node>; | ||
|
|
||
| return { | ||
| Program(ast) { | ||
| contextIdentifiers = getContextIdentifiers(scopeManager, ast); | ||
| }, | ||
|
|
There was a problem hiding this comment.
you can just get the ast directly.
| let contextIdentifiers: Set<Node>; | |
| return { | |
| Program(ast) { | |
| contextIdentifiers = getContextIdentifiers(scopeManager, ast); | |
| }, | |
| const contextIdentifiers: Set<Node> = getContextIdentifiers(scopeManager, sourceCode.ast); | |
| return { | |
|
and the rule is a candidate to be enabled in recommended config. refs: #538 |
Should i open a draft PR for this after merging? |
please feel free, we'll just merge it when we're ready for a major release. |
…ttps://github.com/StyleShit/eslint-plugin-eslint-plugin into feat/368-no-matching-violation-suggest-message-ids
you mean add it as recommended here and you'll merge this PR to the major release? or you'll merge this and then i'll open a pr for recommended? |
|
sorry for not clear - i mean another pr to add it to recommended config. 🙏 |
|
Opened #570 |
Closes #368