-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: reporting variable used in catch block in no-useless-assignment
#19423
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
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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. Would like @mdjermanovic to review before merging.
There are merge conflicts now. |
After thinking about this more, the problem is not only when variables are used in catch blocks. For example, this would still be a false positive: /*eslint no-useless-assignment: "error" */
function fn() {
let intermediaryValue;
try {
intermediaryValue = 42; // false positive
unsafeFn();
intermediaryValue = 55;
} catch {
// handle error
}
return intermediaryValue;
}
function unsafeFn() {
throw new Error();
}
console.log(fn()); // 42 So, I think instead of ignoring variables that are used in catch blocks, the rule should ignore assignments in try blocks. |
Done! should we document it also? |
I think it's a good idea to add a Known Limitations section with this detail. |
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! Leaving open for @nzakas to re-review.
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!
Co-authored-by: Nitin Kumar <[email protected]>
b6ad83f
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, and nice work on this.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
Tell us about your environment (
npx eslint --env-info
):What parser are you using (place an "X" next to just one item)?
[x]
Default (Espree)
[ ]
@typescript-eslint/parser
[ ]
@babel/eslint-parser
[ ]
vue-eslint-parser
[ ]
@angular-eslint/template-parser
[ ]
Other
Please show your full configuration:
Configuration
What did you do? Please include the actual source code causing the issue.\
What did you expect to happen?
intermediaryValue
won't get reported as it is used incatch
block.What actually happened? Please include the actual, raw output from ESLint.
it reports the
intermediaryValue
as unused variable.Is there anything you'd like reviewers to focus on?
fixes #19245