Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ import (
"github.com/web-infra-dev/rslint/internal/rules/no_loss_of_precision"
"github.com/web-infra-dev/rslint/internal/rules/no_sparse_arrays"
"github.com/web-infra-dev/rslint/internal/rules/no_template_curly_in_string"
"github.com/web-infra-dev/rslint/internal/rules/no_unsafe_finally"
)

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

// getAllTypeScriptEslintPluginRules returns all rules from the global registry.
Expand Down
150 changes: 150 additions & 0 deletions internal/rules/no_unsafe_finally/no_unsafe_finally.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package no_unsafe_finally

import (
"github.com/microsoft/typescript-go/shim/ast"
"github.com/web-infra-dev/rslint/internal/rule"
)

// buildUnsafeUsageMessage creates the diagnostic message for unsafe finally usage
func buildUnsafeUsageMessage(nodeType string) rule.RuleMessage {
return rule.RuleMessage{
Id: "unsafeUsage",
Description: "Unsafe usage of " + nodeType + ".",
}
}

// isFunctionBoundary returns true for nodes that are function boundaries
// (these stop return/throw from being flagged, and also stop break/continue)
func isFunctionBoundary(kind ast.Kind) bool {
switch kind {
case ast.KindFunctionDeclaration,
ast.KindFunctionExpression,
ast.KindArrowFunction,
ast.KindMethodDeclaration,
ast.KindGetAccessor,
ast.KindSetAccessor,
ast.KindConstructor:
return true
}
return false
}

// isClassBoundary returns true for class declaration/expression nodes
func isClassBoundary(kind ast.Kind) bool {
return kind == ast.KindClassDeclaration || kind == ast.KindClassExpression
}

// isLoopNode returns true for loop statement nodes
func isLoopNode(kind ast.Kind) bool {
switch kind {
case ast.KindForStatement,
ast.KindForInStatement,
ast.KindForOfStatement,
ast.KindWhileStatement,
ast.KindDoStatement:
return true
}
return false
}

// isReturnThrowSentinel checks if a node kind is a sentinel for return/throw statements.
// Return and throw are only stopped by function boundaries and class boundaries.
func isReturnThrowSentinel(kind ast.Kind) bool {
return isFunctionBoundary(kind) || isClassBoundary(kind)
}

// isBreakSentinel checks if a node kind is a sentinel for unlabeled break statements.
// Unlabeled break is stopped by function boundaries, class boundaries, loops, and switch.
func isBreakSentinel(kind ast.Kind) bool {
return isFunctionBoundary(kind) || isClassBoundary(kind) || isLoopNode(kind) || kind == ast.KindSwitchStatement
}

// isContinueSentinel checks if a node kind is a sentinel for unlabeled continue statements.
// Unlabeled continue is stopped by function boundaries, class boundaries, and loops (NOT switch).
func isContinueSentinel(kind ast.Kind) bool {
return isFunctionBoundary(kind) || isClassBoundary(kind) || isLoopNode(kind)
}

// isInFinally walks up the parent chain and checks if the node is inside a finally block
// of a try statement. If a sentinel node is encountered first, the node is considered safe.
// For labeled break/continue, the label target must be outside the finally block for it to be unsafe.
func isInFinally(node *ast.Node, isSentinel func(ast.Kind) bool, label *ast.Node) bool {
current := node.Parent
for current != nil {
// Check if this is a sentinel node that makes the statement safe
if isSentinel(current.Kind) {
return false
}

// For labeled break/continue: if we find the matching label inside the
// finally block, the break/continue is safe (targets something inside finally)
if label != nil && current.Kind == ast.KindLabeledStatement {
labeledStmt := current.AsLabeledStatement()
if labeledStmt.Label != nil && labeledStmt.Label.Text() == label.Text() {
// The label is between the statement and any finally block,
// so the break/continue targets something inside the finally block
return false
}
}

// Check if we're in a finally block of a try statement
if current.Parent != nil && current.Parent.Kind == ast.KindTryStatement {
tryStmt := current.Parent.AsTryStatement()
if tryStmt.FinallyBlock != nil && tryStmt.FinallyBlock == current {
return true
}
}

current = current.Parent
}
return false
}

