-
-
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
Breaking: no-unused-vars reports all after-used params (fixes #9909) #10119
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.
Any suggestions for improving the new description I've put in the documentation?
I had one idea, let me know what you think!
Code LGTM, and test coverage looks to be sufficient.
docs/rules/no-unused-vars.md
Outdated
@@ -150,7 +150,7 @@ console.log(secondVar); | |||
|
|||
The `args` option has three settings: | |||
|
|||
* `after-used` - only the last argument must be used. This allows you, for instance, to have two named parameters to a function and as long as you use the second argument, ESLint will not warn you about the first. This is the default setting. | |||
* `after-used` - unused arguments that occur before the last used argument will not be checked, but all other arguments (including after the last used argument) will be checked. |
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.
What about this tweak?
"Unused positional arguments that occur before the last used argument will not be checked, but all named arguments and all positional arguments after the last used argument will be checked."
4a3b09b
to
0e246ae
Compare
@btmills Rebased and pushed an extra commit with your suggested wording, please review at your convenience. 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, nice work!
0e246ae
to
ee36e76
Compare
What is the purpose of this pull request? (put an "X" next to 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:
See #9909.
What changes did you make? (Give an overview)
The
no-unused-vars
rule was changed some time ago (likely unintentionally) to only report the last unused parameter when using theargs: "after-used"
setting. This resulted in poor UX in the case of a function with multiple unused parameters after the last used parameter, because ESLint would only report one parameter, and then report another parameter after the user fixed the first reported parameter.This change simplifies the
args: "after-used"
logic by using this simple algorithm to check if a parameter is "after used parameters": If there exists at least one parameter (declared after the one being examined) which has at least one reference, then the parameter being examined is not "after the last used parameter" and so it shouldn't be reported. This approach allows for multiple parameters to be reported, as needed.Is there anything you'd like reviewers to focus on?