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
New: Add grouped-accessor-pairs rule (fixes #12277) #12331
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.
Could this rule be made to autofix, perhaps only in the absence of any computed or duplicate getters or setters?
I think it's possible for this rule to have autofix, in general. An issue with this could be comments around the code that is removed and around its new position, as the fixer can't know what are they related to. Also, the fixer should check for a non-accessor duplicate between, because it 'breaks' the accessor property in the original code. Similar, a spread between could potentially do the same. There are probably even more things to analyse further. Maybe to start without the autofix and add it later? :) |
Regarding the autofix, a getter/setter defined far below its pair could be a naming error. Perhaps better let the user see what's going on and fix it manually. Could be safer to fix only the adjacent ones (switch positions if the order is incorrect). |
They can use git diffs to see that, just like any autofix that might mask a small error, but won't change the behavior. |
I think it's fine to add autofixing later 👍 |
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
bc5102a
to
5896421
Compare
it is also allowed to define a getter/setter using |
It wasn't initially. I thought that this rule wouldn't provide much value in that case, since the getter and the setter for the same key are already "grouped" by being in the same property descriptor. While thinking about it again, it can be useful when one wants to enforce the consistent order (e.g., get before set). Maybe to add this as an option later? (it's a completely different code, like a rule within the rule). |
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! Appreciate the thorough documentation and tests :)
Thanks for contributing! |
What is the purpose of this pull request? (put an "X" next to item)
[X] New rule #12277
Please describe what the rule should do:
Checks object literals, class declarations and class expressions.
If a property has a getter and a setter, their definitions should be one right after the other.
Optionally, the rule can also enforce consistent order (
getBeforeSet
orsetBeforeGet
).What category of rule is this? (place an "X" next to just one item)
[X] Enforces code style
Provide 2-3 code examples that this rule will warn about:
What changes did you make? (Give an overview)
Added new rule.
Is there anything you'd like reviewers to focus on?