// NoUnsafeFinallyRule disallows control flow statements in finally blocks
var NoUnsafeFinallyRule = rule.Rule{
Name: "no-unsafe-finally",
Run: func(ctx rule.RuleContext, options any) rule.RuleListeners {
return rule.RuleListeners{
ast.KindReturnStatement: func(node *ast.Node) {
if isInFinally(node, isReturnThrowSentinel, nil) {
ctx.ReportNode(node, buildUnsafeUsageMessage("ReturnStatement"))
}
},
ast.KindThrowStatement: func(node *ast.Node) {
if isInFinally(node, isReturnThrowSentinel, nil) {
ctx.ReportNode(node, buildUnsafeUsageMessage("ThrowStatement"))
}
},
ast.KindBreakStatement: func(node *ast.Node) {
breakStmt := node.AsBreakStatement()
if breakStmt.Label != nil {
// Labeled break: only sentinel is function/class boundary
// The label check is handled inside isInFinally
if isInFinally(node, isReturnThrowSentinel, breakStmt.Label) {
ctx.ReportNode(node, buildUnsafeUsageMessage("BreakStatement"))
}
} else {
// Unlabeled break: loops and switch are sentinels
if isInFinally(node, isBreakSentinel, nil) {
ctx.ReportNode(node, buildUnsafeUsageMessage("BreakStatement"))
}
}
},
ast.KindContinueStatement: func(node *ast.Node) {
continueStmt := node.AsContinueStatement()
if continueStmt.Label != nil {
// Labeled continue: only sentinel is function/class boundary
// The label check is handled inside isInFinally
if isInFinally(node, isReturnThrowSentinel, continueStmt.Label) {
ctx.ReportNode(node, buildUnsafeUsageMessage("ContinueStatement"))
}
} else {
// Unlabeled continue: loops are sentinels (NOT switch)
if isInFinally(node, isContinueSentinel, nil) {
ctx.ReportNode(node, buildUnsafeUsageMessage("ContinueStatement"))
}
}
},
}
},
}
74 changes: 74 additions & 0 deletions internal/rules/no_unsafe_finally/no_unsafe_finally.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# no-unsafe-finally

## Rule Details

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.

Examples of **incorrect** code for this rule:

```javascript
function foo() {
try {
return 1;
} catch (err) {
return 2;
} finally {
return 3; // overrides the return in try/catch
}
}

function bar() {
try {
doSomething();
} finally {
throw new Error(); // overrides any error thrown in try
}
}

label: try {
return 0;
} finally {
break label; // overrides the return in try
}
```

Examples of **correct** code for this rule:

```javascript
function foo() {
try {
return 1;
} catch (err) {
return 2;
} finally {
console.log('done');
}
}

function bar() {
try {
doSomething();
} finally {
// control flow inside nested functions is fine
function cleanup(x) {
return x;
}
cleanup();
}
}

function baz() {
try {
doSomething();
} finally {
// break/continue inside loops within finally is fine
while (condition) {
break;
}
}
}
```

## Original Documentation

