Skip to content

Commit eec9044

Browse files
fansenzefansenze
committed
feat: port rule no-unsafe-finally
1 parent a0a2ef5 commit eec9044

File tree

7 files changed

+450
-0
lines changed

7 files changed

+450
-0
lines changed

‎internal/config/config.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ import (
125125
"github.com/web-infra-dev/rslint/internal/rules/no_loss_of_precision"
126126
"github.com/web-infra-dev/rslint/internal/rules/no_sparse_arrays"
127127
"github.com/web-infra-dev/rslint/internal/rules/no_template_curly_in_string"
128+
"github.com/web-infra-dev/rslint/internal/rules/no_unsafe_finally"
128129
)
129130

130131
// RslintConfig represents the top-level configuration array
@@ -425,6 +426,7 @@ func registerAllCoreEslintRules() {
425426
GlobalRuleRegistry.Register("no-loss-of-precision", no_loss_of_precision.NoLossOfPrecisionRule)
426427
GlobalRuleRegistry.Register("no-template-curly-in-string", no_template_curly_in_string.NoTemplateCurlyInString)
427428
GlobalRuleRegistry.Register("no-sparse-arrays", no_sparse_arrays.NoSparseArraysRule)
429+
GlobalRuleRegistry.Register("no-unsafe-finally", no_unsafe_finally.NoUnsafeFinallyRule)
428430
}
429431

430432
// getAllTypeScriptEslintPluginRules returns all rules from the global registry.

