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: object-property-newline end location (refs #12334) #13399
Update: object-property-newline end location (refs #12334) #13399
Conversation
d7478ed
to
5acc7cb
Compare
I think reporting the whitespace as suggested above makes more sense, personally! |
Cool, I will change that 👍 |
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.
The change itself looks fine. Can you please add a test?
cool |
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, and thanks for knocking another one of these rules out, @anikethsaha!
I noticed that just one of the tests hadn't yet had endLine
/endColumn
added, so I pushed a quick commit to add those to save you time.
While the suggestion to report the whitespace before the key rather than the key itself would be neat, it could drastically complicate the implementation due to potential placement of the comma, so I think warning on the key as we've been doing (now with an end location) is perfectly adequate.
Thanks a lot 💯 |
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)
[x] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
removed the
.start
of the reporting location to include bothstart
andend
as well.Is there anything you'd like reviewers to focus on?
Currently, the location of the key is being reported (both in master and in this PR)
Will it make sense to report the leading whitespace?
like this
by replacing the
loc