diff --git a/.eslintrc.js b/.eslintrc.js index 793772f08d7..22332c38998 100644 --- a/.eslintrc.js +++ b/.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: [ { diff --git a/conf/eslint-recommended.js b/conf/eslint-recommended.js index 847cd44c1cf..ca0ca8fa95d 100644 --- a/conf/eslint-recommended.js +++ b/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", diff --git a/docs/rules/no-useless-catch.md b/docs/rules/no-useless-catch.md new file mode 100644 index 00000000000..d320e561d07 --- /dev/null +++ b/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. diff --git a/tools/internal-rules/no-useless-catch.js b/lib/rules/no-useless-catch.js similarity index 74% rename from tools/internal-rules/no-useless-catch.js rename to lib/rules/no-useless-catch.js index 86e0e2a70f6..3211ed2c736 100644 --- a/tools/internal-rules/no-useless-catch.js +++ b/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." }); } diff --git a/packages/eslint-config-eslint/default.yml b/packages/eslint-config-eslint/default.yml index 42ce7b1ec7d..9fdb4c767db 100644 --- a/packages/eslint-config-eslint/default.yml +++ b/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" diff --git a/tests/lib/rules/no-useless-catch.js b/tests/lib/rules/no-useless-catch.js new file mode 100644 index 00000000000..8c1a7118e20 --- /dev/null +++ b/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" + }] + } + ] +}); diff --git a/tests/tools/internal-rules/no-useless-catch.js b/tests/tools/internal-rules/no-useless-catch.js deleted file mode 100644 index 6e1a65099f7..00000000000 --- a/tests/tools/internal-rules/no-useless-catch.js +++ /dev/null @@ -1,62 +0,0 @@ -/** - * @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/rule-types.json b/tools/rule-types.json index 40010c8969d..44c1d48abcd 100644 --- a/tools/rule-types.json +++ b/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",