Update: add fixer for dot-notation (fixes #7014) #7054
Conversation
|
@not-an-aardvark, thanks for your PR! By analyzing the annotation information on this pull request, we identified @michaelficarra, @gyandeeps and @nzakas to be potential reviewers |
|
LGTM |
kaicataldo
added
the
evaluating
|
LGTM but would like someone else to verify. |
|
|
||
| return fixer.replaceTextRange( | ||
| [leftBracket.range[0], rightBracket.range[1]], | ||
| `${textBeforeProperty}.${node.property.value}${textAfterProperty}` |
There was a problem hiding this comment.
This appears to replace a[ 'b' ] with a .b . This behaviour is not tested, and I don't think it's desirable. I would just use `.${node.property.value}` instead.
There was a problem hiding this comment.
Good point; this was originally there to ensure that comments didn't get deleted, but the rule now doesn't do the fix if there are comments anyway.
There was a problem hiding this comment.
Can you please add a test, then, that the spacing is not preserved?
not-an-aardvark
force-pushed
the
issue7014
branch
from
8be38f0
to
0aa69ab
Compare
|
LGTM |
not-an-aardvark
force-pushed
the
issue7014
branch
from
0aa69ab
to
f28a97c
Compare
|
LGTM |
1 similar comment
|
LGTM |
|
LGTM. Thanks for PR! |
eslint-deprecated
bot
added
the
archived due to age
What issue does this pull request address?
#7014
What changes did you make? (Give an overview)
This adds a fixer for the
dot-notationrule, as described in #7014. It does not make any fix if the original code contains comments in the property-brackets, or between the dot and the property name.Is there anything you'd like reviewers to focus on?
Nothing in particular.
#7014 has not been marked as accepted yet, so this PR should not be merged before that happens.