-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: no-case-declarations
add suggestions
#18388
feat: no-case-declarations
add suggestions
#18388
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
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!
Leaving it open as, per our policies, it requires at least one additional approval from any other team member (because this is a new feature), and also a waiting period of 2 days before merging.
When a declared identifier is used in another // ecmaVersion: "latest"
switch ("foo") {
case "bar":
function baz() { }
break;
default:
baz();
} compared to: // ecmaVersion: "latest"
switch ("foo") {
case "bar":
{ function baz() { }
break; }
default:
baz(); // TypeError!
} I assume this is acceptable for a suggestion, but maybe we should add a unit test to show that we are aware of the risk and we know what we are doing? |
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! Leaving open to respect the the 2 days period.
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)
[x] Add
autofixsuggestion to a rule[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
Fixes #18349
What changes did you make? (Give an overview)
Adds a suggestion fixer to
no-case-declarations
to add{
}
brackets around blocks.Also adds a couple new tests to make sure switches with multiple violating cases report on the right case (consequent).
Is there anything you'd like reviewers to focus on?
I'd considered adding logic for friendlier
{
}
bracket placement but then decided against the added complexity.