fix Narrowing inside try is preserved into finally, ignoring type outside of try #2845

pyrefly/lib/binding/scope.rs

@@ -756,7 +756,8 @@ impl FlowStyle {
                         MergeStyle::Loop
                         | MergeStyle::LoopDefinitelyRuns
                         | MergeStyle::Exclusive
-                        | MergeStyle::Inclusive => FlowStyle::PossiblyUninitialized,
+                        | MergeStyle::Inclusive
+                        | MergeStyle::Finally => FlowStyle::PossiblyUninitialized,
                     }
                 }
             }
@@ -2945,6 +2946,10 @@ enum MergeStyle {
     /// Distinct from [Branching] because we have to be more lax about
     /// uninitialized locals (see `FlowStyle::merge` for details).
     BoolOp,
+    /// This is the merge immediately before analyzing a `finally` block.
+    /// Terminating branches still contribute because `finally` executes before
+    /// their control-flow effect is observed outside the statement.
+    Finally,
 }
 
 impl MergeStyle {
@@ -3297,17 +3302,20 @@ impl<'a> BindingsBuilder<'a> {
             return;
         }
 
+        let use_all_branches = matches!(merge_style, MergeStyle::Finally);
         // We normally only merge the live branches (where control flow is not
         // known to terminate), but if nothing is live we still need to fill in
         // the Phi keys and potentially analyze downstream code, so in that case
         // we'll use the terminated branches.
-        let (terminated_branches, live_branches): (Vec<_>, Vec<_>) =
-            branches.into_iter().partition(|flow| flow.has_terminated);
-        let has_terminated = live_branches.is_empty() && !merge_style.is_loop();
-        let flows = if has_terminated {
-            terminated_branches
+        let n_live_branches = branches.iter().filter(|flow| !flow.has_terminated).count();
+        let has_terminated = n_live_branches == 0 && !merge_style.is_loop();
+        let flows = if use_all_branches || has_terminated {
+            branches
         } else {
-            live_branches
+            branches
+                .into_iter()
+                .filter(|flow| !flow.has_terminated)
+                .collect()
         };
         // Determine reachability of the merged flow.
         // For Loop style with empty flows (all branches terminated), the loop body might
@@ -3319,6 +3327,11 @@ impl<'a> BindingsBuilder<'a> {
                 MergeStyle::Loop => base.is_definitely_unreachable,
                 _ => true,
             }
+        } else if use_all_branches && n_live_branches > 0 {
+            flows
+                .iter()
+                .filter(|f| !f.has_terminated)
+                .all(|f| f.is_definitely_unreachable)
         } else {
             flows.iter().all(|f| f.is_definitely_unreachable)
         };
@@ -3623,6 +3636,18 @@ impl<'a> BindingsBuilder<'a> {
         self.finish_fork_impl(None, false, None)
     }
 
+    /// Finish an exhaustive fork for a `finally` block. Unlike a normal merge,
+    /// branches that have already terminated still contribute to the merged type
+    /// state because they will execute the `finally` body first.
+    pub fn finish_finally_fork(&mut self) {
+        let fork = self.scopes.current_mut().forks.pop().unwrap();
+        assert!(
+            !fork.branch_started,
+            "A branch is started - did you forget to call `finish_branch`?"
+        );
+        self.merge_flow(fork.base, fork.branches, fork.range, MergeStyle::Finally);
+    }
+
     /// Finish a non-exhaustive fork in which the base flow is part of the merge. It negates
     /// the branch-choosing narrows by applying `negated_prev_ops` to base before merging, which
     /// is important so that we can preserve any cases where a termanating branch has permanently

pyrefly/lib/binding/stmt.rs

@@ -1184,11 +1184,10 @@ impl<'a> BindingsBuilder<'a> {
                         // https://docs.python.org/3/reference/compound_stmts.html#except-clause
                         self.scopes.mark_as_deleted(&name.id);
                     }
-
                     self.finish_branch();
                 }
 
-                self.finish_exhaustive_fork();
+                self.finish_finally_fork();
                 self.scopes.enter_finally();
                 self.stmts(x.finalbody, parent);
                 self.scopes.exit_finally();

pyrefly/lib/test/flow_branching.rs

@@ -2036,6 +2036,33 @@ def main(resolve: bool) -> None:
 "#,
 );
 
+// Issue #2845: `finally` must see both the successful `try` state and terminating `except` state.
+testcase!(
+    test_try_finally_preserves_pre_try_possibility_from_terminating_except,
+    r#"
+from typing import assert_type
+
+def something_that_might_throw() -> None:
+    raise Exception()
+
+class Thing:
+    def something(self) -> None:
+        pass
+
+def blah() -> None:
+    x = None
+    try:
+        something_that_might_throw()
+        if x is None:
+            x = Thing()
+    except Exception:
+        raise
+    finally:
+        assert_type(x, Thing | None)
+        x.something()  # E: Object of class `NoneType` has no attribute `something`
+"#,
+);
+
 // for https://github.com/facebook/pyrefly/issues/1840
 testcase!(
     test_exhaustive_flow_no_fall_through,