New: no-useless-catch rule (fixes #11174)

[agrasley]

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)

• Moved no-useless-catch from an internal rule to a core rule.
• Made tests more robust for no-useless-catch.
• Added documentation for no-useless-catch.
• no-useless-catch now reports the whole TryStatement node when there is no 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.

[jsf-clabot]

All committers have signed the CLA.

[ljharb]

Can we add some tests that ensure that it warns on the same pattern inside an async function?

[agrasley]

@ljharb Do you have a case in mind that you feel would be important to guard against for async functions? I'm happy to add it but I want to make sure that we're on the same page as far as what it should be testing 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;
  }
}

[ljharb]

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.

[agrasley]

Okay, those have been added now.

[not-an-aardvark]

LGTM, thanks!

[not-an-aardvark]

Marking as accepted because #11174 has also been marked as accepted.

[platinumazure]

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.

[platinumazure]

I've decided we can add to the documentation in a separate change, so I'll merge this. Thanks @agrasley for contributing!