Skip to content

Commit f02c0e9

Browse files
committed
feat: port rule no-useless-catch
1 parent 1e5a9c6 commit f02c0e9

File tree

7 files changed

+269
-0
lines changed

7 files changed

+269
-0
lines changed

internal/config/config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ import (
123123
"github.com/web-infra-dev/rslint/internal/rules/no_loss_of_precision"
124124
"github.com/web-infra-dev/rslint/internal/rules/no_sparse_arrays"
125125
"github.com/web-infra-dev/rslint/internal/rules/no_template_curly_in_string"
126+
"github.com/web-infra-dev/rslint/internal/rules/no_useless_catch"
126127
)
127128

128129
// RslintConfig represents the top-level configuration array
@@ -422,6 +423,7 @@ func registerAllCoreEslintRules() {
422423
GlobalRuleRegistry.Register("no-loss-of-precision", no_loss_of_precision.NoLossOfPrecisionRule)
423424
GlobalRuleRegistry.Register("no-template-curly-in-string", no_template_curly_in_string.NoTemplateCurlyInString)
424425
GlobalRuleRegistry.Register("no-sparse-arrays", no_sparse_arrays.NoSparseArraysRule)
426+
GlobalRuleRegistry.Register("no-useless-catch", no_useless_catch.NoUselessCatchRule)
425427
}
426428

