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
27 changes: 18 additions & 9 deletions pyrefly/lib/binding/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2510,8 +2510,11 @@ impl<'a> BindingsBuilder<'a> {
let mut flow_infos = merge_item.branches;
// Track the number of branch values before adding base (for LoopDefinitelyRuns)
let n_branch_flow_infos = flow_infos.len();
// Track if base has a value for this name (for LoopDefinitelyRuns init check)
let base_has_value = merge_item.base.as_ref().is_some_and(|b| b.value.is_some());
// Track if base has a binding for this name (value or narrow) for loop init checks.
let base_has_binding = merge_item
.base
.as_ref()
.is_some_and(|base| base.value.is_some() || base.narrow.is_some());
// If this is a loop, we want to use the current default in any phis we produce,
// and the base flow is part of the merge for type inference purposes.
let loop_prior = if merge_style.is_loop()
Expand Down Expand Up @@ -2552,11 +2555,9 @@ impl<'a> BindingsBuilder<'a> {
let mut value_idxs = SmallSet::with_capacity(flow_infos.len());
let mut branch_idxs = SmallSet::with_capacity(flow_infos.len());
let mut styles = Vec::with_capacity(flow_infos.len());
let mut n_values = 0;
for flow_info in flow_infos.into_iter() {
let branch_idx = flow_info.idx();
if let Some(v) = flow_info.value {
n_values += 1;
if v.idx == phi_idx {
continue;
}
Expand All @@ -2568,13 +2569,21 @@ impl<'a> BindingsBuilder<'a> {
}
branch_idxs.insert(branch_idx);
}
let is_name_exists_in_all_branch_flow = match merge_style {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block of code was inserted between the comment above & the piece of code it describes below. I think it should be moved above the comment

The comment might also need to be updated since you made changes below

Copy link
Contributor Author

@tannguyencse19 tannguyencse19 Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed new commit: cdbed60


I moved the new logic so it sits directly under the comment it describes, and updated the comment to match the final behavior

MergeStyle::Loop => n_branch_flow_infos == n_branches.saturating_sub(1),
_ => n_branch_flow_infos == n_branches,
};
// For LoopDefinitelyRuns, a name is always defined if:
// - It was defined before the loop (base_has_value), OR
// - It's defined in all loop body branches (since the loop definitely runs at least once)
// For regular loops and other merges, a name is always defined if it's in all branches.
// - It was defined before the loop (base_has_binding), OR
// - It's defined in all loop body branches (since the loop definitely runs at least once).
// For Loop, the name must exist in the base flow and in all loop body branches.
// For BoolOp, avoid propagating uninitialized styles across the merge.
// For other merges, a name is always defined if it appears in every branch flow.
let this_name_always_defined = match merge_style {
MergeStyle::LoopDefinitelyRuns => base_has_value || n_branch_flow_infos == n_branches,
_ => n_values == n_branches,
MergeStyle::Loop => base_has_binding && is_name_exists_in_all_branch_flow,
MergeStyle::LoopDefinitelyRuns => base_has_binding || is_name_exists_in_all_branch_flow,
MergeStyle::BoolOp => false,
_ => is_name_exists_in_all_branch_flow,
};
match value_idxs.len() {
// If there are no values, then this name isn't assigned at all
Expand Down
76 changes: 76 additions & 0 deletions pyrefly/lib/test/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,82 @@ def f():
"#,
);

fn env_singleton_attribute_assignment() -> TestEnv {
TestEnv::one(
"singleton",
r#"
class GlobalInventory:
_instance: GlobalInventory | None = None
_initialized: bool = False

initialization_status: bool
config_file_inventory: dict[str, int]

def __new__(cls) -> GlobalInventory:
if cls._instance is None:
cls._instance = super().__new__(cls)
assert cls._instance is not None, "GlobalInventory instance creation failed"
return cls._instance

def __init__(self) -> None:
if not self._initialized:
self.initialization_status = False
self.config_file_inventory = {}
GlobalInventory._initialized = True

globals_inv: GlobalInventory = GlobalInventory()
"#,
)
}

testcase!(
test_attribute_assignment_to_imported_singleton_in_method,
env_singleton_attribute_assignment(),
r#"
from singleton import globals_inv

class Foo:
def __init__(self) -> None:
globals_inv.config_file_inventory = {"foo": 1}
for _ in globals_inv.config_file_inventory.values():
if globals_inv.initialization_status:
break
globals_inv.initialization_status = True
"#,
);

testcase!(
test_walrus_reuse_name_in_if_condition,
r#"
from re import compile

interface_re = compile(r"^foo")
ipv4_re = compile(r"bar$")
line = str()

if match := interface_re.match(line):
pass

if line and (match := ipv4_re.search(line)):
print(match)
"#,
);

testcase!(
test_walrus_getattr_items_then_iterate,
r#"
from typing import Any

def test(thing: Any) -> None:
if not (items := getattr(thing, "items")):
return
if not isinstance(items, tuple | list):
items = (items,)
for item in items:
print(item)
"#,
);

testcase!(
test_unbound_local_name_error_in_def,
r#"
Expand Down