- [ESLint no-unsafe-finally](https://eslint.org/docs/latest/rules/no-unsafe-finally)
150 changes: 150 additions & 0 deletions internal/rules/no_unsafe_finally/no_unsafe_finally_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package no_unsafe_finally

import (
"testing"

"github.com/web-infra-dev/rslint/internal/plugins/typescript/rules/fixtures"
"github.com/web-infra-dev/rslint/internal/rule_tester"
)

func TestNoUnsafeFinallyRule(t *testing.T) {
rule_tester.RunRuleTester(
fixtures.GetRootDir(),
"tsconfig.json",
t,
&NoUnsafeFinallyRule,
// Valid cases
[]rule_tester.ValidTestCase{
// No control flow in finally
{Code: `var foo = function() { try { return 1; } catch(err) { return 2; } finally { console.log("hola!") } }`},

// Return inside a nested function in finally is safe
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { function a(x) { return x } } }`},
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { var a = function(x) { return x } } }`},
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { var a = (x) => { return x } } }`},

// Break inside a loop in finally is safe
{Code: `var foo = function() { try {} finally { while (true) break; } }`},
{Code: `var foo = function() { try {} finally { for (var i = 0; i < 10; i++) break; } }`},
{Code: `var foo = function() { try {} finally { for (var x in obj) break; } }`},
{Code: `var foo = function() { try {} finally { for (var x of arr) break; } }`},
{Code: `var foo = function() { try {} finally { do { break; } while (true); } }`},

// Continue inside a loop in finally is safe
{Code: `var foo = function() { try {} finally { while (true) continue; } }`},
{Code: `var foo = function() { try {} finally { for (var i = 0; i < 10; i++) continue; } }`},

// Break inside switch in finally is safe
{Code: `var foo = function() { try {} finally { switch (true) { case true: break; } } }`},

// Labeled break/continue where the label is inside finally is safe
{Code: `var foo = function() { try {} finally { label: while (true) { break label; } } }`},
{Code: `var foo = function() { try {} finally { label: while (true) { continue label; } } }`},
{Code: `var foo = function() { try {} finally { label: for (var i = 0; i < 10; i++) { break label; } } }`},

// Throw inside nested function in finally is safe
{Code: `var foo = function() { try {} finally { function a() { throw new Error() } } }`},

// Class boundary stops propagation
{Code: `var foo = function() { try {} finally { class C { method() { return 1 } } } }`},

// No finally block at all
{Code: `var foo = function() { try { return 1 } catch(err) { return 2 } }`},

// Control flow in try/catch but not finally
{Code: `var foo = function() { try { throw new Error() } catch(err) { return 2 } finally { console.log("done") } }`},

// Return in getter/setter inside finally is safe
{Code: `var foo = function() { try {} finally { var obj = { get x() { return 1 } } } }`},
{Code: `var foo = function() { try {} finally { var obj = { set x(v) { return } } } }`},
},
// Invalid cases
[]rule_tester.InvalidTestCase{
// Return in finally
{
Code: `var foo = function() { try { return 1; } catch(err) { return 2; } finally { return 3; } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 77},
},
},
// Return in finally (no catch)
{
Code: `var foo = function() { try { return 1; } finally { return 3; } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 52},
},
},
// Throw in finally
{
Code: `var foo = function() { try { return 1 } catch(err) { return 2 } finally { throw new Error() } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 75},
},
},
// Break with label targeting outside finally
{
Code: `label: try { return 0; } finally { break label; }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 36},
},
},
// Unlabeled break exits loop outside finally
{
Code: `while (true) try {} finally { break; }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 31},
},
},
// Unlabeled continue exits loop outside finally
{
Code: `while (true) try {} finally { continue; }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 31},
},
},
// Continue with label targeting outside finally
{
Code: `label: while (true) try {} finally { continue label; }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 38},
},
},
// Return in nested try-finally (inner finally)
{
Code: `var foo = function() { try {} finally { try {} finally { return 1; } } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 58},
},
},
// Throw in finally (simple)
{
Code: `var foo = function() { try {} finally { throw new Error() } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 41},
},
},
// Break in finally (labeled, label outside)
{
Code: `label: try {} finally { break label; }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 25},
},
},
// Continue in finally exits switch and loop outside
{
Code: `while (true) try {} finally { switch (true) { case true: continue; } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 58},
},
},
// Multiple unsafe statements in finally
{
Code: `var foo = function() { try {} finally { return 1; return 2; } }`,
Errors: []rule_tester.InvalidTestCaseError{
{MessageId: "unsafeUsage", Line: 1, Column: 41},
{MessageId: "unsafeUsage", Line: 1, Column: 51},
},
},
},
)
}
1 change: 1 addition & 0 deletions packages/rslint-test-tools/rstest.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -196,5 +196,6 @@ export default defineConfig({
// './tests/typescript-eslint/rules/unbound-method.test.ts',
// './tests/typescript-eslint/rules/unified-signatures.test.ts',
// './tests/typescript-eslint/rules/use-unknown-in-catch-callback-variable.test.ts',
'./tests/eslint/rules/no-unsafe-finally.test.ts',
],
});
Loading
Loading