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: no-useless-catch rule (fixes #11174) #11198
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.
Can we add some tests that ensure that it warns on the same pattern inside an async function
?
@ljharb Do you have a case in mind that you feel would be important to guard against for So would something like this be what you had in mind?: valid: async () => {
try {
await doSomething();
} catch (e) {
doSomethingAfterCatch();
throw e;
}
} invalid: async () => {
try {
await doSomething();
} catch (e) {
throw e;
}
} |
Both of those are perfect :-) i'm pretty confident they'll pass as-is, but this way the intention is clearly documented by the test cases for future explorers. |
Moves no-useless-catch from an internal rule to a core rule. Adds docs and test cases.
Okay, those have been added now. |
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!
Marking as accepted because #11174 has also been marked as accepted. |
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!
Only one thought: Should the docs make some explicit mention that try/catch (with rethrow)/finally can be replaced with try/finally? It's a corner case but some users might not be aware that try/finally workout catch is valid.
I've decided we can add to the documentation in a separate change, so I'll merge this. Thanks @agrasley for contributing! |
Moves no-useless-catch from an internal rule to a core rule. Adds docs and test cases.
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[X] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
#11174
What changes did you make? (Give an overview)
finally
clause.Is there anything you'd like reviewers to focus on?
Double-checking the type and category of the rule and whether it should be recommended. Also I don't think this is a candidate for the
--fix
option but if people believe it is and could give me pointers on how to implement it I'd be glad to do so.