fix(noUnreachable): handle dead implicit jumps in finally by ematipico · Pull Request #9305 · biomejs/biome

ematipico · 2026-03-02T21:57:09Z

Summary

Closes #4946

It took a while for the AI agent to get the fix right.

Test Plan

Added a test

Docs

changeset-bot · 2026-03-02T21:57:14Z

🦋 Changeset detected

Latest commit: 8c3ef40

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@biomejs/biome	Patch
@biomejs/cli-win32-x64	Patch
@biomejs/cli-win32-arm64	Patch
@biomejs/cli-darwin-x64	Patch
@biomejs/cli-darwin-arm64	Patch
@biomejs/cli-linux-x64	Patch
@biomejs/cli-linux-arm64	Patch
@biomejs/cli-linux-x64-musl	Patch
@biomejs/cli-linux-arm64-musl	Patch
@biomejs/wasm-web	Patch
@biomejs/wasm-bundler	Patch
@biomejs/wasm-nodejs	Patch
@biomejs/backend-jsonrpc	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

coderabbitai · 2026-03-02T22:04:03Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2148b2 and 8c3ef40.

⛔ Files ignored due to path filters (1)

crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js.snap is excluded by !**/*.snap and included by **

📒 Files selected for processing (2)

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs
crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js

🚧 Files skipped from review as they are similar to previous changes (1)

crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js

Walkthrough

Fixes the noUnreachable analysis so code in finally blocks is not marked unreachable when the corresponding try body contains a break, continue, or return. Introduces a Terminator abstraction and a pre_finally_terminator field in path state to track terminators active before entering finally. Updates CFG traversal and jump handling to propagate and restore terminator state across jumps and finally fallthroughs. Adds tests covering try/finally reachability and a changeset for a patch release.

Suggested reviewers

dyc3

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarises the main fix: handling dead implicit jumps in finally blocks within the noUnreachable rule.
Description check	✅ Passed	The description references the linked issue `#4946` and explains the fix was for handling jumps in finally blocks, directly relating to the changeset.
Linked Issues check	✅ Passed	The PR comprehensively addresses issue `#4946` by refactoring control-flow analysis to correctly handle finally block reachability when jumps precede them, including test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly in scope: the rule logic fix, test expansion, and changeset documentation all address the linked issue without extraneous modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/no-unreacheable-finally

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js (1)

62-93: Consider adding one nested try/finally jump case.

A nested cleanup-chain test would harden this against future regressions in terminator restoration.

Possible test extension

+function JsTryFinallyStatement8() {
+    while (true) {
+        try {
+            try {
+                break;
+            } finally {
+                console.log("inner finally reachable");
+            }
+        } finally {
+            console.log("outer finally reachable");
+        }
+        console.log("unreachable");
+    }
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js`
around lines 62 - 93, Add a nested try/finally test to ensure cleanup chain
restoration: create a new test function (e.g., JsTryFinallyStatement8) that has
an outer try { try { /*jump: break/continue/return*/ } finally {
console.log("inner reachable"); } } finally { console.log("outer reachable"); }
followed by an unreachable console.log to assert the jump does not skip either
finally; use the same pattern as JsTryFinallyStatement5–7 but nest an inner
try/finally that performs the jump so both inner and outer finally blocks are
executed.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js`:
- Around line 62-93: Add a nested try/finally test to ensure cleanup chain
restoration: create a new test function (e.g., JsTryFinallyStatement8) that has
an outer try { try { /*jump: break/continue/return*/ } finally {
console.log("inner reachable"); } } finally { console.log("outer reachable"); }
followed by an unreachable console.log to assert the jump does not skip either
finally; use the same pattern as JsTryFinallyStatement5–7 but nest an inner
try/finally that performs the jump so both inner and outer finally blocks are
executed.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1003229 and f4db0b3.

⛔ Files ignored due to path filters (1)

crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js.snap is excluded by !**/*.snap and included by **

📒 Files selected for processing (3)

.changeset/fix-no-unreachable-try-finally.md
crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs
crates/biome_js_analyze/tests/specs/correctness/noUnreachable/JsTryFinallyStatement.js

codspeed-hq · 2026-03-02T22:11:25Z

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks¹

_{Comparing fix/no-unreacheable-finally (8c3ef40) with main (1003229)}

156 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

dyc3

would you mind including which models were used for AI prs? its no big deal but I'm trying to gauge how different models perform for what tasks

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs

ematipico · 2026-03-02T22:51:56Z

I've used 4.5 sonnet until last week. I recently switched to 4.6 sonnet

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs`:
- Around line 637-643: When handling finally_fallthrough, prefer the actual
terminator produced by the finally block over the saved pre_finally_terminator
so we don't drop a newer cause; change the continuation_terminator selection
(currently let continuation_terminator =
path.pre_finally_terminator.unwrap_or(path.terminator)) to prefer
path.terminator when it represents a terminating outcome (e.g., return/throw)
and only fall back to path.pre_finally_terminator when path.terminator is
non-terminating/empty—i.e., choose the active finally terminator
(path.terminator) if it indicates termination, otherwise use
path.pre_finally_terminator.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4db0b3 and f2148b2.

📒 Files selected for processing (1)

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs

fix(noUnreachable): handle dead implicit jumps in finally

f4db0b3

ematipico requested review from a team March 2, 2026 21:57

github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 2, 2026

coderabbitai bot reviewed Mar 2, 2026

View reviewed changes

dyc3 reviewed Mar 2, 2026

View reviewed changes

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs Outdated Show resolved Hide resolved

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs Outdated Show resolved Hide resolved

add type alias

f2148b2

coderabbitai bot reviewed Mar 3, 2026

View reviewed changes

crates/biome_js_analyze/src/lint/correctness/no_unreachable.rs Show resolved Hide resolved

address comment

8c3ef40

ematipico merged commit 40869b5 into main Mar 3, 2026
18 checks passed

ematipico deleted the fix/no-unreacheable-finally branch March 3, 2026 16:36

github-actions bot mentioned this pull request Mar 3, 2026

ci: release #9301

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(noUnreachable): handle dead implicit jumps in finally#9305

fix(noUnreachable): handle dead implicit jumps in finally#9305
ematipico merged 3 commits intomainfrom
fix/no-unreacheable-finally

ematipico commented Mar 2, 2026

Uh oh!

changeset-bot bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

codspeed-hq bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

dyc3 left a comment

Uh oh!

Uh oh!

Uh oh!

ematipico commented Mar 2, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ematipico commented Mar 2, 2026

Summary

Test Plan

Docs

Uh oh!

changeset-bot bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coderabbitai bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

dyc3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ematipico commented Mar 2, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Mar 2, 2026 •

edited

Loading

coderabbitai bot commented Mar 2, 2026 •

edited

Loading

codspeed-hq bot commented Mar 2, 2026 •

edited

Loading