‎internal/rules/no_unsafe_finally/no_unsafe_finally.go‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package no_unsafe_finally
2+
3+
import (
4+
"github.com/microsoft/typescript-go/shim/ast"
5+
"github.com/web-infra-dev/rslint/internal/rule"
6+
)
7+
8+
// buildUnsafeUsageMessage creates the diagnostic message for unsafe finally usage
9+
func buildUnsafeUsageMessage(nodeType string) rule.RuleMessage {
10+
return rule.RuleMessage{
11+
Id: "unsafeUsage",
12+
Description: "Unsafe usage of " + nodeType + ".",
13+
}
14+
}
15+
16+
// isFunctionBoundary returns true for nodes that are function boundaries
17+
// (these stop return/throw from being flagged, and also stop break/continue)
18+
func isFunctionBoundary(kind ast.Kind) bool {
19+
switch kind {
20+
case ast.KindFunctionDeclaration,
21+
ast.KindFunctionExpression,
22+
ast.KindArrowFunction,
23+
ast.KindMethodDeclaration,
24+
ast.KindGetAccessor,
25+
ast.KindSetAccessor,
26+
ast.KindConstructor:
27+
return true
28+
}
29+
return false
30+
}
31+
32+
// isClassBoundary returns true for class declaration/expression nodes
33+
func isClassBoundary(kind ast.Kind) bool {
34+
return kind == ast.KindClassDeclaration || kind == ast.KindClassExpression
35+
}
36+
37+
// isLoopNode returns true for loop statement nodes
38+
func isLoopNode(kind ast.Kind) bool {
39+
switch kind {
40+
case ast.KindForStatement,
41+
ast.KindForInStatement,
42+
ast.KindForOfStatement,
43+
ast.KindWhileStatement,
44+
ast.KindDoStatement:
45+
return true
46+
}
47+
return false
48+
}
49+
50+
// isReturnThrowSentinel checks if a node kind is a sentinel for return/throw statements.
51+
// Return and throw are only stopped by function boundaries and class boundaries.
52+
func isReturnThrowSentinel(kind ast.Kind) bool {
53+
return isFunctionBoundary(kind) || isClassBoundary(kind)
54+
}
55+
56+
// isBreakSentinel checks if a node kind is a sentinel for unlabeled break statements.
57+
// Unlabeled break is stopped by function boundaries, class boundaries, loops, and switch.
58+
func isBreakSentinel(kind ast.Kind) bool {
59+
return isFunctionBoundary(kind) || isClassBoundary(kind) || isLoopNode(kind) || kind == ast.KindSwitchStatement
60+
}
61+
62+
// isContinueSentinel checks if a node kind is a sentinel for unlabeled continue statements.
63+
// Unlabeled continue is stopped by function boundaries, class boundaries, and loops (NOT switch).
64+
func isContinueSentinel(kind ast.Kind) bool {
65+
return isFunctionBoundary(kind) || isClassBoundary(kind) || isLoopNode(kind)
66+
}
67+
68+
// isInFinally walks up the parent chain and checks if the node is inside a finally block
69+
// of a try statement. If a sentinel node is encountered first, the node is considered safe.
70+
// For labeled break/continue, the label target must be outside the finally block for it to be unsafe.
71+
func isInFinally(node *ast.Node, isSentinel func(ast.Kind) bool, label *ast.Node) bool {
72+
current := node.Parent
73+
for current != nil {
74+
// Check if this is a sentinel node that makes the statement safe
75+
if isSentinel(current.Kind) {
76+
return false
77+
}
78+
79+
// For labeled break/continue: if we find the matching label inside the
80+
// finally block, the break/continue is safe (targets something inside finally)
81+
if label != nil && current.Kind == ast.KindLabeledStatement {
82+
labeledStmt := current.AsLabeledStatement()
83+
if labeledStmt.Label != nil && labeledStmt.Label.Text() == label.Text() {
84+
// The label is between the statement and any finally block,
85+
// so the break/continue targets something inside the finally block
86+
return false
87+
}
88+
}
89+
90+
// Check if we're in a finally block of a try statement
91+
if current.Parent != nil && current.Parent.Kind == ast.KindTryStatement {
92+
tryStmt := current.Parent.AsTryStatement()
93+
if tryStmt.FinallyBlock != nil && tryStmt.FinallyBlock == current {
94+
return true
95+
}
96+
}
97+
98+
current = current.Parent
99+
}
100+
return false
101+
}
102+
103+
// NoUnsafeFinallyRule disallows control flow statements in finally blocks
104+
var NoUnsafeFinallyRule = rule.Rule{
105+
Name: "no-unsafe-finally",
106+
Run: func(ctx rule.RuleContext, options any) rule.RuleListeners {
107+
return rule.RuleListeners{
108+
ast.KindReturnStatement: func(node *ast.Node) {
109+
if isInFinally(node, isReturnThrowSentinel, nil) {
110+
ctx.ReportNode(node, buildUnsafeUsageMessage("ReturnStatement"))
111+
}
112+
},
113+
ast.KindThrowStatement: func(node *ast.Node) {
114+
if isInFinally(node, isReturnThrowSentinel, nil) {
115+
ctx.ReportNode(node, buildUnsafeUsageMessage("ThrowStatement"))
116+
}
117+
},
118+
ast.KindBreakStatement: func(node *ast.Node) {
119+
breakStmt := node.AsBreakStatement()
120+
if breakStmt.Label != nil {
121+
// Labeled break: only sentinel is function/class boundary
122+
// The label check is handled inside isInFinally
123+
if isInFinally(node, isReturnThrowSentinel, breakStmt.Label) {
124+
ctx.ReportNode(node, buildUnsafeUsageMessage("BreakStatement"))
125+
}
126+
} else {
127+
// Unlabeled break: loops and switch are sentinels
128+
if isInFinally(node, isBreakSentinel, nil) {
129+
ctx.ReportNode(node, buildUnsafeUsageMessage("BreakStatement"))
130+
}
131+
}
132+
},
133+
ast.KindContinueStatement: func(node *ast.Node) {
134+
continueStmt := node.AsContinueStatement()
135+
if continueStmt.Label != nil {
136+
// Labeled continue: only sentinel is function/class boundary
137+
// The label check is handled inside isInFinally
138+
if isInFinally(node, isReturnThrowSentinel, continueStmt.Label) {
139+
ctx.ReportNode(node, buildUnsafeUsageMessage("ContinueStatement"))
140+
}
141+
} else {
142+
// Unlabeled continue: loops are sentinels (NOT switch)
143+
if isInFinally(node, isContinueSentinel, nil) {
144+
ctx.ReportNode(node, buildUnsafeUsageMessage("ContinueStatement"))
145+
}
146+
}
147+
},
148+
}
149+
},
150+
}

