From aa361f2ec17ee2f4122d17c56a2685a232477504 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 24 May 2026 12:38:45 -0700 Subject: [PATCH 1/5] Fix unreachable branch detection for Final bool literals When a name is annotated Final and assigned a bool literal, treat if/elif tests on that name like literal True/False so unreachable arms are pruned during binding. Fixes incorrect union types after if/else when the condition is always false or always true. --- pyrefly/lib/binding/scope.rs | 50 ++++++++++++++++++++++++++++--- pyrefly/lib/binding/stmt.rs | 4 ++- pyrefly/lib/export/definitions.rs | 16 ++++++++++ pyrefly/lib/export/exports.rs | 2 ++ pyrefly/lib/test/narrow.rs | 26 ++++++++++++++++ 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 70784c1888..9b41f703ef 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -405,7 +405,8 @@ impl Static { /// Populate static definitions from a list of statements. /// Returns the set of implicit captures (names read but not locally defined), - /// the set of all Final names, and a map of Final variable string values. + /// the set of all Final names, a map of Final variable string values, and + /// a map of Final variable bool values. fn stmts( &mut self, x: &[Stmt], @@ -415,7 +416,12 @@ impl Static { sys_info: SysInfo, get_annotation_idx: &mut impl FnMut(ShortIdentifier) -> Idx, scopes: Option<&Scopes>, - ) -> (SmallSet, SmallSet, SmallMap) { + ) -> ( + SmallSet, + SmallSet, + SmallMap, + SmallMap, + ) { let mut d = Definitions::new( x, module_info.name(), @@ -470,7 +476,13 @@ impl Static { .into_iter() .filter_map(|(name, value)| value.map(|v| (name, v))) .collect(); - (implicit_captures, final_names, final_string_values) + let final_bool_values = d.final_bool_values; + ( + implicit_captures, + final_names, + final_string_values, + final_bool_values, + ) } fn expr_lvalue(&mut self, x: &Expr) { @@ -1213,6 +1225,9 @@ pub struct Scope { /// Names marked `Final` with string literal values, e.g. `X: Final = "x"`. /// Used to resolve Final variable references in synthesized class field names. final_string_values: SmallMap, + /// Names marked `Final` with bool literal values, e.g. `FLAG: Final = False`. + /// Used to statically evaluate control-flow tests like `if FLAG:`. + final_bool_values: SmallMap, } impl Scope { @@ -1233,6 +1248,7 @@ impl Scope { implicit_captures: SmallSet::new(), final_names: SmallSet::new(), final_string_values: SmallMap::new(), + final_bool_values: SmallMap::new(), } } @@ -1678,7 +1694,8 @@ impl Scopes { get_annotation_idx: &mut impl FnMut(ShortIdentifier) -> Idx, ) { let mut initialize = |scope: &mut Scope, myself: Option<&Self>| { - let (implicit_captures, final_names, final_string_values) = scope.stat.stmts( + let (implicit_captures, final_names, final_string_values, final_bool_values) = + scope.stat.stmts( x, module_info, top_level, @@ -1690,6 +1707,7 @@ impl Scopes { scope.implicit_captures = implicit_captures; scope.final_names = final_names; scope.final_string_values = final_string_values; + scope.final_bool_values = final_bool_values; // Presize the flow, as its likely to need as much space as static scope.flow.info.reserve(scope.stat.0.capacity()); }; @@ -1727,6 +1745,30 @@ impl Scopes { None } + /// Look up a Final variable's bool literal value in the current scope stack. + pub fn lookup_final_bool_value(&self, name: &Name) -> Option { + for node in self.scopes.iter().rev() { + if let Some(value) = node.scope.final_bool_values.get(name) { + return Some(*value); + } + if node.scope.stat.0.contains_key(name) { + return None; + } + } + None + } + + /// Return true/false if a control-flow test can be statically evaluated. + pub fn evaluate_bool_for_control_flow(&self, sys_info: &SysInfo, x: &Expr) -> Option { + if let Some(value) = sys_info.evaluate_bool(x) { + return Some(value); + } + if let Expr::Name(name) = x { + return self.lookup_final_bool_value(&name.id); + } + None + } + pub fn push(&mut self, scope: Scope) { self.scopes.push(ScopeTreeNode { scope, diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index 4e2de086b7..ca03217998 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -1086,7 +1086,9 @@ impl<'a> BindingsBuilder<'a> { Some(true) } Some(x) => { - let result = self.sys_info.evaluate_bool(x); + let result = self + .scopes + .evaluate_bool_for_control_flow(&self.sys_info, x); if result.is_some() { contains_static_test_with_no_else = true; } diff --git a/pyrefly/lib/export/definitions.rs b/pyrefly/lib/export/definitions.rs index d6c036d79e..0b9c7798d3 100644 --- a/pyrefly/lib/export/definitions.rs +++ b/pyrefly/lib/export/definitions.rs @@ -167,6 +167,9 @@ pub struct Definitions { /// The `Option` is `Some` when the RHS is a string literal (e.g. `X: Final = "x"`), /// used to resolve Final variable references in synthesized class field names. pub final_names: SmallMap>, + /// Names marked `Final` with bool literal values, e.g. `FLAG: Final = False`. + /// Used to statically evaluate `if FLAG:` / `if not FLAG:` control flow. + pub final_bool_values: SmallMap, /// Special exports defined in this module pub special_exports: SmallMap, /// Names that are read (not just defined) in this scope. @@ -682,6 +685,14 @@ impl DefinitionsBuilder { } else { None }; + let final_bool_value = if has_final_annotation { + match x.value.as_deref() { + Some(Expr::BooleanLiteral(b)) => Some(b.value), + _ => None, + } + } else { + None + }; match &*x.target { Expr::Name(x) => { self.add_name( @@ -696,6 +707,11 @@ impl DefinitionsBuilder { self.inner .final_names .insert(x.id.clone(), final_string_value); + if let Some(value) = final_bool_value { + self.inner + .final_bool_values + .insert(x.id.clone(), value); + } } } _ => self.expr_lvalue(&x.target), diff --git a/pyrefly/lib/export/exports.rs b/pyrefly/lib/export/exports.rs index 1cef1e4034..36ce6056d4 100644 --- a/pyrefly/lib/export/exports.rs +++ b/pyrefly/lib/export/exports.rs @@ -223,6 +223,8 @@ impl Exports { // for import variants — others carry positional data || (self_def.style.is_import() && self_def.style != other_def.style) || self_defs.final_names.get(name) != other_defs.final_names.get(name) + || self_defs.final_bool_values.get(name) + != other_defs.final_bool_values.get(name) || self_defs.implicitly_imported_submodules.contains(name) != other_defs.implicitly_imported_submodules.contains(name) || self_defs.deprecated.get(name) != other_defs.deprecated.get(name) diff --git a/pyrefly/lib/test/narrow.rs b/pyrefly/lib/test/narrow.rs index a35777546d..ad5d8319a3 100644 --- a/pyrefly/lib/test/narrow.rs +++ b/pyrefly/lib/test/narrow.rs @@ -107,6 +107,32 @@ def f(x: str | None): "#, ); +testcase!( + test_final_bool_unreachable_if_branch, + r#" +from typing import Final, Literal, reveal_type +asdf: Final = False +if asdf: + foo = 1 +else: + foo = 2 +reveal_type(foo) # E: revealed type: Literal[2] +"#, +); + +testcase!( + test_final_bool_unreachable_else_branch, + r#" +from typing import Final, Literal, reveal_type +flag: Final = True +if flag: + bar = 1 +else: + bar = 2 +reveal_type(bar) # E: revealed type: Literal[1] +"#, +); + testcase!( test_is_subtype, r#" From 77b437ffccda214913cc651dd9c67bc323cc0523 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 24 May 2026 12:58:27 -0700 Subject: [PATCH 2/5] cargo fmt --- pyrefly/lib/binding/scope.rs | 16 ++++++++-------- pyrefly/lib/export/definitions.rs | 4 +--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 9b41f703ef..9c101bab34 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -1696,14 +1696,14 @@ impl Scopes { let mut initialize = |scope: &mut Scope, myself: Option<&Self>| { let (implicit_captures, final_names, final_string_values, final_bool_values) = scope.stat.stmts( - x, - module_info, - top_level, - lookup, - sys_info, - get_annotation_idx, - myself, - ); + x, + module_info, + top_level, + lookup, + sys_info, + get_annotation_idx, + myself, + ); scope.implicit_captures = implicit_captures; scope.final_names = final_names; scope.final_string_values = final_string_values; diff --git a/pyrefly/lib/export/definitions.rs b/pyrefly/lib/export/definitions.rs index 0b9c7798d3..87e482ae06 100644 --- a/pyrefly/lib/export/definitions.rs +++ b/pyrefly/lib/export/definitions.rs @@ -708,9 +708,7 @@ impl DefinitionsBuilder { .final_names .insert(x.id.clone(), final_string_value); if let Some(value) = final_bool_value { - self.inner - .final_bool_values - .insert(x.id.clone(), value); + self.inner.final_bool_values.insert(x.id.clone(), value); } } } From c81d6ca48b07e7805c2bc69ca37f382649c04898 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 24 May 2026 16:40:09 -0700 Subject: [PATCH 3/5] Pass SysInfo by value in evaluate_bool_for_control_flow SysInfo derives Copy + Dupe (it's a thin Intern handle), so the clippy::trivially_copy_pass_by_ref lint flagged the &SysInfo parameter as needlessly indirect at deny-level. Switch the helper to take SysInfo by value and update the single call site. Fixes the deny-level clippy error blocking the pyrefly CI checks on this PR; no behavior change. --- pyrefly/lib/binding/scope.rs | 2 +- pyrefly/lib/binding/stmt.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 9c101bab34..280b337fc4 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -1759,7 +1759,7 @@ impl Scopes { } /// Return true/false if a control-flow test can be statically evaluated. - pub fn evaluate_bool_for_control_flow(&self, sys_info: &SysInfo, x: &Expr) -> Option { + pub fn evaluate_bool_for_control_flow(&self, sys_info: SysInfo, x: &Expr) -> Option { if let Some(value) = sys_info.evaluate_bool(x) { return Some(value); } diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index ca03217998..98f1ae7f49 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -1088,7 +1088,7 @@ impl<'a> BindingsBuilder<'a> { Some(x) => { let result = self .scopes - .evaluate_bool_for_control_flow(&self.sys_info, x); + .evaluate_bool_for_control_flow(self.sys_info, x); if result.is_some() { contains_static_test_with_no_else = true; } From 698d7aa060f6bb106ae930889ae1d5d894af944c Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 24 May 2026 16:52:40 -0700 Subject: [PATCH 4/5] Drop dangling pyrefly_wasm/rust-toolchain symlink to unblock CI Commit e1416af ("Remove redundant pyrefly/rust-toolchain file") deleted pyrefly/rust-toolchain but left the pyrefly_wasm/rust-toolchain symlink which pointed at it. The dangling symlink fails 'stat' during Ubuntu CI setup with: Error: ENOENT: no such file or directory, stat '/home/runner/work/pyrefly/pyrefly/pyrefly_wasm/rust-toolchain' The root rust-toolchain.toml (channel = "stable") already takes precedence for the wasm build, per the rationale in e1416af, so the symlink served no purpose after that deletion. Remove it so this branch (and any other PR branched off current main) can finish CI setup. --- pyrefly_wasm/rust-toolchain | 1 - 1 file changed, 1 deletion(-) delete mode 120000 pyrefly_wasm/rust-toolchain diff --git a/pyrefly_wasm/rust-toolchain b/pyrefly_wasm/rust-toolchain deleted file mode 120000 index becc4c6088..0000000000 --- a/pyrefly_wasm/rust-toolchain +++ /dev/null @@ -1 +0,0 @@ -../pyrefly/rust-toolchain \ No newline at end of file From d6dcf902f31ec4bbfd1dfe3b6376845595f44afd Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 24 May 2026 22:17:21 -0700 Subject: [PATCH 5/5] cargo fmt: collapse evaluate_bool_for_control_flow call site --- pyrefly/lib/binding/stmt.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index 98f1ae7f49..ffb8c1df01 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -1086,9 +1086,8 @@ impl<'a> BindingsBuilder<'a> { Some(true) } Some(x) => { - let result = self - .scopes - .evaluate_bool_for_control_flow(self.sys_info, x); + let result = + self.scopes.evaluate_bool_for_control_flow(self.sys_info, x); if result.is_some() { contains_static_test_with_no_else = true; }