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

Moves no-useless-catch from an internal rule to a core rule. Adds docs
and test cases.

.eslintrc.js

@@ -23,8 +23,7 @@ module.exports = {
         "eslint-plugin/report-message-format": ["error", "[^a-z].*\\.$"],
         "eslint-plugin/test-case-property-ordering": "error",
         "eslint-plugin/test-case-shorthand-strings": "error",
-        "rulesdir/multiline-comment-style": "error",
-        "rulesdir/no-useless-catch": "error"
+        "rulesdir/multiline-comment-style": "error"
     },
     overrides: [
         {

conf/eslint-recommended.js

@@ -207,6 +207,7 @@ module.exports = {
         "no-unused-vars": "error",
         "no-use-before-define": "off",
         "no-useless-call": "off",
+        "no-useless-catch": "off",
         "no-useless-computed-key": "off",
         "no-useless-concat": "off",
         "no-useless-constructor": "off",

docs/rules/no-useless-catch.md

@@ -0,0 +1,50 @@
+# Disallow unnecessary catch clauses (no-useless-catch)
+
+A `catch` clause that only rethrows the original error is redundant, and has no effect on the runtime behavior of the program. These redundant clauses can be a source of confusion and code bloat, so it's better to disallow these unnecessary `catch` clauses.
+
+## Rule Details
+
+This rule reports `catch` clauses that only `throw` the caught error.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint no-useless-catch: "error"*/
+
+try {
+  doSomethingThatMightThrow();
+} catch (e) {
+  throw e;
+}
+
+try {
+  doSomethingThatMightThrow();
+} catch (e) {
+  throw e;
+} finally {
+  cleanUp();
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint no-useless-catch: "error"*/
+
+try {
+  doSomethingThatMightThrow();
+} catch (e) {
+  doSomethingBeforeRethrow();
+  throw e;
+}
+
+try {
+  doSomethingThatMightThrow();
+} catch (e) {
+  handleError(e);
+}
+```
+
+## When Not To Use It
+
+If you don't want to be notified about unnecessary catch clauses, you can safely disable this rule.

tools/internal-rules/no-useless-catch.js → lib/rules/no-useless-catch.js

@@ -5,12 +5,19 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
 module.exports = {
     meta: {
+        type: "suggestion",
+
         docs: {
             description: "disallow unnecessary `catch` clauses",
-            category: "Internal",
-            recommended: false
+            category: "Best Practices",
+            recommended: false,
+            url: "https://eslint.org/docs/rules/no-useless-catch"
         },
 
         schema: []
@@ -33,7 +40,7 @@ module.exports = {
                         });
                     } else {
                         context.report({
-                            node,
+                            node: node.parent,
                             message: "Unnecessary try/catch wrapper."
                         });
                     }

packages/eslint-config-eslint/default.yml

@@ -118,6 +118,7 @@ rules:
     no-unused-vars: ["error", {vars: "all", args: "after-used"}]
     no-use-before-define: "error"
     no-useless-call: "error"
+    no-useless-catch: "error"
     no-useless-computed-key: "error"
     no-useless-concat: "error"
     no-useless-constructor: "error"

tests/lib/rules/no-useless-catch.js

@@ -0,0 +1,185 @@
+/**
+ * @fileoverview Tests for no-useless-throw rule
+ * @author Teddy Katz
+ * @author Alex Grasley
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/no-useless-catch"),
+    RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester();
+
+ruleTester.run("no-useless-catch", rule, {
+    valid: [
+        `
+            try {
+                foo();
+            } catch (err) {
+                console.error(err);
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch (err) {
+                console.error(err);
+            } finally {
+                bar();
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch (err) {
+                doSomethingBeforeRethrow();
+                throw err;
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch (err) {
+                throw err.msg;
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch (err) {
+                throw new Error("whoops!");
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch (err) {
+                throw bar;
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch (err) { }
+        `,
+        {
+            code: `
+                try {
+                    foo();
+                } catch ({ err }) {
+                    throw err;
+                }
+            `,
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: `
+                try {
+                    foo();
+                } catch ([ err ]) {
+                    throw err;
+                }
+            `,
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: `
+                async () => {
+                    try {
+                        await doSomething();
+                    } catch (e) {
+                        doSomethingAfterCatch();
+                        throw e;
+                    }
+                }
+            `,
+            parserOptions: { ecmaVersion: 8 }
+        }
+
+    ],
+    invalid: [
+        {
+            code: `
+                try {
+                    foo();
+                } catch (err) {
+                    throw err;
+                }
+            `,
+            errors: [{
+                message: "Unnecessary try/catch wrapper.",
+                type: "TryStatement"
+            }]
+        },
+        {
+            code: `
+                try {
+                    foo();
+                } catch (err) {
+                    throw err;
+                } finally {
+                    foo();
+                }
+            `,
+            errors: [{
+                message: "Unnecessary catch clause.",
+                type: "CatchClause"
+            }]
+        },
+        {
+            code: `
+                try {
+                    foo();
+                } catch (err) {
+                    /* some comment */
+                    throw err;
+                }
+            `,
+            errors: [{
+                message: "Unnecessary try/catch wrapper.",
+                type: "TryStatement"
+            }]
+        },
+        {
+            code: `
+                try {
+                    foo();
+                } catch (err) {
+                    /* some comment */
+                    throw err;
+                } finally {
+                    foo();
+                }
+            `,
+            errors: [{
+                message: "Unnecessary catch clause.",
+                type: "CatchClause"
+            }]
+        },
+        {
+            code: `
+                async () => {
+                    try {
+                        await doSomething();
+                    } catch (e) {
+                        throw e;
+                    }
+                }
+            `,
+            parserOptions: { ecmaVersion: 8 },
+            errors: [{
+                message: "Unnecessary try/catch wrapper.",
+                type: "TryStatement"
+            }]
+        }
+    ]
+});

tools/rule-types.json

@@ -196,6 +196,7 @@
     "no-unused-vars": "problem",
     "no-use-before-define": "problem",
     "no-useless-call": "suggestion",
+    "no-useless-catch": "suggestion",
     "no-useless-computed-key": "suggestion",
     "no-useless-concat": "suggestion",
     "no-useless-constructor": "suggestion",