Skip to content
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

Make no-useless-catch a core rule #11174

Closed
agrasley opened this issue Dec 9, 2018 · 6 comments
Closed

Make no-useless-catch a core rule #11174

agrasley opened this issue Dec 9, 2018 · 6 comments

Comments

@agrasley
Copy link
Contributor

@agrasley agrasley commented Dec 9, 2018

Please describe what the rule should do:

Prevents useless catch clauses that do nothing but rethrow the caught error. This rule already exists as an ESLint internal rule. Instead of just an internal rule, this should be a core rule.

What category of rule is this? (place an "X" next to just one item)

[ ] Warns about a potential error (problem)
[X] Suggests an alternate way of doing something (suggestion)
[ ] Enforces code style (layout)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

/* warn about useless try/catch statement */
try {
  throw new Error('whoops');
} catch (e) {
  throw e;
}

/* only warn about catch clause */
try {
  throw new Error('whoops');
} catch (e) {
  throw e;
} finally {
  console.log('done');
}

Why should this rule be included in ESLint (instead of a plugin)?

As far as I know, there is no semantic difference between

try {
  throw new Error('whoops');
} catch (e) {
  throw e;
}

and

throw new Error('whoops');

Because of that, I can't think of a good reason why you would ever want a useless catch clause in any javascript codebase, seeing as how it just adds unnecessary clutter and complexity to your source code. It seems ESLint devs themselves agree since it exists as an internal rule.

Are you willing to submit a pull request to implement this rule?

If it is something that people want as a core rule, then sure.

@platinumazure
Copy link
Member

@platinumazure platinumazure commented Dec 9, 2018

I'll support this!

Although the rule definition exists, we would definitely need documentation and tests if we want to expose the rule as part of ESLint core.

@agrasley
Copy link
Contributor Author

@agrasley agrasley commented Dec 10, 2018

Cool. I'll look at existing rules and other PRs and try to come up with something.

agrasley pushed a commit to agrasley/eslint that referenced this issue Dec 16, 2018
Moves no-useless-catch from an internal rule to a core rule. Adds docs
and test cases.
agrasley pushed a commit to agrasley/eslint that referenced this issue Dec 16, 2018
Moves no-useless-catch from an internal rule to a core rule. Adds docs
and test cases.
@agrasley
Copy link
Contributor Author

@agrasley agrasley commented Dec 16, 2018

Okay, I've got a PR ready for this.

agrasley pushed a commit to agrasley/eslint that referenced this issue Dec 17, 2018
Moves no-useless-catch from an internal rule to a core rule. Adds docs
and test cases.
@platinumazure
Copy link
Member

@platinumazure platinumazure commented Dec 17, 2018

I've decided I'll champion this proposal.

@not-an-aardvark @nzakas @ilyavolodin @kaicataldo Thoughts on adding this rule to core? I think it's a great idea personally.

@platinumazure platinumazure self-assigned this Dec 17, 2018
@ilyavolodin
Copy link
Member

@ilyavolodin ilyavolodin commented Dec 17, 2018

Sounds good to me.

@platinumazure
Copy link
Member

@platinumazure platinumazure commented Dec 22, 2018

@not-an-aardvark Thoughts on this? This was accepted a little early but I'm hoping you might be willing to support this. There's a PR linked as well. Thanks!

Edit: Oops, you are one of the original supporters, sorry...

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jun 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants