-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Fix: dot-location errors with parenthesized objects (fixes #11868) #11933
Conversation
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.
Thanks for the PR! This looks good to me, I just have a minor question.
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'm okay with leaving the if
check in for now.
I'll be away for two weeks from next Tuesday. If the |
My vote would be to remove it, but I also don’t want to hold up landing this. If you can remove it, that sounds good to me! Otherwise, I’d be fine with merging this and revisiting that. Thanks for working on this! |
It's removed now. It looks like this conditional (in the form: I didn't remove I've also added more test cases with computed properties. |
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!
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!
What is the purpose of this pull request? (put an "X" next to item)
[X] Bug fix #11868
What changes did you make? (Give an overview)
Fixed
dot-location
rule ("object"
option) because it produces syntax errors with parenthesized objects.Parentheses around
MemberExpression.object
node are now treated as a part of the 'object'.For example, the following was a false positive:
and the auto-fix produced syntax error:
With this PR, the example above does not have a warning.
Also, with this PR auto-fix works correctly for similar cases that do have warnings. For example, this:
will be fixed to:
I've also added several test cases unrelated to this issue.
Is there anything you'd like reviewers to focus on?
MemberExpression.property
is not adot
token, the rule will ignore that node. I believe there is no need to support this case (post).dot
is right after theobject
(or parens now) / right before theproperty
. This is valid syntax:a . b
and produces nodot-location
warnings. Perhaps there could be an additional option to 'attach' the dot, I could open an issue if this wasn't already discussed.