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

Author: ematipico

.changeset/fix-no-unreachable-try-finally.md

@@ -0,0 +1,5 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed [#4946](https://github.com/biomejs/biome/issues/4946): `noUnreachable` no longer reports code inside `finally` blocks as unreachable when there is a `break`, `continue`, or `return` in the corresponding `try` body.

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

@@ -422,25 +422,36 @@ struct PathState<'cfg> {
     next_block: BlockId,
     /// Set of all blocks already visited on this path
     visited: RoaringBitmap,
-    /// Current terminating instruction for the path, if one was
-    /// encountered
-    terminator: Option<Option<PathTerminator>>,
+    /// Current terminating instruction for the path, if one was encountered.
+    /// See [`Terminator`] for the semantics of each `Option` level.
+    terminator: Option<Terminator>,
     exception_handlers: Option<&'cfg [ExceptionHandler]>,
+    /// The terminator that was active before entering a `finally` block via a
+    /// dead implicit jump (i.e. when `break`/`continue`/`return`/`throw`
+    /// precedes the implicit try-body-to-finally fall-through).
+    ///
+    /// `None` means we are not currently inside such a finally block.
+    /// `Some(t)` stores the `Option<Terminator>` that was in `self.terminator`
+    /// when the finally block was entered; it is restored by `finally_fallthrough`
+    /// so that code *after* the `try/finally` construct is still correctly seen
+    /// as unreachable.
+    pre_finally_terminator: Option<Option<Terminator>>,
 }
 
 /// Perform a simple reachability analysis on the control flow graph by
 /// traversing the function starting at the entry points
 fn traverse_cfg(
     cfg: &JsControlFlowGraph,
     signals: &mut UnreachableRanges,
-) -> FxHashMap<BlockId, Vec<Option<Option<PathTerminator>>>> {
+) -> FxHashMap<BlockId, Vec<Option<Terminator>>> {
     let mut queue = VecDeque::new();
 
     queue.push_back(PathState {
         next_block: ROOT_BLOCK_ID,
         visited: RoaringBitmap::new(),
         terminator: None,
         exception_handlers: None,
+        pre_finally_terminator: None,
     });
 
     // This maps holds a list of "path state", the active terminator
@@ -480,6 +491,7 @@ fn traverse_cfg(
                         visited: path.visited.clone(),
                         terminator: path.terminator,
                         exception_handlers: find_catch_handlers(handlers),
+                        pre_finally_terminator: None,
                     });
                 }
             }
@@ -495,10 +507,34 @@ fn traverse_cfg(
                 InstructionKind::Statement => {}
                 InstructionKind::Jump {
                     conditional,
-                    block,
+                    block: jump_target,
                     finally_fallthrough,
                 } => {
-                    handle_jump(&mut queue, &path, block, finally_fallthrough);
+                    // If we are in dead code (has_direct_terminator) but the jump
+                    // target is a cleanup (finally) handler for this block, the finally
+                    // block is still reachable — it always runs before any early exit
+                    // (break/continue/return/throw). Clear the terminator so the finally
+                    // block is not incorrectly reported as unreachable. Store the
+                    // original terminator in `pre_finally_terminator` so it can be
+                    // restored when the finally block exits via `finally_fallthrough`.
+                    let (effective_terminator, pre_finally_terminator) = if has_direct_terminator
+                        && block
+                            .cleanup_handlers
+                            .iter()
+                            .any(|h| h.target == jump_target)
+                    {
+                        (None, Some(path.terminator))
+                    } else {
+                        (path.terminator, None)
+                    };
+                    handle_jump(
+                        &mut queue,
+                        &path,
+                        jump_target,
+                        finally_fallthrough,
+                        effective_terminator,
+                        pre_finally_terminator,
+                    );
 
                     // Jump is a terminator instruction if it's unconditional
                     if path.terminator.is_none() && !conditional {
@@ -565,12 +601,20 @@ fn find_catch_handlers(handlers: &[ExceptionHandler]) -> Option<&[ExceptionHandl
     }
 }
 
-/// Create an additional visitor path from a jump instruction and push it to the queue
+/// Create an additional visitor path from a jump instruction and push it to the queue.
+/// `effective_terminator` is the terminator to propagate to the target block; callers may
+/// pass `None` when the target is known to be reachable regardless of any prior terminator
+/// (e.g. an implicit jump from the end of a try body to its finally block).
+/// `pre_finally_terminator` should be set when `effective_terminator` was cleared to allow
+/// a finally block to appear reachable; it stores the original terminator so it can be
+/// restored when the finally block exits via `finally_fallthrough`.
 fn handle_jump<'cfg>(
     queue: &mut VecDeque<PathState<'cfg>>,
     path: &PathState<'cfg>,
     block: BlockId,
     finally_fallthrough: bool,
+    effective_terminator: Option<Terminator>,
+    pre_finally_terminator: Option<Option<Terminator>>,
 ) {
     // If this jump is exiting a finally clause and this path is visiting
     // an exception handlers chain
@@ -587,16 +631,45 @@ fn handle_jump<'cfg>(
                 visited: path.visited.clone(),
                 terminator: path.terminator,
                 exception_handlers: Some(handlers),
+                pre_finally_terminator: None,
+            });
+        }
+    } else if finally_fallthrough {
+        // Exiting a finally block in the normal (non-exception) context.
+        // Determine the terminator to propagate past the try/finally:
+        // - If the finally block itself terminates (e.g. contains a `return`
+        //   or `throw`), prefer that as the most proximate cause.
+        // - Otherwise, if we entered finally via a dead implicit jump (the
+        //   try body had `break`/`continue`/`return`), restore the saved
+        //   pre-finally terminator so code after the try/finally is still
+        //   correctly seen as unreachable.
+        let continuation_terminator = path
+            .terminator
+            .or(path.pre_finally_terminator.unwrap_or(None));
+
+        if !path.visited.contains(block.index()) {
+            queue.push_back(PathState {
+                next_block: block,
+                visited: path.visited.clone(),
+                terminator: continuation_terminator,
+                exception_handlers: path.exception_handlers,
+                pre_finally_terminator: None,
             });
         }
     } else if !path.visited.contains(block.index()) {
         // Push the jump target block to the queue if it hasn't
-        // been visited yet in this path
+        // been visited yet in this path.
+        // Propagate `pre_finally_terminator` from the current path so it
+        // survives across internal jumps inside a finally block (e.g. an
+        // if/else inside finally) and is available when the block exits via
+        // `finally_fallthrough`.
+        let effective_pre_finally = pre_finally_terminator.or(path.pre_finally_terminator);
         queue.push_back(PathState {
             next_block: block,
             visited: path.visited.clone(),
-            terminator: path.terminator,
+            terminator: effective_terminator,
             exception_handlers: path.exception_handlers,
+            pre_finally_terminator: effective_pre_finally,
         });
     }
 }
@@ -616,6 +689,7 @@ fn handle_return<'cfg>(
             visited: path.visited.clone(),
             terminator: path.terminator,
             exception_handlers: Some(handlers),
+            pre_finally_terminator: None,
         });
     }
 }
@@ -967,6 +1041,16 @@ pub struct UnreachableRange {
     terminators: Vec<PathTerminator>,
 }
 
+/// The terminator state of a path through the control flow graph.
+///
+/// Each level of `Option` carries distinct semantics:
+/// - Outer `Option`: `None` = the path has no terminator yet (code is still live);
+///   `Some(_)` = a terminating instruction has been encountered on this path.
+/// - Inner `Option<PathTerminator>`: `None` = the terminator has no associated
+///   syntax node (e.g. an implicit jump); `Some(pt)` = the terminator corresponds
+///   to a concrete syntax node that can be pointed to in a diagnostic.
+type Terminator = Option<PathTerminator>;
+
 #[derive(Debug, Clone, Copy)]
 struct PathTerminator {
     kind: JsSyntaxKind,

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

@@ -1,3 +1,5 @@
+// should generate diagnostics
+
 function JsTryFinallyStatement1() {
     try {
         tryBlock();
@@ -58,3 +60,63 @@ function JsTryFinallyStatement4() {
 
     afterTryStatement();
 }
+
+// https://github.com/biomejs/biome/issues/4946
+// finally block should be reachable when there is a jump (break/continue/return) before it
+function JsTryFinallyStatement5() {
+    while (true) {
+        try {
+            break;
+        } finally {
+            console.log("reachable");
+        }
+        console.log("unreachable");
+    }
+}
+
+function JsTryFinallyStatement6() {
+    while (true) {
+        try {
+            continue;
+        } finally {
+            console.log("reachable");
+        }
+        console.log("unreachable");
+    }
+}
+
+function JsTryFinallyStatement7() {
+    try {
+        return;
+    } finally {
+        console.log("reachable");
+    }
+    console.log("unreachable");
+}
+
+// finally itself contains a return: code after the try/finally is unreachable
+// due to the finally's return (not the try's return)
+function JsTryFinallyStatement8() {
+    try {
+        return 1;
+    } finally {
+        return 2;
+    }
+    console.log("unreachable");
+}
+
+// nested try/finally: both inner and outer finally blocks should be reachable
+function JsTryFinallyStatement9() {
+    while (true) {
+        try {
+            try {
+                break;
+            } finally {
+                console.log("inner finally reachable");
+            }
+        } finally {
+            console.log("outer finally reachable");
+        }
+        console.log("unreachable");
+    }
+}