427429
// getAllTypeScriptEslintPluginRules returns all rules from the global registry.
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package no_useless_catch
2+
3+
import (
4+
"github.com/microsoft/typescript-go/shim/ast"
5+
"github.com/web-infra-dev/rslint/internal/rule"
6+
)
7+
8+
// https://eslint.org/docs/latest/rules/no-useless-catch
9+
var NoUselessCatchRule = rule.Rule{
10+
Name: "no-useless-catch",
11+
Run: func(ctx rule.RuleContext, options any) rule.RuleListeners {
12+
return rule.RuleListeners{
13+
ast.KindCatchClause: func(node *ast.Node) {
14+
catchClause := node.AsCatchClause()
15+
if catchClause == nil {
16+
return
17+
}
18+
19+
// The catch must have a parameter
20+
if catchClause.VariableDeclaration == nil {
21+
return
22+
}
23+
24+
// The parameter must be a simple identifier (not destructured)
25+
varDecl := catchClause.VariableDeclaration.AsVariableDeclaration()
26+
if varDecl == nil || varDecl.Name() == nil {
27+
return
28+
}
29+
paramName := varDecl.Name()
30+
if paramName.Kind != ast.KindIdentifier {
31+
return
32+
}
33+
catchParamText := paramName.AsIdentifier().Text
34+
35+
// The body must have exactly one statement
36+
if catchClause.Block == nil {
37+
return
38+
}
39+
block := catchClause.Block.AsBlock()
40+
if block == nil || block.Statements == nil || len(block.Statements.Nodes) != 1 {
41+
return
42+
}
43+
44+
// That statement must be a ThrowStatement
45+
stmt := block.Statements.Nodes[0]
46+
if stmt == nil || stmt.Kind != ast.KindThrowStatement {
47+
return
48+
}
49+
50+
// The throw argument must be an Identifier
51+
throwStmt := stmt.AsThrowStatement()
52+
if throwStmt == nil || throwStmt.Expression == nil {
53+
return
54+
}
55+
if throwStmt.Expression.Kind != ast.KindIdentifier {
56+
return
57+
}
58+
59+
// The thrown identifier must match the catch parameter name
60+
thrownText := throwStmt.Expression.AsIdentifier().Text
61+
if thrownText != catchParamText {
62+
return
63+
}
64+
65+
// Determine whether the parent TryStatement has a finally block
66+
tryStmt := node.Parent
67+
if tryStmt == nil || tryStmt.Kind != ast.KindTryStatement {
68+
return
69+
}
70+
tryData := tryStmt.AsTryStatement()
71+
if tryData == nil {
72+
return
73+
}
74+
75+
if tryData.FinallyBlock != nil {
76+
// Has finally: report on the catch clause
77+
ctx.ReportNode(node, rule.RuleMessage{
78+
Id: "unnecessaryCatchClause",
79+
Description: "Unnecessary catch clause.",
80+
})
81+
} else {
82+
// No finally: report on the try statement
83+
ctx.ReportNode(tryStmt, rule.RuleMessage{
84+
Id: "unnecessaryCatch",
85+
Description: "Unnecessary try/catch wrapper.",
86+
})
87+
}
88+
},
89+
}
90+
},
91+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# no-useless-catch
2+
3+
## Rule Details
4+
5+
Disallows catch clauses that only rethrow the caught error. 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 is better to disallow them.
6+
7+
Examples of **incorrect** code for this rule:
8+
9+
```javascript
10+
try {
11+
doSomethingThatMightThrow();
12+
} catch (e) {
13+
throw e;
14+
}
15+
16+
try {
17+
doSomethingThatMightThrow();
18+
} catch (e) {
19+
throw e;
20+
} finally {
21+
cleanUp();
22+
}
23+
```
24+
25+
Examples of **correct** code for this rule:
26+
27+
```javascript
28+
try {
29+
doSomethingThatMightThrow();
30+
} catch (e) {
31+
doSomethingBeforeRethrow();
32+
throw e;
33+
}
34+
35+
try {
36+
doSomethingThatMightThrow();
37+
} catch (e) {
38+
handleError(e);
39+
}
40+
41+
try {
42+
doSomethingThatMightThrow();
43+
} catch ({ message }) {
44+
throw message;
45+
}
46+
```
47+
48+
## Original Documentation
49+
50+
- [ESLint no-useless-catch](https://eslint.org/docs/latest/rules/no-useless-catch)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package no_useless_catch
2+
3+
import (
4+
"testing"
5+
6+
"github.com/web-infra-dev/rslint/internal/plugins/typescript/rules/fixtures"
7+
"github.com/web-infra-dev/rslint/internal/rule_tester"
8+
)
9+
10+
func TestNoUselessCatchRule(t *testing.T) {
11+
rule_tester.RunRuleTester(
12+
fixtures.GetRootDir(),
13+
"tsconfig.json",
14+
t,
15+
&NoUselessCatchRule,
16+
// Valid cases - ported from ESLint
17+
[]rule_tester.ValidTestCase{
18+
{Code: `try { foo(); } catch (err) { console.error(err); }`},
19+
{Code: `try { foo(); } catch (err) { console.error(err); } finally { bar(); }`},
20+
{Code: `try { foo(); } catch (err) { doSomethingBeforeRethrow(); throw err; }`},
21+
{Code: `try { foo(); } catch (err) { throw err.msg; }`},
22+
{Code: `try { foo(); } catch (err) { throw new Error("whoops!"); }`},
23+
{Code: `try { foo(); } catch (err) { throw bar; }`},
24+
{Code: `try { foo(); } catch (err) { }`},
25+
{Code: `try { foo(); } catch ({ err }) { throw err; }`},
26+
{Code: `try { foo(); } catch ([ err ]) { throw err; }`},
27+
},
28+
// Invalid cases - ported from ESLint
29+
[]rule_tester.InvalidTestCase{
30+
{
31+
Code: `try { foo(); } catch (err) { throw err; }`,
32+
Errors: []rule_tester.InvalidTestCaseError{
33+
{MessageId: "unnecessaryCatch", Line: 1, Column: 1},
34+
},
35+
},
36+
{
37+
Code: `try { foo(); } catch (err) { throw err; } finally { foo(); }`,
38+
Errors: []rule_tester.InvalidTestCaseError{
39+
{MessageId: "unnecessaryCatchClause", Line: 1, Column: 16},
40+
},
41+
},
42+
{
43+
Code: `try { foo(); } catch (err) { /* comment */ throw err; }`,
44+
Errors: []rule_tester.InvalidTestCaseError{
45+
{MessageId: "unnecessaryCatch", Line: 1, Column: 1},
46+
},
47+
},
48+
},
49+
)
50+
}

packages/rslint-test-tools/rstest.config.mts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export default defineConfig({
2020
'./tests/eslint/rules/no-empty-pattern.test.ts',
2121
'./tests/eslint/rules/getter-return.test.ts',
2222
'./tests/eslint/rules/no-loss-of-precision.test.ts',
23+
'./tests/eslint/rules/no-useless-catch.test.ts',
2324

2425
// eslint-plugin-import
2526
'./tests/eslint-plugin-import/rules/no-self-import.test.ts',
@@ -110,6 +111,7 @@ export default defineConfig({
110111
'./tests/typescript-eslint/rules/no-invalid-void-type.test.ts',
111112
// './tests/typescript-eslint/rules/no-loop-func.test.ts',
112113
// './tests/typescript-eslint/rules/no-loss-of-precision.test.ts',
114+
'./tests/eslint/rules/no-useless-catch.test.ts',
113115
// './tests/typescript-eslint/rules/no-magic-numbers.test.ts',
114116
// './tests/typescript-eslint/rules/no-meaningless-void-operator.test.ts',
115117
'./tests/typescript-eslint/rules/no-misused-new.test.ts',
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Rstest Snapshot v1
2+
3+
exports[`no-useless-catch > invalid 1`] = `
4+
{
5+
"code": "try { foo(); } catch (err) { throw err; }",
6+
"diagnostics": [
7+
{
8+
"message": "Unnecessary try/catch wrapper.",
9+
"messageId": "unnecessaryCatch",
10+
"range": {
11+
"end": {
12+
"column": 42,
13+
"line": 1,
14+
},
15+
"start": {
16+
"column": 1,
17+
"line": 1,
18+
},
19+
},
20+
"ruleName": "no-useless-catch",
21+
},
22+
],
23+
"errorCount": 1,
24+
"fileCount": 1,
25+
"ruleCount": 1,
26+
}
27+
`;
28+
29+
exports[`no-useless-catch > invalid 2`] = `
30+
{
31+
"code": "try { foo(); } catch (err) { throw err; } finally { foo(); }",
32+
"diagnostics": [
33+
{
34+
"message": "Unnecessary catch clause.",
35+
"messageId": "unnecessaryCatchClause",
36+
"range": {
37+
"end": {
38+
"column": 42,
39+
"line": 1,
40+
},
41+
"start": {
42+
"column": 16,
43+
"line": 1,
44+
},
45+
},
46+
"ruleName": "no-useless-catch",
47+
},
48+
],
49+
"errorCount": 1,
50+
"fileCount": 1,
51+
"ruleCount": 1,
52+
}
53+
`;
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { RuleTester } from '../rule-tester';
2+
3+
const ruleTester = new RuleTester();
4+
5+
ruleTester.run('no-useless-catch', {
6+
valid: [
7+
'try { foo(); } catch (err) { console.error(err); }',
8+
'try { foo(); } catch (err) { throw bar; }',
9+
'try { foo(); } catch (err) { }',
10+
],
11+
invalid: [
12+
{
13+
code: 'try { foo(); } catch (err) { throw err; }',
14+
errors: [{ messageId: 'unnecessaryCatch' }],
15+
},
16+
{
17+
code: 'try { foo(); } catch (err) { throw err; } finally { foo(); }',
18+
errors: [{ messageId: 'unnecessaryCatchClause' }],
19+
},
20+
],
21+
});

0 commit comments

Comments
 (0)