‎internal/rules/no_unsafe_finally/no_unsafe_finally.md‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# no-unsafe-finally
2+
3+
## Rule Details
4+
5+
Disallows control flow statements (`return`, `throw`, `break`, `continue`) inside `finally` blocks. When control flow statements are used inside `finally` blocks, they override the control flow of `try` and `catch` blocks, which can lead to unexpected behavior and make code harder to understand.
6+
7+
Examples of **incorrect** code for this rule:
8+
9+
```javascript
10+
function foo() {
11+
try {
12+
return 1;
13+
} catch (err) {
14+
return 2;
15+
} finally {
16+
return 3; // overrides the return in try/catch
17+
}
18+
}
19+
20+
function bar() {
21+
try {
22+
doSomething();
23+
} finally {
24+
throw new Error(); // overrides any error thrown in try
25+
}
26+
}
27+
28+
label: try {
29+
return 0;
30+
} finally {
31+
break label; // overrides the return in try
32+
}
33+
```
34+
35+
Examples of **correct** code for this rule:
36+
37+
```javascript
38+
function foo() {
39+
try {
40+
return 1;
41+
} catch (err) {
42+
return 2;
43+
} finally {
44+
console.log('done');
45+
}
46+
}
47+
48+
function bar() {
49+
try {
50+
doSomething();
51+
} finally {
52+
// control flow inside nested functions is fine
53+
function cleanup(x) {
54+
return x;
55+
}
56+
cleanup();
57+
}
58+
}
59+
60+
function baz() {
61+
try {
62+
doSomething();
63+
} finally {
64+
// break/continue inside loops within finally is fine
65+
while (condition) {
66+
break;
67+
}
68+
}
69+
}
70+
```
71+
72+
## Original Documentation
73+
74+
- [ESLint no-unsafe-finally](https://eslint.org/docs/latest/rules/no-unsafe-finally)

‎internal/rules/no_unsafe_finally/no_unsafe_finally_test.go‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package no_unsafe_finally
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 TestNoUnsafeFinallyRule(t *testing.T) {
11+
rule_tester.RunRuleTester(
12+
fixtures.GetRootDir(),
13+
"tsconfig.json",
14+
t,
15+
&NoUnsafeFinallyRule,
16+
// Valid cases
17+
[]rule_tester.ValidTestCase{
18+
// No control flow in finally
19+
{Code: `var foo = function() { try { return 1; } catch(err) { return 2; } finally { console.log("hola!") } }`},
20+
21+
// Return inside a nested function in finally is safe
22+
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { function a(x) { return x } } }`},
23+
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { var a = function(x) { return x } } }`},
24+
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { var a = (x) => { return x } } }`},
25+
26+
// Break inside a loop in finally is safe
27+
{Code: `var foo = function() { try {} finally { while (true) break; } }`},
28+
{Code: `var foo = function() { try {} finally { for (var i = 0; i < 10; i++) break; } }`},
29+
{Code: `var foo = function() { try {} finally { for (var x in obj) break; } }`},
30+
{Code: `var foo = function() { try {} finally { for (var x of arr) break; } }`},
31+
{Code: `var foo = function() { try {} finally { do { break; } while (true); } }`},
32+
33+
// Continue inside a loop in finally is safe
34+
{Code: `var foo = function() { try {} finally { while (true) continue; } }`},
35+
{Code: `var foo = function() { try {} finally { for (var i = 0; i < 10; i++) continue; } }`},
36+
37+
// Break inside switch in finally is safe
38+
{Code: `var foo = function() { try {} finally { switch (true) { case true: break; } } }`},
39+
40+
// Labeled break/continue where the label is inside finally is safe
41+
{Code: `var foo = function() { try {} finally { label: while (true) { break label; } } }`},
42+
{Code: `var foo = function() { try {} finally { label: while (true) { continue label; } } }`},
43+
{Code: `var foo = function() { try {} finally { label: for (var i = 0; i < 10; i++) { break label; } } }`},
44+
45+
// Throw inside nested function in finally is safe
46+
{Code: `var foo = function() { try {} finally { function a() { throw new Error() } } }`},
47+
48+
// Class boundary stops propagation
49+
{Code: `var foo = function() { try {} finally { class C { method() { return 1 } } } }`},
50+
51+
// No finally block at all
52+
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } }`},
53+
54+
// Control flow in try/catch but not finally
55+
{Code: `var foo = function() { try { throw new Error() } catch(err) { return 2 } finally { console.log("done") } }`},
56+
57+
// Return in getter/setter inside finally is safe
58+
{Code: `var foo = function() { try {} finally { var obj = { get x() { return 1 } } } }`},
59+
{Code: `var foo = function() { try {} finally { var obj = { set x(v) { return } } } }`},
60+
},
61+
// Invalid cases
62+
[]rule_tester.InvalidTestCase{
63+
// Return in finally
64+
{
65+
Code: `var foo = function() { try { return 1; } catch(err) { return 2; } finally { return 3; } }`,
66+
Errors: []rule_tester.InvalidTestCaseError{
67+
{MessageId: "unsafeUsage", Line: 1, Column: 77},
68+
},
69+
},
70+
// Return in finally (no catch)
71+
{
72+
Code: `var foo = function() { try { return 1; } finally { return 3; } }`,
73+
Errors: []rule_tester.InvalidTestCaseError{
74+
{MessageId: "unsafeUsage", Line: 1, Column: 52},
75+
},
76+
},
77+
// Throw in finally
78+
{
79+
Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { throw new Error() } }`,
80+
Errors: []rule_tester.InvalidTestCaseError{
81+
{MessageId: "unsafeUsage", Line: 1, Column: 75},
82+
},
83+
},
84+
// Break with label targeting outside finally
85+
{
86+
Code: `label: try { return 0; } finally { break label; }`,
87+
Errors: []rule_tester.InvalidTestCaseError{
88+
{MessageId: "unsafeUsage", Line: 1, Column: 36},
89+
},
90+
},
91+
// Unlabeled break exits loop outside finally
92+
{
93+
Code: `while (true) try {} finally { break; }`,
94+
Errors: []rule_tester.InvalidTestCaseError{
95+
{MessageId: "unsafeUsage", Line: 1, Column: 31},
96+
},
97+
},
98+
// Unlabeled continue exits loop outside finally
99+
{
100+
Code: `while (true) try {} finally { continue; }`,
101+
Errors: []rule_tester.InvalidTestCaseError{
102+
{MessageId: "unsafeUsage", Line: 1, Column: 31},
103+
},
104+
},
105+
// Continue with label targeting outside finally
106+
{
107+
Code: `label: while (true) try {} finally { continue label; }`,
108+
Errors: []rule_tester.InvalidTestCaseError{
109+
{MessageId: "unsafeUsage", Line: 1, Column: 38},
110+
},
111+
},
112+
// Return in nested try-finally (inner finally)
113+
{
114+
Code: `var foo = function() { try {} finally { try {} finally { return 1; } } }`,
115+
Errors: []rule_tester.InvalidTestCaseError{
116+
{MessageId: "unsafeUsage", Line: 1, Column: 58},
117+
},
118+
},
119+
// Throw in finally (simple)
120+
{
121+
Code: `var foo = function() { try {} finally { throw new Error() } }`,
122+
Errors: []rule_tester.InvalidTestCaseError{
123+
{MessageId: "unsafeUsage", Line: 1, Column: 41},
124+
},
125+
},
126+
// Break in finally (labeled, label outside)
127+
{
128+
Code: `label: try {} finally { break label; }`,
129+
Errors: []rule_tester.InvalidTestCaseError{
130+
{MessageId: "unsafeUsage", Line: 1, Column: 25},
131+
},
132+
},
133+
// Continue in finally exits switch and loop outside
134+
{
135+
Code: `while (true) try {} finally { switch (true) { case true: continue; } }`,
136+
Errors: []rule_tester.InvalidTestCaseError{
137+
{MessageId: "unsafeUsage", Line: 1, Column: 58},
138+
},
139+
},
140+
// Multiple unsafe statements in finally
141+
{
142+
Code: `var foo = function() { try {} finally { return 1; return 2; } }`,
143+
Errors: []rule_tester.InvalidTestCaseError{
144+
{MessageId: "unsafeUsage", Line: 1, Column: 41},
145+
{MessageId: "unsafeUsage", Line: 1, Column: 51},
146+
},
147+
},
148+
},
149+
)
150+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,5 +196,6 @@ export default defineConfig({
196196
// './tests/typescript-eslint/rules/unbound-method.test.ts',
197197
// './tests/typescript-eslint/rules/unified-signatures.test.ts',
198198
// './tests/typescript-eslint/rules/use-unknown-in-catch-callback-variable.test.ts',
199+
'./tests/eslint/rules/no-unsafe-finally.test.ts',
199200
],
200201
});

0 commit comments

Comments
 (0)