Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Request: no-empty should ignore empty catch blocks #1088

Closed
RomkeVdMeulen opened this issue Apr 5, 2016 · 7 comments · Fixed by #2886
Closed

Request: no-empty should ignore empty catch blocks #1088

RomkeVdMeulen opened this issue Apr 5, 2016 · 7 comments · Fixed by #2886

Comments

@RomkeVdMeulen
Copy link
Contributor

I'm getting a no-empty error on this:

try {
  doSomethingThatMayFail();
} catch (e) {
}

but not this:

try {
  doSomethingThatMayFail();
} catch (e) {
  // ignore this
}

Of course it's open to interpretation, but I would expect the first example to also be okay. What do you think?

@m-abs
Copy link

m-abs commented May 24, 2016

I'd like this too. +1

@jkillian
Copy link
Contributor

This could be added as an option to the rule: allow-empty-catch

@adidahiya
Copy link
Contributor

I don't really feel comfortable making this the only exception to the no-empty rule. Why should this be treated specially?

@ikokostya
Copy link

I don't really feel comfortable making this the only exception to the no-empty rule. Why should this be treated specially?

Because add comment to every empty block is annoying. Eslint has similar option http://eslint.org/docs/rules/no-empty#options. Why not make this behavior configurable for users?

@adidahiya
Copy link
Contributor

Why not make this behavior configurable for users?

alright, sure, go for it, we'll accept a PR (but it won't make it into tslint:recommended).

@RomkeVdMeulen
Copy link
Contributor Author

Hasn't there been a PR for this already which got reverted? Why not change it back?

@Izhaki
Copy link

Izhaki commented Jun 1, 2017

Here's another case:

            try {
                await reject(this.dummyTask, 'Reject reason')
            } catch (err) {}

We want to reject a promise in an async block (in tests).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants