From 59bb32c93d1ba903480dc1e0f317f41063b061fe Mon Sep 17 00:00:00 2001
From: Teddy Katz <teddy.katz@gmail.com>
Date: Fri, 23 Feb 2018 16:43:20 -0500
Subject: [PATCH] Chore: avoid useless catch clauses that just rethrow errors

---
 .eslintrc.js                                  |  3 +-
 tests/lib/testers/no-test-runners.js          |  2 -
 .../tools/internal-rules/no-useless-catch.js  | 62 +++++++++++++++++++
 tools/internal-rules/no-useless-catch.js      | 44 +++++++++++++
 4 files changed, 108 insertions(+), 3 deletions(-)
 create mode 100644 tests/tools/internal-rules/no-useless-catch.js
 create mode 100644 tools/internal-rules/no-useless-catch.js

diff --git a/.eslintrc.js b/.eslintrc.js
index b7b73ec55e8..ea75710e81a 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -25,6 +25,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/multiline-comment-style": "error",
+        "rulesdir/no-useless-catch": "error"
     }
 };
diff --git a/tests/lib/testers/no-test-runners.js b/tests/lib/testers/no-test-runners.js
index b77c6bb9036..5ca2de12898 100644
--- a/tests/lib/testers/no-test-runners.js
+++ b/tests/lib/testers/no-test-runners.js
@@ -27,8 +27,6 @@ try {
             ]
         });
     }, /Output is incorrect\. \(' foo = bar;' == 'invalid output'\)$/);
-} catch (e) {
-    throw e;
 } finally {
     it = tmpIt;
     describe = tmpDescribe;
diff --git a/tests/tools/internal-rules/no-useless-catch.js b/tests/tools/internal-rules/no-useless-catch.js
new file mode 100644
index 00000000000..6e1a65099f7
--- /dev/null
+++ b/tests/tools/internal-rules/no-useless-catch.js
@@ -0,0 +1,62 @@
+/**
+ * @fileoverview Tests for no-useless-throw rule
+ * @author Teddy Katz
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../tools/internal-rules/no-useless-catch");
+const RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+new RuleTester({ parserOptions: { ecmaVersion: 6 } }).run("rulesdir/no-useless-catch", rule, {
+    valid: [
+        `
+            try {
+                foo();
+            } catch (err) {
+                console.error(err);
+            } finally {
+                foo;
+            }
+        `,
+        `
+            try {
+                foo();
+            } catch ({ err }) {
+                throw err;
+            }
+        `
+    ],
+    invalid: [
+        {
+            code: `
+                try {
+                    foo();
+                } catch (e) {
+                    throw e;
+                }
+            `,
+            errors: [{ message: "Unnecessary try/catch wrapper." }]
+        },
+        {
+            code: `
+                try {
+                    foo();
+                } catch (e) {
+                    throw e;
+                } finally {
+                    foo();
+                }
+            `,
+            errors: [{ message: "Unnecessary catch clause." }]
+        }
+    ]
+});
diff --git a/tools/internal-rules/no-useless-catch.js b/tools/internal-rules/no-useless-catch.js
new file mode 100644
index 00000000000..86e0e2a70f6
--- /dev/null
+++ b/tools/internal-rules/no-useless-catch.js
@@ -0,0 +1,44 @@
+/**
+ * @fileoverview Reports useless `catch` clauses that just rethrow their error.
+ * @author Teddy Katz
+ */
+
+"use strict";
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "disallow unnecessary `catch` clauses",
+            category: "Internal",
+            recommended: false
+        },
+
+        schema: []
+    },
+
+    create(context) {
+        return {
+            CatchClause(node) {
+                if (
+                    node.param.type === "Identifier" &&
+                    node.body.body.length &&
+                    node.body.body[0].type === "ThrowStatement" &&
+                    node.body.body[0].argument.type === "Identifier" &&
+                    node.body.body[0].argument.name === node.param.name
+                ) {
+                    if (node.parent.finalizer) {
+                        context.report({
+                            node,
+                            message: "Unnecessary catch clause."
+                        });
+                    } else {
+                        context.report({
+                            node,
+                            message: "Unnecessary try/catch wrapper."
+                        });
+                    }
+                }
+            }
+        };
+    }
+};