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
Update: add fixer for no-useless-computed-key
#7207
Update: add fixer for no-useless-computed-key
#7207
Conversation
@not-an-aardvark, thanks for your PR! By analyzing the annotation information on this pull request, we identified @vitorbal, @alberto and @BYK to be potential reviewers |
LGTM |
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.
One question about the square bracket search. (Accidentally started a review, this isn't really a review, sorry.)
data: { property: sourceCode.getText(key) }, | ||
fix(fixer) { | ||
const leftSquareBracket = sourceCode.getTokenBefore(node.key); | ||
const rightSquareBracket = sourceCode.getTokenAfter(node.key); |
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.
Question: Can parentheses get in the way here?
const obj = {
[('x')]: null
};
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.
Whoops, yes, that does cause issues; it causes the parens to be removed and not the square brackets. Thanks, I'll figure out how to fix that.
(Technically it doesn't cause incorrect behavior overall since the square brackets would be removed on the next pass, but it's worth fixing regardless.)
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.
Updated to fix that case (and added a test for it).
047ee83
to
3fe5763
Compare
LGTM |
Could you add the following 2 tests?
|
LGTM |
Updated to work properly for generator and async properties, and added tests for them. |
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! (I'd like at least one more review, though.)
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, thank you!
I'll champion this. |
LGTM. Thanks for contributing to ESLint! |
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[x] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
Please check each item to ensure your pull request is ready:
What changes did you make? (Give an overview)
This adds a fixer for
no-useless-computed-key
.The fixer removes the square brackets around unnecessary computed keys. For example,
({ ['x']: 0 })
is fixed to({ 'x': 0 })
.If there are comments between the square brackets and the key, no fix is performed.
Is there anything you'd like reviewers to focus on?
Nothing in particular.