New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat!: disallow multiple configuration comments for same rule #18157
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
💯 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Just a suggestion on improving the error message.
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
I have updated the message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Leaving open for a couple more days to give others a chance to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:
Fixes #18132
What changes did you make? (Give an overview)
Updated code that processes
/*eslint*/
comments (both in flat and eslintrc modes) to check if the rule has already been configured by another comment. In that case, the previous comment still applies, while the duplicate is reported as a lint error.Is there anything you'd like reviewers to focus on?
This doesn't cover cases where a rule appears multiple times in the same comment. Configuration comments are parsed by
levn
(with a fallback to JSON parsing), which allows duplicate keys in objects and just returns the last entry. Detecting these duplicates would probably require another tool or custom parsing. I think these cases are much less important than the cases with multiple comments.