From 6f5c87dd0849eb6c1fed278f1ce78b6c26e674d1 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Wed, 29 Jul 2026 00:31:45 +0900 Subject: [PATCH 01/10] fix fix test --- pyrefly/lib/alt/solve.rs | 26 +- pyrefly/lib/binding/binding.rs | 13 +- pyrefly/lib/binding/class.rs | 50 +++- pyrefly/lib/binding/django.rs | 5 +- pyrefly/lib/binding/function.rs | 2 + pyrefly/lib/binding/scope.rs | 196 +++++++++--- pyrefly/lib/binding/stmt.rs | 26 +- pyrefly/lib/binding/target.rs | 11 +- pyrefly/lib/test/attributes.rs | 515 -------------------------------- pyrefly/lib/test/scope.rs | 7 +- 10 files changed, 243 insertions(+), 608 deletions(-) diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index dcb9c0ebb1..d6ba8907a6 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3211,27 +3211,23 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { BindingExpect::UninitializedAttributeCheck { name, range, - class, + getattr_class, termination_keys, } => { - let initialized = termination_keys + let may_be_uninitialized = termination_keys .as_ref() - .is_some_and(|keys| keys.iter().all(|key| self.get_idx(*key).ty().is_never())); - let inherited = !initialized - && self.get_idx(*class).0.as_ref().is_some_and(|cls| { - self.get_mro_for_class(cls) - .ancestors_no_object() - .iter() - .any(|base| { - base.class_object() != cls - && self.get_class_member(base.class_object(), name).is_some() - }) - }); - if !initialized && !inherited { + .is_none_or(|keys| !keys.iter().all(|key| self.get_idx(*key).ty().is_never())); + let has_getattr_fallback = getattr_class.is_some_and(|class_idx| { + self.get_idx(class_idx) + .0 + .as_ref() + .is_some_and(|cls| self.get_class_member(cls, &dunder::GETATTR).is_some()) + }); + if may_be_uninitialized && !has_getattr_fallback { errors .error_builder( *range, - ErrorKind::PossiblyUninitializedAttribute, + ErrorKind::MissingAttribute, format!("Attribute `{name}` may be uninitialized"), ) .emit(); diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index b4e8bf7b30..7b06a2ccce 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1366,12 +1366,15 @@ pub enum BindingExpect { /// If any don't, the variable may be uninitialized. termination_keys: Vec>, }, - /// Check whether a class-body attribute is initialized on every completing path. + /// Check that a class attribute is initialized on every control-flow path. UninitializedAttributeCheck { name: Name, range: TextRange, - class: Idx, - /// All missing paths must terminate. None means a path lacks termination evidence. + /// The owning class when this is an instance attribute. Its `__getattr__` may make a + /// conditionally initialized attribute safe to access. + getattr_class: Option>, + /// `None` means binding proved the declaration conditional. Otherwise, initialization + /// depends on whether all of these branches terminate with `Never`. termination_keys: Option>>, }, /// Check for forward reference string literal in union type. @@ -1548,7 +1551,7 @@ impl DisplayWith for BindingExpect { Self::UninitializedAttributeCheck { name, range, - class, + getattr_class, termination_keys, } => { write!( @@ -1556,7 +1559,7 @@ impl DisplayWith for BindingExpect { "UninitializedAttributeCheck({}, {}, {:?}, {:?})", name, ctx.module().display(range), - class, + getattr_class, termination_keys ) } diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 12e5089095..319f12d37d 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -73,8 +73,10 @@ use crate::binding::binding::KeyClassSynthesizedFields; use crate::binding::binding::KeyExpect; use crate::binding::binding::KeyTParams; use crate::binding::binding::KeyVariance; +use crate::binding::binding::MethodSelfKind; use crate::binding::bindings::BindingsBuilder; use crate::binding::bindings::CurrentIdx; +use crate::binding::bindings::InitializedInFlow; use crate::binding::bindings::LegacyTParamCollector; use crate::binding::expr::Usage; use crate::binding::pydantic::PydanticConfigDict; @@ -469,7 +471,7 @@ impl<'a> BindingsBuilder<'a> { &NestingContext::class(ShortIdentifier::new(&x.name), parent.dupe()), ); self.finish_classmethod_receivers(); - let (field_definitions, nn_module_registrations, uninitialized_fields) = + let (field_definitions, nn_module_registrations) = self.scopes.finish_class_and_get_field_definitions(); self.track_classmethod_uses = previous_tracking; self.insert_binding_idx( @@ -484,7 +486,8 @@ impl<'a> BindingsBuilder<'a> { let django_field_info = self.extract_django_fields_from_class_body(&field_definitions); let mut fields = SmallMap::with_capacity(field_definitions.len()); let mut django_relation_fields = Vec::new(); - for (name, (definition, range)) in field_definitions.into_iter_hashed() { + + for (name, (definition, range, initialized)) in field_definitions.into_iter_hashed() { if let ClassFieldDefinition::AssignedInBody { value, .. } = &definition && let ExprOrBinding::Expr(e) = value.as_ref() { @@ -508,19 +511,36 @@ impl<'a> BindingsBuilder<'a> { let attrs_field_specifier = self.attrs_field_specifier(&definition, name.key(), range, &attrs_decorators); - // Stub files declare attributes without runtime initialization. - if !self.module_info.path().is_interface() - && let Some(termination_keys) = uninitialized_fields.get(name.key()) - { - self.insert_binding( - KeyExpect::UninitializedAttributeCheck(range), - BindingExpect::UninitializedAttributeCheck { - name: name.key().clone(), - range, - class: class_indices.class_idx, - termination_keys: termination_keys.clone(), - }, - ); + let check_initialization = !matches!( + &definition, + ClassFieldDefinition::DefinedInMethod { method, .. } + if !method.recognized_attribute_defining_method + ); + if check_initialization { + let termination_keys = match &initialized { + InitializedInFlow::Yes => None, + InitializedInFlow::Conditionally | InitializedInFlow::No => Some(None), + InitializedInFlow::DeferredCheck(keys) => Some(Some(keys.clone())), + }; + if let Some(termination_keys) = termination_keys { + let getattr_class = matches!( + &definition, + ClassFieldDefinition::DefinedInMethod { + receiver_kind: MethodSelfKind::Instance, + .. + } + ) + .then_some(class_indices.class_idx); + self.insert_binding( + KeyExpect::UninitializedAttributeCheck(range), + BindingExpect::UninitializedAttributeCheck { + name: name.key().clone(), + range, + getattr_class, + termination_keys, + }, + ); + } } fields.insert_hashed( diff --git a/pyrefly/lib/binding/django.rs b/pyrefly/lib/binding/django.rs index a5ca5b4e97..f0c87a2c27 100644 --- a/pyrefly/lib/binding/django.rs +++ b/pyrefly/lib/binding/django.rs @@ -13,6 +13,7 @@ use starlark_map::small_map::SmallMap; use crate::binding::binding::ClassFieldDefinition; use crate::binding::binding::ExprOrBinding; use crate::binding::bindings::BindingsBuilder; +use crate::binding::bindings::InitializedInFlow; const PRIMARY_KEY: Name = Name::new_static("primary_key"); const FOREIGN_KEY: Name = Name::new_static("ForeignKey"); @@ -39,13 +40,13 @@ impl<'a> BindingsBuilder<'a> { /// (primary_key, ForeignKey, choices). pub fn extract_django_fields_from_class_body( &self, - field_definitions: &SmallMap, + field_definitions: &SmallMap, ) -> DjangoFieldInfo { let mut primary_key_field = None; let mut foreign_key_like_fields = Vec::new(); let mut relation_fields = Vec::new(); let mut fields_with_choices = Vec::new(); - for (name, (definition, _range)) in field_definitions.iter() { + for (name, (definition, _range, _initialized)) in field_definitions.iter() { if let ClassFieldDefinition::AssignedInBody { value, .. } = definition && let ExprOrBinding::Expr(e) = value.as_ref() && let Some(call) = e.as_call_expr() diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index cda4685dbf..5ac733f250 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -69,6 +69,7 @@ use crate::binding::binding::ReturnImplicit; use crate::binding::binding::ReturnType; use crate::binding::binding::ReturnTypeKind; use crate::binding::bindings::BindingsBuilder; +use crate::binding::bindings::InitializedInFlow; use crate::binding::bindings::LegacyTParamCollector; use crate::binding::classmethod::FunctionDecorators; use crate::binding::expr::Usage; @@ -271,6 +272,7 @@ impl<'a> SelfAttrNames<'a> { None, r, MethodSelfKind::Instance, + InitializedInFlow::Yes, ), ) }) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 17eeb50ca4..e8f3c9eff1 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -1219,8 +1219,15 @@ pub struct InstanceAttribute( pub Option>, pub TextRange, pub MethodSelfKind, + pub InitializedInFlow, ); +fn instance_attribute_flow_name(name: &Name) -> Name { + // This cannot collide with a Python identifier, but lets attribute initialization reuse + // the same branch-merging machinery as local variables. + Name::new(format!("$self.{name}")) +} + #[derive(Clone, Debug)] struct ScopeMethod { name: Identifier, @@ -1230,6 +1237,7 @@ struct ScopeMethod { new_instance_names: SmallSet, instance_attributes: SmallMap, nn_module_registrations: NNModuleRegistrations, + return_attribute_initialization: Vec>, parameters: SmallMap, yields_and_returns: YieldsAndReturns, is_async: bool, @@ -1303,6 +1311,7 @@ impl ScopeMethod { new_instance_names: SmallSet::new(), instance_attributes: SmallMap::new(), nn_module_registrations: SmallMap::new(), + return_attribute_initialization: Vec::new(), parameters: SmallMap::new(), yields_and_returns: Default::default(), is_async, @@ -2308,16 +2317,61 @@ impl Scopes { let scope = self.pop(); let unused_variables = Self::collect_unused_variables(scope.variables.clone()); match scope.kind { - ScopeKind::Method(method_scope) => ( - method_scope.yields_and_returns, - Some(SelfAssignments { - method_name: method_scope.name.id, - instance_attributes: method_scope.instance_attributes, - nn_module_registrations: method_scope.nn_module_registrations, - }), - Self::collect_unused_parameters(method_scope.parameters), - unused_variables, - ), + ScopeKind::Method(mut method_scope) => { + for (name, attribute) in method_scope.instance_attributes.iter_mut() { + let mut initialized = if scope.flow.has_terminated { + None + } else { + Some( + scope + .flow + .get_info(&instance_attribute_flow_name(name)) + .map_or(InitializedInFlow::Conditionally, FlowInfo::initialized), + ) + }; + for return_initialization in &method_scope.return_attribute_initialization { + let on_return = return_initialization + .get(name) + .cloned() + .unwrap_or(InitializedInFlow::Conditionally); + initialized = Some(match (initialized, on_return) { + (None, initialized) => initialized, + (Some(InitializedInFlow::Yes), InitializedInFlow::Yes) => { + InitializedInFlow::Yes + } + ( + Some(InitializedInFlow::DeferredCheck(mut existing)), + InitializedInFlow::DeferredCheck(mut new), + ) => { + existing.append(&mut new); + InitializedInFlow::DeferredCheck(existing) + } + ( + Some(InitializedInFlow::Yes), + InitializedInFlow::DeferredCheck(keys), + ) + | ( + Some(InitializedInFlow::DeferredCheck(keys)), + InitializedInFlow::Yes, + ) => InitializedInFlow::DeferredCheck(keys), + _ => InitializedInFlow::Conditionally, + }); + } + // If every path raises or otherwise terminates without returning an instance, + // there is no successfully initialized object missing this attribute. + attribute.4 = initialized.unwrap_or(InitializedInFlow::Yes); + } + ( + method_scope.yields_and_returns, + Some(SelfAssignments { + method_name: method_scope.name.id, + instance_attributes: method_scope.instance_attributes, + nn_module_registrations: method_scope.nn_module_registrations, + }), + Self::collect_unused_parameters(method_scope.parameters), + unused_variables, + ) + } ScopeKind::Function(function_scope) => ( function_scope.yields_and_returns, None, @@ -2351,12 +2405,29 @@ impl Scopes { x: &ExprAttribute, value: ExprOrBinding, annotation: Option>, + idx: Option>, ) -> bool { + let in_loop = self.loop_depth() != 0; for scope in self.iter_rev_mut() { if let ScopeKind::Method(method_scope) = &mut scope.kind && let Some(self_name) = &method_scope.self_name && matches!(&*x.value, Expr::Name(name) if name.id == self_name.id) { + if let Some(idx) = idx { + match scope + .flow + .info + .entry(instance_attribute_flow_name(&x.attr.id)) + { + Entry::Vacant(entry) => { + entry.insert(FlowInfo::new_value(idx, FlowStyle::Other)); + } + Entry::Occupied(mut entry) => { + *entry.get_mut() = + entry.get().updated_value(idx, FlowStyle::Other, in_loop); + } + } + } if let Some(attr) = method_scope.instance_attributes.get_mut(&x.attr.id) { // Accumulate subsequent assignments in the method. attr.0.push(value); @@ -2373,6 +2444,7 @@ impl Scopes { annotation, x.attr.range(), // Keep the range of the first assignment as the definition location. method_scope.receiver_kind, + InitializedInFlow::Yes, ), ); } @@ -3300,15 +3372,43 @@ impl Scopes { x: StmtReturn, is_unreachable: bool, ) -> Result<(), (CurrentIdx, StmtReturn)> { - match self.current_yields_and_returns_mut() { - Some(yields_and_returns) => { - yields_and_returns - .returns - .push((ret.into_idx(), x, is_unreachable)); - Ok(()) + for scope in self.iter_rev_mut() { + match &mut scope.kind { + ScopeKind::Function(function) => { + function + .yields_and_returns + .returns + .push((ret.into_idx(), x, is_unreachable)); + return Ok(()); + } + ScopeKind::Method(method) => { + if !is_unreachable { + let mut initialization = + SmallMap::with_capacity(method.instance_attributes.len()); + for name in method.instance_attributes.keys() { + initialization.insert( + name.clone(), + scope + .flow + .get_info(&instance_attribute_flow_name(name)) + .map_or( + InitializedInFlow::Conditionally, + FlowInfo::initialized, + ), + ); + } + method.return_attribute_initialization.push(initialization); + } + method + .yields_and_returns + .returns + .push((ret.into_idx(), x, is_unreachable)); + return Ok(()); + } + _ => {} } - None => Err((ret, x)), } + Err((ret, x)) } /// Record a yield in the enclosing function body there is one. @@ -3367,18 +3467,13 @@ impl Scopes { /// coming from self-assignment in methods. If both occur, only the class body /// definition is tracked. /// - Panics if the current scope is not a class body. - /// - /// The final map contains possibly uninitialized class-body fields. Its optional - /// keys can prove that every path without an assignment terminates. pub fn finish_class_and_get_field_definitions( &mut self, ) -> ( - SmallMap, + SmallMap, NNModuleRegistrations, - SmallMap>>>, ) { let mut field_definitions = SmallMap::new(); - let mut uninitialized_fields = SmallMap::new(); let class_body = self.pop(); let mut class_scope = { if let ScopeKind::Class(class_scope) = class_body.kind { @@ -3488,24 +3583,26 @@ impl Scopes { definition: value.idx, }, }; - match &value.style { - _ if static_info.function_def_only => {} - FlowStyle::PossiblyUninitialized => { - uninitialized_fields.insert_hashed(name.owned(), None); - } + let initialized = match &value.style { + FlowStyle::PossiblyUninitialized => InitializedInFlow::Conditionally, FlowStyle::MaybeInitialized(keys) => { - uninitialized_fields.insert_hashed(name.owned(), Some(keys.clone())); + InitializedInFlow::DeferredCheck(keys.clone()) } - _ => {} - } - field_definitions.insert_hashed(name.owned(), (definition, static_info.range)); + _ => InitializedInFlow::Yes, + }; + field_definitions + .insert_hashed(name.owned(), (definition, static_info.range, initialized)); } }); // Merge assignments from different methods. // `method_attrs` yields attributes from recognized constructor methods first (e.g. __init__), // followed by other helper methods. method_attrs.into_iter().for_each( - |(name, method, InstanceAttribute(values, annotation, range, receiver_kind))| { + |( + name, + method, + InstanceAttribute(values, annotation, range, receiver_kind, initialized), + )| { if let Some(( ClassFieldDefinition::DefinedInMethod { values: existing_values, @@ -3515,6 +3612,7 @@ impl Scopes { has_instance_write: existing_instance_write, }, _, + existing_initialized, )) = field_definitions.get_mut(name.key()) { if matches!(receiver_kind, MethodSelfKind::Instance) { @@ -3537,6 +3635,31 @@ impl Scopes { if matches!(receiver_kind, MethodSelfKind::Class) { *existing_receiver = MethodSelfKind::Class; } + match (&mut *existing_initialized, initialized) { + (InitializedInFlow::Yes, _) | (_, InitializedInFlow::Yes) => { + *existing_initialized = InitializedInFlow::Yes; + } + ( + InitializedInFlow::DeferredCheck(existing), + InitializedInFlow::DeferredCheck(mut new), + ) => existing.append(&mut new), + ( + InitializedInFlow::DeferredCheck(_), + InitializedInFlow::Conditionally, + ) + | ( + InitializedInFlow::Conditionally, + InitializedInFlow::DeferredCheck(_), + ) + | ( + InitializedInFlow::Conditionally, + InitializedInFlow::Conditionally, + ) + | (InitializedInFlow::No, _) + | (_, InitializedInFlow::No) => { + *existing_initialized = InitializedInFlow::Conditionally; + } + } } } else if !field_definitions.contains_key_hashed(name.as_ref()) { field_definitions.insert_hashed( @@ -3553,16 +3676,13 @@ impl Scopes { ), }, range, + initialized, ), ); } }, ); - ( - field_definitions, - nn_module_registrations, - uninitialized_fields, - ) + (field_definitions, nn_module_registrations) } /// Return a pair Some((method_name, class_key)) if we are currently in a method diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index fd2e62d8ed..f439cf27e1 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -1287,11 +1287,14 @@ impl<'a> BindingsBuilder<'a> { None, ), ); - let value = match x.value { + let (value, attr_idx) = match x.value { Some(mut assigned) => { - self.bind_attr_assign(attr.clone(), &mut assigned, |_attr, value, _| { - ExprOrBinding::Expr(value.clone()) - }) + let (value, idx) = self.bind_attr_assign( + attr.clone(), + &mut assigned, + |_attr, value, _| ExprOrBinding::Expr(value.clone()), + ); + (value, Some(idx)) } _ => { self.ensure_expr( @@ -1300,13 +1303,18 @@ impl<'a> BindingsBuilder<'a> { is_annotation: false, }, ); - ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit)) + ( + ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit)), + None, + ) } }; - if !self - .scopes - .record_self_attr_assign(&attr, value.clone(), Some(ann_key)) - { + if !self.scopes.record_self_attr_assign( + &attr, + value.clone(), + Some(ann_key), + attr_idx, + ) { self.error( x.range, ErrorKind::BadAssignment, diff --git a/pyrefly/lib/binding/target.rs b/pyrefly/lib/binding/target.rs index bb3191fa12..30f08f3f67 100644 --- a/pyrefly/lib/binding/target.rs +++ b/pyrefly/lib/binding/target.rs @@ -193,7 +193,7 @@ impl<'a> BindingsBuilder<'a> { Option>, ) -> ExprOrBinding, ensure_assigned: bool, - ) -> ExprOrBinding { + ) -> (ExprOrBinding, Idx) { let narrowing_identifier = identifier_and_chain_prefix_for_expr(&Expr::Attribute(attr.clone())) .map(|(identifier, _)| identifier); @@ -236,7 +236,7 @@ impl<'a> BindingsBuilder<'a> { if let Some(identifier) = narrowing_identifier { self.narrow_if_name_is_defined(identifier, idx); } - value + (value, idx) } pub fn bind_attr_assign( @@ -248,7 +248,7 @@ impl<'a> BindingsBuilder<'a> { &Expr, Option>, ) -> ExprOrBinding, - ) -> ExprOrBinding { + ) -> (ExprOrBinding, Idx) { self.bind_attr_assign_impl( attr, Some(assigned), @@ -348,7 +348,7 @@ impl<'a> BindingsBuilder<'a> { ); } Expr::Attribute(x) => { - let attr_value = self.bind_attr_assign_impl( + let (attr_value, attr_idx) = self.bind_attr_assign_impl( x.clone(), assigned, |_attr, value, ann| make_assigned_value(value, ann), @@ -356,7 +356,8 @@ impl<'a> BindingsBuilder<'a> { ); // If this is a self-assignment, record it because we may use it to infer // the existence of an instance-only attribute. - self.scopes.record_self_attr_assign(x, attr_value, None); + self.scopes + .record_self_attr_assign(x, attr_value, None, Some(attr_idx)); self.scopes.record_new_instance_attr_assign(x); } Expr::Subscript(x) => { diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 273c2d0523..0f83d54374 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -10,421 +10,6 @@ use crate::test::util::TestEnv; use crate::testcase; -testcase!( - test_class_body_initialization_is_independent_of_methods, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -def cond() -> bool: ... - -class Constructor: - if cond(): - x = 1 # E: Attribute `x` may be uninitialized - - def __init__(self) -> None: - self.x = 2 - -class Helper: - if cond(): - x = 1 # E: Attribute `x` may be uninitialized - - def initialize(self) -> None: - self.x = 2 -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_inherited, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -class Base: - timeout = 10 - def __init__(self) -> None: - self.value = 0 - -class Child(Base): - def __init__(self, t: int | None = None) -> None: - super().__init__() - if t is not None: - self.timeout = t - self.value = t - self.own = t - -class Grandchild(Child): - def __init__(self, t: int | None = None) -> None: - if t is not None: - self.timeout = t - self.value = t - -class ClassBody(Base): - if bool(): - timeout = 20 -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_trailing_noreturn, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -from typing import NoReturn - -def stop() -> NoReturn: - raise ValueError() - -def proceed() -> None: ... - -class TrailingCall: - def __init__(self, flag: bool) -> None: - if flag: - self.value = 1 - stop() - -class TrailingRaise: - def __init__(self, flag: bool) -> None: - if flag: - self.value = 1 - raise ValueError() - -class DeferredThenStop: - def __init__(self, flag: bool) -> None: - if flag: - self.value = 1 - else: - proceed() - stop() - -class DeferredThenProceed: - def __init__(self, flag: bool) -> None: - if flag: - self.value = 1 - else: - stop() - proceed() - -class EarlyReturn: - def __init__(self, flag: bool) -> None: - if flag: - return - self.value = 1 - stop() -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_while_true_return, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -def connect() -> int: ... - -class Retry: - def __init__(self) -> None: - while True: - try: - self.conn = connect() - return - except OSError: - continue - -class EarlyReturn: - def __init__(self, flag: bool) -> None: - while True: - if flag: - return - self.value = 1 - return - -class ReturnOrBreak: - def __init__(self, flag: bool) -> None: - while True: - if flag: - self.value = 1 - return - break -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_nested_function, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -class NestedOnly: - def __init__(self) -> None: - def later() -> None: - self.value = 1 - -class ConditionalOuter: - def __init__(self, flag: bool) -> None: - if flag: - self.value = 1 - def later() -> None: - self.value = 2 - -class InitializedOuter: - def __init__(self) -> None: - self.value = 1 - def later() -> None: - del self.value - -class NestedMethod: - def __init__(self) -> None: - class Inner: - def later(inner) -> None: - self.value = 1 - -class NestedReturn: - def __init__(self) -> None: - def later() -> None: - return - self.value = 1 -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_control_flow, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -from contextlib import nullcontext -from typing import Literal - -class TryExcept: - def __init__(self, raw: str) -> None: - try: - self.value = int(raw) - except ValueError: - self.value = 0 - -class TryExceptMissing: - def __init__(self, raw: str) -> None: - try: - self.value = int(raw) - except ValueError: - pass - -class Finally: - def __init__(self) -> None: - try: - int("1") - finally: - self.value = 1 - -class With: - def __init__(self) -> None: - with nullcontext(): - self.value = 1 - -class WithConditional: - def __init__(self, flag: bool) -> None: - with nullcontext(): - if flag: - self.value = 1 - -class Match: - def __init__(self, kind: Literal["a", "b"]) -> None: - match kind: - case "a": - self.value = 1 - case "b": - self.value = 2 - -class MatchMissing: - def __init__(self, kind: str) -> None: - match kind: - case "a": - self.value = 1 - -class Deleted: - def __init__(self) -> None: - self.value = 1 - del self.value - -class DeletedConditionally: - def __init__(self, flag: bool) -> None: - self.value = 1 - if flag: - del self.value - -class Reassigned: - def __init__(self) -> None: - self.value = 1 - del self.value - self.value = 2 -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_class_conditions_and_hooks, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -import sys -from dataclasses import dataclass -from typing import TYPE_CHECKING, assert_type - -class Conditional: - if TYPE_CHECKING: - type_only = 1 - if sys.version_info >= (3, 10): - versioned = 1 - -class VersionedLoop: - while sys.version_info >= (3, 0): - pass - value = 1 - -@dataclass -class PostInit: - flag: bool - def __post_init__(self) -> None: - self.value = 1 - -@dataclass -class ConditionalPostInit: - flag: bool - def __post_init__(self) -> None: - if self.flag: - self.value = 1 - -class SubclassHook: - def __init_subclass__(cls) -> None: - cls.value = 1 - -class ConditionalSubclassHook: - def __init_subclass__(cls, flag: bool = False) -> None: - if flag: - cls.value = 1 - -class ClassLoop: - for value in [1]: - pass - -assert_type(Conditional.type_only, int) -assert_type(Conditional.versioned, int) -assert_type(ClassLoop.value, int) -"#, -); - -testcase!( - bug = "Suppressed exceptions do not merge attribute initialization", - test_attribute_initialization_suppressed_exception, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -from contextlib import suppress - -class C: - def __init__(self, raw: str) -> None: - with suppress(ValueError): - self.value = int(raw) -"#, -); - -testcase!( - test_attribute_initialization_finally_after_return, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -class C: - def __init__(self) -> None: - try: - return - finally: - self.value = 1 -"#, -); - -testcase!( - bug = "For loop targets are treated as assigned before the first iteration", - test_attribute_initialization_class_loop_target, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -from typing import assert_type - -class C: - for value in list[int](): - pass - -assert_type(C.value, int) -"#, -); - -testcase!( - test_possibly_uninitialized_attribute_default_disabled, - r#" -def enabled() -> bool: ... - -class C: - if enabled(): - class_value = 1 - - def __init__(self, flag: bool) -> None: - if flag: - self.value = 1 - -C(True).missing # E: Object of class `C` has no attribute `missing` -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_possibly_uninitialized_attribute_suppression, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -class C: - def __init__(self, flag: bool) -> None: - if flag: - self.suppressed = 1 # pyrefly: ignore[possibly-uninitialized-attribute] - self.reported = 1 # pyrefly: ignore[missing-attribute] - -C(True).missing # pyrefly: ignore[possibly-uninitialized-attribute] # E: Object of class `C` has no attribute `missing` -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_possibly_uninitialized_attribute_stub, - TestEnv::one_with_path( - "foo", - "foo.pyi", - r#" -def enabled() -> bool: ... - -class Stub: - if enabled(): - value = 1 - annotated: int = 1 - - def __init__(self, flag: bool) -> None: - if flag: - self.instance_value = 1 -"#, - ) - .enable_possibly_uninitialized_attribute_error(), - r#" -from typing import assert_type -from foo import Stub - -assert_type(Stub.value, int) -assert_type(Stub.annotated, int) -assert_type(Stub(True).instance_value, int) - -class Child(Stub): ... - -assert_type(Child.value, int) - -def enabled() -> bool: ... - -class Implementation: - if enabled(): - value = 1 # E: Attribute `value` may be uninitialized - - def __init__(self, flag: bool) -> None: - if flag: - self.instance_value = 1 -"#, -); - fn env_with_conditional_attribute() -> TestEnv { TestEnv::one( "foo", @@ -435,7 +20,6 @@ class Base: value = 1 # E: Attribute `value` may be uninitialized "#, ) - .enable_possibly_uninitialized_attribute_error() } testcase!( @@ -449,105 +33,6 @@ Child.value "#, ); -testcase!( - test_conditionally_defined_methods, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -from typing import assert_type - -def enabled() -> bool: ... -def fallback() -> None: ... - -class ConditionalMethods: - if enabled(): - def method(self) -> int: - return 1 - - async def async_method(self) -> int: - return 1 - - @staticmethod - def static_method() -> int: - return 1 - - @classmethod - def class_method(cls) -> int: - return 1 - - @property - def prop(self) -> int: - return 1 - - if enabled(): - def nested(self) -> int: - return 1 - else: - def nested(self) -> int: - return 2 - - while enabled(): - def loop_method(self) -> int: - return 1 - break - - if enabled(): - def deferred(self) -> int: - return 1 - else: - fallback() - - # A later merge must also preserve the method's definition source. - if enabled(): - data = 1 # E: Attribute `data` may be uninitialized - -assert_type(ConditionalMethods().method(), int) -assert_type(ConditionalMethods.static_method(), int) -assert_type(ConditionalMethods.class_method(), int) -assert_type(ConditionalMethods().prop, int) -assert_type(ConditionalMethods().nested(), int) -assert_type(ConditionalMethods().loop_method(), int) -assert_type(ConditionalMethods().deferred(), int) - -async def use_async() -> None: - assert_type(await ConditionalMethods().async_method(), int) -"#, -); - -testcase!( - test_conditional_method_assignment_initialization, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -def enabled() -> bool: ... -def callback() -> int: - return 1 - -class CallableAttribute: - if enabled(): - value = callback # E: Attribute `value` may be uninitialized - -class ReassignedMethod: - if enabled(): - @staticmethod - def value() -> int: ... - value = callback # E: Attribute `value` may be uninitialized - -class MixedDefinitions: - if enabled(): - if enabled(): - @staticmethod - def value() -> int: ... - else: - value = callback # E: Attribute `value` may be uninitialized - -# Local function reads still require a definition on every reachable path. -def local() -> None: - if enabled(): - def value() -> int: - return 1 - value() # E: `value` may be uninitialized -"#, -); - // Test case for various edge cases where a name isn't in the flow, and we might // or might not decide an attribute has been defined. testcase!( diff --git a/pyrefly/lib/test/scope.rs b/pyrefly/lib/test/scope.rs index d145fb3afc..7c3897c358 100644 --- a/pyrefly/lib/test/scope.rs +++ b/pyrefly/lib/test/scope.rs @@ -1865,7 +1865,6 @@ def f(): // https://github.com/facebook/pyrefly/issues/4318 testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_conditionally_defined_class_member, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -1898,7 +1897,7 @@ class Instance: else: self.complete = 2 if coin(): - self.partial = 1 + self.partial = 1 # E: Attribute `partial` may be uninitialized class DynamicInstance: if coin(): @@ -1921,11 +1920,11 @@ class PartialWithReturn: def __init__(self) -> None: if coin(): return - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized class BareAnnotation: def __init__(self) -> None: - self.value: int + self.value: int # E: Attribute `value` may be uninitialized class CompleteWithNever: def __init__(self) -> None: From 28f8eb6358f332b0fa77b5801616206fa4bf11de Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 8 Oct 2026 13:25:38 +0900 Subject: [PATCH 02/10] keep init function's priority, and merge helper --- pyrefly/lib/binding/scope.rs | 45 ++++++++++++++-------------------- pyrefly/lib/test/attributes.rs | 4 +-- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index e8f3c9eff1..a702c70edb 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -3621,8 +3621,8 @@ impl Scopes { if existing_method.recognized_attribute_defining_method && !method.recognized_attribute_defining_method { - // Prioritization: Existing is from a recognized constructor, new is from an - // unrecognized helper method. The constructor wins, so ignore the new assignment. + // The constructor determines the attribute type, so the helper's values, + // annotation, and receiver kind do not affect the definition. } else { // Merge: Either both are constructors (e.g. __new__ and __init__), or both are // helper methods. We combine all their assignments. @@ -3635,30 +3635,23 @@ impl Scopes { if matches!(receiver_kind, MethodSelfKind::Class) { *existing_receiver = MethodSelfKind::Class; } - match (&mut *existing_initialized, initialized) { - (InitializedInFlow::Yes, _) | (_, InitializedInFlow::Yes) => { - *existing_initialized = InitializedInFlow::Yes; - } - ( - InitializedInFlow::DeferredCheck(existing), - InitializedInFlow::DeferredCheck(mut new), - ) => existing.append(&mut new), - ( - InitializedInFlow::DeferredCheck(_), - InitializedInFlow::Conditionally, - ) - | ( - InitializedInFlow::Conditionally, - InitializedInFlow::DeferredCheck(_), - ) - | ( - InitializedInFlow::Conditionally, - InitializedInFlow::Conditionally, - ) - | (InitializedInFlow::No, _) - | (_, InitializedInFlow::No) => { - *existing_initialized = InitializedInFlow::Conditionally; - } + } + // Initialization is merged independently of definition priority. + // An unconditional helper assignment also counts as initialization. + match (&mut *existing_initialized, initialized) { + (InitializedInFlow::Yes, _) | (_, InitializedInFlow::Yes) => { + *existing_initialized = InitializedInFlow::Yes; + } + ( + InitializedInFlow::DeferredCheck(existing), + InitializedInFlow::DeferredCheck(mut new), + ) => existing.append(&mut new), + (InitializedInFlow::DeferredCheck(_), InitializedInFlow::Conditionally) + | (InitializedInFlow::Conditionally, InitializedInFlow::DeferredCheck(_)) + | (InitializedInFlow::Conditionally, InitializedInFlow::Conditionally) + | (InitializedInFlow::No, _) + | (_, InitializedInFlow::No) => { + *existing_initialized = InitializedInFlow::Conditionally; } } } else if !field_definitions.contains_key_hashed(name.as_ref()) { diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 0f83d54374..ed93b8c9f3 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -187,9 +187,7 @@ def f(a: A): ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_conditional_attribute_initialized_in_helper, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import assert_type @@ -215,7 +213,7 @@ class HelperFirst: class ConditionalHelper: def __init__(self, cfg: int | None = None) -> None: if cfg is not None: - self.cfg = cfg + self.cfg = cfg # E: Attribute `cfg` may be uninitialized def _load_defaults(self, enabled: bool) -> None: if enabled: From c0fd825861b1300223e6eabb833b6a37e3fe14d7 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 8 Oct 2026 13:32:11 +0900 Subject: [PATCH 03/10] keep is fn def in flow info --- pyrefly/lib/binding/scope.rs | 23 +++++++- pyrefly/lib/test/attributes.rs | 97 ++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index a702c70edb..5584c18382 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -786,6 +786,9 @@ struct FlowInfo { struct FlowValue { idx: Idx, style: FlowStyle, + /// True if every reaching value comes from a function definition, including decorated ones. + /// This survives merges that replace `FunctionDef` with an initialization status. + is_function_def: bool, } /// The most recent narrow for a name. @@ -797,7 +800,11 @@ struct FlowNarrow { impl FlowInfo { fn new_value(idx: Idx, style: FlowStyle) -> Self { Self { - value: Some(FlowValue { idx, style }), + value: Some(FlowValue { + idx, + is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), + style, + }), narrow: None, narrow_depth: 0, loop_prior: idx, @@ -815,7 +822,11 @@ impl FlowInfo { fn updated_value(&self, idx: Idx, style: FlowStyle, in_loop: bool) -> Self { Self { - value: Some(FlowValue { idx, style }), + value: Some(FlowValue { + idx, + is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), + style, + }), // Note that any existing narrow is wiped when a new value is bound. narrow: None, narrow_depth: 0, @@ -2812,6 +2823,7 @@ impl Scopes { scope.flow.excluded_none_pairs.invalidate(name); if let Some(value) = scope.flow.get_value_mut(name) { value.style = FlowStyle::Uninitialized; + value.is_function_def = false; } scope.deleted_names.insert(name.clone()); } @@ -3584,6 +3596,8 @@ impl Scopes { }, }; let initialized = match &value.style { + // Conditional method definitions are exempt from attribute initialization checks. + _ if value.is_function_def => InitializedInFlow::Yes, FlowStyle::PossiblyUninitialized => InitializedInFlow::Conditionally, FlowStyle::MaybeInitialized(keys) => { InitializedInFlow::DeferredCheck(keys.clone()) @@ -4492,6 +4506,7 @@ impl<'a> BindingsBuilder<'a> { let mut branch_idxs = SmallSet::with_capacity(merge_branches.len()); let mut branch_infos = Vec::with_capacity(merge_branches.len()); let mut styles = Vec::with_capacity(merge_branches.len()); + let mut is_function_def = true; let mut styled_idxs = SmallSet::with_capacity(merge_branches.len()); let mut n_values = 0; let mut n_possibly_set_branches = 0; @@ -4555,6 +4570,7 @@ impl<'a> BindingsBuilder<'a> { if Some(v.idx) == phi_idx { continue; } + is_function_def &= v.is_function_def; value_idxs.insert(v.idx); // An invariant in Pyrefly is that we only set style when we // set a value, so duplicate value_idxs always have the same style. @@ -4631,6 +4647,7 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: *value_idxs.first().unwrap(), style: compute_final_style(styles), + is_function_def, }), narrow: Some(FlowNarrow { idx: merged_idx }), narrow_depth: 1, @@ -4652,6 +4669,7 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: merged_idx, style: compute_final_style(styles), + is_function_def, }), narrow: None, narrow_depth: 0, @@ -4814,6 +4832,7 @@ impl<'a> BindingsBuilder<'a> { info.value = Some(FlowValue { idx: phi_idx, style: FlowStyle::LoopRecursion, + is_function_def: false, }); info.narrow = None; } diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index ed93b8c9f3..a21b010c6d 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -33,6 +33,103 @@ Child.value "#, ); +testcase!( + test_conditionally_defined_methods, + r#" +from typing import assert_type + +def enabled() -> bool: ... +def fallback() -> None: ... + +class ConditionalMethods: + if enabled(): + def method(self) -> int: + return 1 + + async def async_method(self) -> int: + return 1 + + @staticmethod + def static_method() -> int: + return 1 + + @classmethod + def class_method(cls) -> int: + return 1 + + @property + def prop(self) -> int: + return 1 + + if enabled(): + def nested(self) -> int: + return 1 + else: + def nested(self) -> int: + return 2 + + while enabled(): + def loop_method(self) -> int: + return 1 + break + + if enabled(): + def deferred(self) -> int: + return 1 + else: + fallback() + + # A later merge must also preserve the method's definition source. + if enabled(): + data = 1 # E: Attribute `data` may be uninitialized + +assert_type(ConditionalMethods().method(), int) +assert_type(ConditionalMethods.static_method(), int) +assert_type(ConditionalMethods.class_method(), int) +assert_type(ConditionalMethods().prop, int) +assert_type(ConditionalMethods().nested(), int) +assert_type(ConditionalMethods().loop_method(), int) +assert_type(ConditionalMethods().deferred(), int) + +async def use_async() -> None: + assert_type(await ConditionalMethods().async_method(), int) +"#, +); + +testcase!( + test_conditional_method_assignment_initialization, + r#" +def enabled() -> bool: ... +def callback() -> int: + return 1 + +class CallableAttribute: + if enabled(): + value = callback # E: Attribute `value` may be uninitialized + +class ReassignedMethod: + if enabled(): + @staticmethod + def value() -> int: ... + value = callback # E: Attribute `value` may be uninitialized + +class MixedDefinitions: + if enabled(): + if enabled(): + @staticmethod + def value() -> int: ... + else: + value = callback # E: Attribute `value` may be uninitialized + +# Local function reads still require a definition on every reachable path. +def local() -> None: + if enabled(): + def value() -> int: + return 1 + value() # E: `value` may be uninitialized +"#, +); + // Test case for various edge cases where a name isn't in the flow, and we might // or might not decide an attribute has been defined. testcase!( From e5c0d77045b7bba70e25cc9bcb6712a3ce7d7edc Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 8 Oct 2026 13:41:19 +0900 Subject: [PATCH 04/10] fix `and` `or` merge --- pyrefly/lib/alt/solve.rs | 8 +-- pyrefly/lib/binding/binding.rs | 12 ++--- pyrefly/lib/binding/bindings.rs | 19 +++++++ pyrefly/lib/binding/class.rs | 47 ++++++++---------- pyrefly/lib/binding/django.rs | 4 +- pyrefly/lib/binding/scope.rs | 37 +++++++------- pyrefly/lib/test/attributes.rs | 87 ++------------------------------- 7 files changed, 75 insertions(+), 139 deletions(-) diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index d6ba8907a6..7410828dbf 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3212,11 +3212,11 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { name, range, getattr_class, - termination_keys, + termination_key_groups, } => { - let may_be_uninitialized = termination_keys - .as_ref() - .is_none_or(|keys| !keys.iter().all(|key| self.get_idx(*key).ty().is_never())); + let may_be_uninitialized = !termination_key_groups + .iter() + .any(|keys| keys.iter().all(|key| self.get_idx(*key).ty().is_never())); let has_getattr_fallback = getattr_class.is_some_and(|class_idx| { self.get_idx(class_idx) .0 diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 7b06a2ccce..6d2bfdebd6 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1366,16 +1366,16 @@ pub enum BindingExpect { /// If any don't, the variable may be uninitialized. termination_keys: Vec>, }, - /// Check that a class attribute is initialized on every control-flow path. + /// Check attribute initialization from the class body or any defining method. UninitializedAttributeCheck { name: Name, range: TextRange, /// The owning class when this is an instance attribute. Its `__getattr__` may make a /// conditionally initialized attribute safe to access. getattr_class: Option>, - /// `None` means binding proved the declaration conditional. Otherwise, initialization - /// depends on whether all of these branches terminate with `Never`. - termination_keys: Option>>, + /// Initialization requires all keys in at least one group to have Never type. + /// No groups means the attribute may be uninitialized without any deferred conditions. + termination_key_groups: Vec>>, }, /// Check for forward reference string literal in union type. /// At runtime, `type.__or__` cannot handle string literals, so expressions @@ -1552,7 +1552,7 @@ impl DisplayWith for BindingExpect { name, range, getattr_class, - termination_keys, + termination_key_groups, } => { write!( f, @@ -1560,7 +1560,7 @@ impl DisplayWith for BindingExpect { name, ctx.module().display(range), getattr_class, - termination_keys + termination_key_groups ) } Self::ForwardRefUnion { diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index 49fc7a787f..ae8a262a18 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -214,6 +214,25 @@ impl InitializedInFlow { } } +#[derive(Debug, Clone)] +pub enum InitializedInClass { + Yes, + /// Each group contains the termination keys required by one defining flow. + /// Initialization requires all keys in any one group to have Never type. + /// An empty list of groups means no flow guarantees initialization. + IfAny(Vec>>), +} + +impl From for InitializedInClass { + fn from(initialized: InitializedInFlow) -> Self { + match initialized { + InitializedInFlow::Yes => Self::Yes, + InitializedInFlow::Conditionally | InitializedInFlow::No => Self::IfAny(Vec::new()), + InitializedInFlow::DeferredCheck(keys) => Self::IfAny(vec![keys]), + } + } +} + pub type BindingEntry = (Index, IndexMap::Value>); table! { diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 319f12d37d..41daf35e37 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -76,7 +76,7 @@ use crate::binding::binding::KeyVariance; use crate::binding::binding::MethodSelfKind; use crate::binding::bindings::BindingsBuilder; use crate::binding::bindings::CurrentIdx; -use crate::binding::bindings::InitializedInFlow; +use crate::binding::bindings::InitializedInClass; use crate::binding::bindings::LegacyTParamCollector; use crate::binding::expr::Usage; use crate::binding::pydantic::PydanticConfigDict; @@ -516,31 +516,26 @@ impl<'a> BindingsBuilder<'a> { ClassFieldDefinition::DefinedInMethod { method, .. } if !method.recognized_attribute_defining_method ); - if check_initialization { - let termination_keys = match &initialized { - InitializedInFlow::Yes => None, - InitializedInFlow::Conditionally | InitializedInFlow::No => Some(None), - InitializedInFlow::DeferredCheck(keys) => Some(Some(keys.clone())), - }; - if let Some(termination_keys) = termination_keys { - let getattr_class = matches!( - &definition, - ClassFieldDefinition::DefinedInMethod { - receiver_kind: MethodSelfKind::Instance, - .. - } - ) - .then_some(class_indices.class_idx); - self.insert_binding( - KeyExpect::UninitializedAttributeCheck(range), - BindingExpect::UninitializedAttributeCheck { - name: name.key().clone(), - range, - getattr_class, - termination_keys, - }, - ); - } + if check_initialization + && let InitializedInClass::IfAny(termination_key_groups) = initialized + { + let getattr_class = matches!( + &definition, + ClassFieldDefinition::DefinedInMethod { + receiver_kind: MethodSelfKind::Instance, + .. + } + ) + .then_some(class_indices.class_idx); + self.insert_binding( + KeyExpect::UninitializedAttributeCheck(range), + BindingExpect::UninitializedAttributeCheck { + name: name.key().clone(), + range, + getattr_class, + termination_key_groups, + }, + ); } fields.insert_hashed( diff --git a/pyrefly/lib/binding/django.rs b/pyrefly/lib/binding/django.rs index f0c87a2c27..3b2b67f83e 100644 --- a/pyrefly/lib/binding/django.rs +++ b/pyrefly/lib/binding/django.rs @@ -13,7 +13,7 @@ use starlark_map::small_map::SmallMap; use crate::binding::binding::ClassFieldDefinition; use crate::binding::binding::ExprOrBinding; use crate::binding::bindings::BindingsBuilder; -use crate::binding::bindings::InitializedInFlow; +use crate::binding::bindings::InitializedInClass; const PRIMARY_KEY: Name = Name::new_static("primary_key"); const FOREIGN_KEY: Name = Name::new_static("ForeignKey"); @@ -40,7 +40,7 @@ impl<'a> BindingsBuilder<'a> { /// (primary_key, ForeignKey, choices). pub fn extract_django_fields_from_class_body( &self, - field_definitions: &SmallMap, + field_definitions: &SmallMap, ) -> DjangoFieldInfo { let mut primary_key_field = None; let mut foreign_key_like_fields = Vec::new(); diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 5584c18382..d28f783bde 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -71,6 +71,7 @@ use crate::binding::binding::SuppressedException; use crate::binding::bindings::BindingTable; use crate::binding::bindings::BindingsBuilder; use crate::binding::bindings::CurrentIdx; +use crate::binding::bindings::InitializedInClass; use crate::binding::bindings::InitializedInFlow; use crate::binding::classmethod::ClassmethodReceiverState; use crate::binding::expr::Usage; @@ -2354,6 +2355,8 @@ impl Scopes { Some(InitializedInFlow::DeferredCheck(mut existing)), InitializedInFlow::DeferredCheck(mut new), ) => { + // All return paths must initialize the attribute. + // Combine their requirements with AND. existing.append(&mut new); InitializedInFlow::DeferredCheck(existing) } @@ -3482,7 +3485,7 @@ impl Scopes { pub fn finish_class_and_get_field_definitions( &mut self, ) -> ( - SmallMap, + SmallMap, NNModuleRegistrations, ) { let mut field_definitions = SmallMap::new(); @@ -3604,8 +3607,10 @@ impl Scopes { } _ => InitializedInFlow::Yes, }; - field_definitions - .insert_hashed(name.owned(), (definition, static_info.range, initialized)); + field_definitions.insert_hashed( + name.owned(), + (definition, static_info.range, initialized.into()), + ); } }); // Merge assignments from different methods. @@ -3651,22 +3656,20 @@ impl Scopes { } } // Initialization is merged independently of definition priority. - // An unconditional helper assignment also counts as initialization. + // Any method can establish initialization, so combine their requirements with OR. match (&mut *existing_initialized, initialized) { - (InitializedInFlow::Yes, _) | (_, InitializedInFlow::Yes) => { - *existing_initialized = InitializedInFlow::Yes; + (InitializedInClass::Yes, _) => {} + (_, InitializedInFlow::Yes) => { + *existing_initialized = InitializedInClass::Yes; } ( - InitializedInFlow::DeferredCheck(existing), - InitializedInFlow::DeferredCheck(mut new), - ) => existing.append(&mut new), - (InitializedInFlow::DeferredCheck(_), InitializedInFlow::Conditionally) - | (InitializedInFlow::Conditionally, InitializedInFlow::DeferredCheck(_)) - | (InitializedInFlow::Conditionally, InitializedInFlow::Conditionally) - | (InitializedInFlow::No, _) - | (_, InitializedInFlow::No) => { - *existing_initialized = InitializedInFlow::Conditionally; - } + InitializedInClass::IfAny(groups), + InitializedInFlow::DeferredCheck(keys), + ) => groups.push(keys), + ( + InitializedInClass::IfAny(_), + InitializedInFlow::Conditionally | InitializedInFlow::No, + ) => {} } } else if !field_definitions.contains_key_hashed(name.as_ref()) { field_definitions.insert_hashed( @@ -3683,7 +3686,7 @@ impl Scopes { ), }, range, - initialized, + initialized.into(), ), ); } diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index a21b010c6d..d946bd1cbe 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -345,9 +345,7 @@ def f(a: A): ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_attribute_initialization_deferred_methods, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import Never @@ -385,7 +383,7 @@ class HelperTerminates: class NeitherTerminates: def __init__(self, flag: bool) -> None: if flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized else: proceed() @@ -420,86 +418,7 @@ class ConditionalConstructor: ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", - test_attribute_initialization_loops, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), - r#" -from typing import Never, assert_type - -def stop() -> Never: - raise RuntimeError() - -class BeforeLoop: - def __init__(self, values: list[int]) -> None: - self.value = 0 - for value in values: - self.value = value - -class PossiblyEmpty: - def __init__(self, values: list[int]) -> None: - for value in values: - self.value = value - -class NonEmpty: - def __init__(self) -> None: - for value in [1]: - self.value = value - -class NoReturningLoop: - def __init__(self) -> None: - self.value = 0 - for value in [1]: - raise RuntimeError() - -class ConditionalContinue: - def __init__(self, flag: bool) -> None: - for value in [1]: - if flag: - continue - self.value = value - -class ConditionalBreak: - def __init__(self, flag: bool) -> None: - while True: - if flag: - break - self.value = 1 - break - -class LoopElse: - def __init__(self, values: list[int]) -> None: - for value in values: - self.value = value - else: - self.value = 0 - -class DeferredBeforeLoop: - def __init__(self, flag: bool, values: list[int]) -> None: - if flag: - self.value = 1 - else: - stop() - for value in values: - pass - -class DeferredInLoop: - def __init__(self, flag: bool) -> None: - for value in [1]: - if flag: - self.value = value - else: - stop() - -assert_type(BeforeLoop([]).value, int) -assert_type(NonEmpty().value, int) -assert_type(LoopElse([]).value, int) -"#, -); - -testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_attribute_initialization_deferred_return_paths, - TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import Never @@ -525,7 +444,7 @@ class EarlyPathContinues: def __init__(self, early: bool, flag: bool) -> None: if early: if flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized else: proceed() return @@ -538,7 +457,7 @@ class FinalPathContinues: def __init__(self, early: bool, flag: bool) -> None: if early: if flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized else: stop() return From f1dd90019615f4ed86ee8afaea5c2d9c6eb69e8c Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 8 Oct 2026 13:47:49 +0900 Subject: [PATCH 05/10] new error msg --- crates/pyrefly_config/src/error_kind.rs | 2 +- pyrefly/lib/alt/solve.rs | 2 +- pyrefly/lib/test/attributes.rs | 37 +++++++++++++++++++++++++ website/docs/error-kinds.mdx | 20 ++++++------- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index bcfa01379b..589fb61748 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -377,7 +377,7 @@ pub enum ErrorKind { /// A value whose type is not itself `Any` but contains an implicit `Any`, /// such as `list[Unknown]` or `Unknown | None`. PartiallyUnknownType, - /// A class-body attribute may be uninitialized on some control-flow paths. + /// A class or instance attribute may be uninitialized on some control-flow paths. PossiblyUninitializedAttribute, /// A potential conflict between an explicit keyword argument and a NotRequired /// TypedDict field. The field may be absent at runtime, so the conflict is not diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 7410828dbf..81ec21b4cb 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3227,7 +3227,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { errors .error_builder( *range, - ErrorKind::MissingAttribute, + ErrorKind::PossiblyUninitializedAttribute, format!("Attribute `{name}` may be uninitialized"), ) .emit(); diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index d946bd1cbe..98fcc4b4ac 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -10,6 +10,37 @@ use crate::test::util::TestEnv; use crate::testcase; +testcase!( + test_possibly_uninitialized_attribute_default_disabled, + r#" +def enabled() -> bool: ... + +class C: + if enabled(): + class_value = 1 + + def __init__(self, flag: bool) -> None: + if flag: + self.value = 1 + +C(True).missing # E: Object of class `C` has no attribute `missing` +"#, +); + +testcase!( + test_possibly_uninitialized_attribute_suppression, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +class C: + def __init__(self, flag: bool) -> None: + if flag: + self.suppressed = 1 # pyrefly: ignore[possibly-uninitialized-attribute] + self.reported = 1 # pyrefly: ignore[missing-attribute] # E: Attribute `reported` may be uninitialized + +C(True).missing # pyrefly: ignore[possibly-uninitialized-attribute] # E: Object of class `C` has no attribute `missing` +"#, +); + fn env_with_conditional_attribute() -> TestEnv { TestEnv::one( "foo", @@ -20,6 +51,7 @@ class Base: value = 1 # E: Attribute `value` may be uninitialized "#, ) + .enable_possibly_uninitialized_attribute_error() } testcase!( @@ -35,6 +67,7 @@ Child.value testcase!( test_conditionally_defined_methods, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import assert_type @@ -98,6 +131,7 @@ async def use_async() -> None: testcase!( test_conditional_method_assignment_initialization, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" def enabled() -> bool: ... def callback() -> int: @@ -285,6 +319,7 @@ def f(a: A): testcase!( test_conditional_attribute_initialized_in_helper, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import assert_type @@ -346,6 +381,7 @@ def f(a: A): testcase!( test_attribute_initialization_deferred_methods, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import Never @@ -419,6 +455,7 @@ class ConditionalConstructor: testcase!( test_attribute_initialization_deferred_return_paths, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" from typing import Never diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 500e451de8..2b66d738be 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1873,22 +1873,19 @@ Default severity: `ignore` The `strict` preset enables this check as an error. -Pyrefly checks whether each value assigned in a class body exists when that body -completes. Every completing path must assign the attribute. A path that raises or -ends in a `Never` or `NoReturn` call does not require an assignment. +An attribute may be uninitialized after class-body execution or instance initialization. +Pyrefly reports this error at the attribute definition, even if the attribute is never read. +A later `hasattr` guard does not suppress this check. +Conditional method definitions are excluded. ```python class C: - if enabled(): - value = 1 # possibly-uninitialized-attribute + def __init__(self, value: int | None) -> None: + if value is not None: + self.value = value # possibly-uninitialized-attribute ``` -Pyrefly reports the error at the definition, even if the attribute is never read. -A later `hasattr` guard does not suppress it. An assignment in `__init__` does not -initialize a class attribute. Inherited attributes, conditional method definitions, -and stub (`.pyi`) files are exempt. - -To disable this check while retaining other strict checks, use: +To disable this check while keeping other strict checks, use: ```toml preset = "strict" @@ -1898,6 +1895,7 @@ possibly-uninitialized-attribute = "ignore" ``` This setting does not disable [`missing-attribute`](#missing-attribute). +The separate [`uninitialized-instance-variable`](#uninitialized-instance-variable) check reports class-body instance declarations that have no initialization. ## potential-bad-keyword-argument From 4987f3848464e5ad5f3ddb6d6f4ec9bdca346281 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 8 Oct 2026 13:50:20 +0900 Subject: [PATCH 06/10] skip pyi check --- pyrefly/lib/binding/class.rs | 4 +++- pyrefly/lib/test/attributes.rs | 43 ++++++++++++++++++++++++++++++++++ website/docs/error-kinds.mdx | 1 + 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 41daf35e37..a08e618145 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -516,7 +516,9 @@ impl<'a> BindingsBuilder<'a> { ClassFieldDefinition::DefinedInMethod { method, .. } if !method.recognized_attribute_defining_method ); - if check_initialization + // Stub files declare attributes without runtime initialization. + if !self.module_info.path().is_interface() + && check_initialization && let InitializedInClass::IfAny(termination_key_groups) = initialized { let getattr_class = matches!( diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 98fcc4b4ac..e7003237f2 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -41,6 +41,49 @@ C(True).missing # pyrefly: ignore[possibly-uninitialized-attribute] # E: Objec "#, ); +testcase!( + test_possibly_uninitialized_attribute_stub, + TestEnv::one_with_path( + "foo", + "foo.pyi", + r#" +def enabled() -> bool: ... + +class Stub: + if enabled(): + value = 1 + annotated: int = 1 + + def __init__(self, flag: bool) -> None: + if flag: + self.instance_value = 1 +"#, + ) + .enable_possibly_uninitialized_attribute_error(), + r#" +from typing import assert_type +from foo import Stub + +assert_type(Stub.value, int) +assert_type(Stub.annotated, int) +assert_type(Stub(True).instance_value, int) + +class Child(Stub): ... + +assert_type(Child.value, int) + +def enabled() -> bool: ... + +class Implementation: + if enabled(): + value = 1 # E: Attribute `value` may be uninitialized + + def __init__(self, flag: bool) -> None: + if flag: + self.instance_value = 1 # E: Attribute `instance_value` may be uninitialized +"#, +); + fn env_with_conditional_attribute() -> TestEnv { TestEnv::one( "foo", diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 2b66d738be..6f261ace38 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1877,6 +1877,7 @@ An attribute may be uninitialized after class-body execution or instance initial Pyrefly reports this error at the attribute definition, even if the attribute is never read. A later `hasattr` guard does not suppress this check. Conditional method definitions are excluded. +This check does not apply to stub (`.pyi`) files because they do not perform runtime initialization. ```python class C: From 799e4cea907be88aeb611890e5eb2d3e6fec3197 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 8 Oct 2026 14:43:58 +0900 Subject: [PATCH 07/10] add bench, fix some perf fix comments --- pyrefly/Cargo.toml | 4 + pyrefly/benches/README.md | 27 ++ pyrefly/benches/attribute_initialization.rs | 103 +++++ pyrefly/benches/micro.rs | 44 ++ pyrefly/lib/alt/solve.rs | 31 +- pyrefly/lib/binding/binding.rs | 24 +- pyrefly/lib/binding/bindings.rs | 88 +++- pyrefly/lib/binding/class.rs | 48 ++- pyrefly/lib/binding/function.rs | 18 +- pyrefly/lib/binding/scope.rs | 435 ++++++++------------ pyrefly/lib/binding/stmt.rs | 22 +- pyrefly/lib/binding/target.rs | 10 +- pyrefly/lib/test/attributes.rs | 396 ++++++++++++++++++ website/docs/error-kinds.mdx | 23 +- 14 files changed, 943 insertions(+), 330 deletions(-) create mode 100644 pyrefly/benches/attribute_initialization.rs diff --git a/pyrefly/Cargo.toml b/pyrefly/Cargo.toml index 5b684701d7..1ffdcb4f52 100644 --- a/pyrefly/Cargo.toml +++ b/pyrefly/Cargo.toml @@ -28,6 +28,10 @@ harness = false name = "micro" harness = false +[[bench]] +name = "attribute_initialization" +harness = false + [[bench]] name = "pytorch" harness = false diff --git a/pyrefly/benches/README.md b/pyrefly/benches/README.md index 992df594f6..88ddea624b 100644 --- a/pyrefly/benches/README.md +++ b/pyrefly/benches/README.md @@ -135,6 +135,33 @@ lands at the fbsource repo root `target/criterion/`. It is throwaway — do not commit it, and do not add repo-root ignore entries (it is already gitignored appropriately). +## Attribute initialization + +These synthetic benchmarks measure the cost of tracking instance attribute +initialization. They do not require a PyTorch checkout. + +- `micro` includes the `attribute_initialization` group. It varies the number + of attributes and branch joins, loop joins, or early returns independently. + Each iteration checks an in-memory module with the standard library already loaded. +- `attribute_initialization` drives a warm LSP server with three worker threads. + It edits a file containing a constructor with 64 attributes and 64 loops with branches, + then waits for diagnostics. Each edit alternates between zero and one error + to ensure that the response belongs to the new edit. Initialization checks + are enabled. Server setup and the first check are outside the measured region. + +Build both targets before measuring. Run them sequentially, with no concurrent +builds or tests: + +```bash +cargo bench -p pyrefly --bench micro --bench attribute_initialization --no-run +cargo bench -p pyrefly --bench micro -- attribute_initialization --noplot +cargo bench -p pyrefly --bench attribute_initialization -- --noplot +``` + +Use Criterion's `--save-baseline before` on the original version and +`--baseline before` on the changed version to compare results. Synthetic +results isolate this code path. They do not establish whole-project LSP performance. + ## TSP benchmark `benches/tsp/` -- buck `tsp_bench`, cargo bench `tsp`, Criterion id diff --git a/pyrefly/benches/attribute_initialization.rs b/pyrefly/benches/attribute_initialization.rs new file mode 100644 index 0000000000..bbe1f44919 --- /dev/null +++ b/pyrefly/benches/attribute_initialization.rs @@ -0,0 +1,103 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +//! Measure edits to a file containing a constructor with many attributes and joins. +//! Server initialization and the first check are outside the measured region. + +use std::fmt::Write as _; +use std::fs; + +use criterion::Criterion; +use criterion::criterion_group; +use criterion::criterion_main; +use lsp_types::DidChangeTextDocumentNotification; +use lsp_types::Uri; +use pyrefly::commands::lsp::IndexingMode; +use pyrefly::commands::lsp::LspArgs; +use pyrefly_lsp_test::object_model::InitializeSettings; +use pyrefly_lsp_test::object_model::LspInteraction; +use pyrefly_lsp_test::object_model::LspInteractionArgs; +use pyrefly_util::telemetry::NoTelemetry; +use pyrefly_util::thread_pool::TEST_THREAD_COUNT; +use serde_json::json; +use tempfile::tempdir; + +fn attribute_initialization_edit(c: &mut Criterion) { + let dir = tempdir().unwrap(); + fs::write( + dir.path().join("pyrefly.toml"), + "[errors]\npossibly-uninitialized-attribute = 'error'\n", + ) + .unwrap(); + let path = dir.path().join("constructor.py"); + let mut source = + String::from("class C:\n def __init__(self, flag: bool, values: list[int]) -> None:\n"); + for i in 0..64 { + let _ = writeln!(source, " self.attribute_{i}: int = 0"); + } + for i in 0..64 { + let _ = writeln!( + source, + " for value in values:\n if flag:\n self.attribute_{i} = value" + ); + } + let with_error = format!("{source}\nresult: int = 'error'\n"); + fs::write(&path, &source).unwrap(); + let mut interaction = LspInteraction::new_with_args(LspInteractionArgs { + args: LspArgs { + indexing_mode: IndexingMode::None, + workspace_indexing_limit: 50, + build_system_blocking: false, + }, + telemetry: Box::new(NoTelemetry), + thread_count: TEST_THREAD_COUNT, + thrift_remapper: None, + }); + interaction.set_root(dir.path().to_path_buf()); + interaction + .initialize(InitializeSettings { + configuration: Some(Some(json!([ + {"pyrefly": {"displayTypeErrors": "force-on"}} + ]))), + ..Default::default() + }) + .unwrap(); + interaction.client.did_open("constructor.py"); + interaction + .client + .expect_publish_diagnostics_eventual_error_count(path.clone(), 0) + .unwrap(); + + let mut version = 1; + c.bench_function("attribute_initialization/edit_to_diagnostics", |b| { + b.iter(|| { + version += 1; + let has_error = version % 2 == 0; + let text = if has_error { &with_error } else { &source }; + interaction + .client + .send_notification::(json!({ + "textDocument": { + "uri": Uri::from_file_path(&path).unwrap(), + "version": version, + }, + "contentChanges": [{"text": text}], + })); + // Alternating the error count ensures that each edit completes. + interaction + .client + .expect_publish_diagnostics_eventual_error_count( + path.clone(), + usize::from(has_error), + ) + .unwrap(); + }); + }); +} + +criterion_group!(benches, attribute_initialization_edit); +criterion_main!(benches); diff --git a/pyrefly/benches/micro.rs b/pyrefly/benches/micro.rs index 5e76cfac0e..cdb4a1b13e 100644 --- a/pyrefly/benches/micro.rs +++ b/pyrefly/benches/micro.rs @@ -453,6 +453,49 @@ fn measure_module( } /// Smoke benchmark validating the harness end-to-end. +/// Attribute initialization must track branches without constructing unused type joins. +fn attribute_initialization(c: &mut Criterion) { + let mut group = c.benchmark_group("attribute_initialization"); + for (attributes, joins) in [(16, 16), (64, 16), (16, 64), (64, 64)] { + for shape in ["branches", "loops", "returns"] { + let mut source = String::from( + "class C:\n def __init__(self, flag: bool, values: list[int]) -> None:\n", + ); + for i in 0..attributes { + let _ = writeln!(source, " self.attribute_{i}: int = 0"); + } + for i in 0..joins { + let attribute = i % attributes; + match shape { + "branches" => { + let _ = writeln!( + source, + " if flag:\n self.attribute_{attribute} = {i}\n else:\n self.attribute_{attribute} = -{i}" + ); + } + "loops" => { + let _ = writeln!( + source, + " for value in values:\n self.attribute_{attribute} = value" + ); + } + "returns" => { + let _ = writeln!(source, " if flag:\n return"); + } + _ => unreachable!("all benchmark shapes are listed above"), + } + } + let name = format!("{shape}_{attributes}_attributes_{joins}_joins"); + let code = Arc::new(FileContents::from_source(source)); + assert_eq!(check_module(code.dupe(), "bench", BENCH_FILE), 0, "{name}"); + group.bench_function(name, |b| { + b.iter(|| check_module(code.dupe(), "bench", BENCH_FILE)) + }); + } + } + group.finish(); +} + fn smoke(c: &mut Criterion) { measure(c, "smoke", "x: int = 1".to_owned(), 0); } @@ -957,6 +1000,7 @@ fn suggestion_nested(c: &mut Criterion) { criterion_group!( benches, + attribute_initialization, smoke, enum_members, enum_value_access, diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 81ec21b4cb..ee559c9f16 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3211,19 +3211,28 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { BindingExpect::UninitializedAttributeCheck { name, range, - getattr_class, - termination_key_groups, + class, + is_instance, + initializations, } => { - let may_be_uninitialized = !termination_key_groups - .iter() - .any(|keys| keys.iter().all(|key| self.get_idx(*key).ty().is_never())); - let has_getattr_fallback = getattr_class.is_some_and(|class_idx| { - self.get_idx(class_idx) - .0 - .as_ref() - .is_some_and(|cls| self.get_class_member(cls, &dunder::GETATTR).is_some()) + let initialized = initializations.iter().any(|initialized| { + initialized.is_initialized(|key| self.get_idx(key).ty().is_never()) }); - if may_be_uninitialized && !has_getattr_fallback { + let inherited_or_dynamic = !initialized + && self.get_idx(*class).0.as_ref().is_some_and(|cls| { + (*is_instance && self.get_class_member(cls, &dunder::GETATTR).is_some()) + || self + .get_mro_for_class(cls) + .ancestors_no_object() + .iter() + .any(|base| { + base.class_object() != cls + && self + .get_class_member(base.class_object(), name) + .is_some() + }) + }); + if !initialized && !inherited_or_dynamic { errors .error_builder( *range, diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 6d2bfdebd6..4b6bfc2d06 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -77,6 +77,7 @@ use crate::binding::base_class::BaseClassGeneric; use crate::binding::bindings::BindingEntry; use crate::binding::bindings::BindingTable; use crate::binding::bindings::Bindings; +use crate::binding::bindings::InitializationStatus; use crate::binding::classmethod::MethodReceiverKind; use crate::binding::django::DjangoFieldInfo; use crate::binding::narrow::NarrowOp; @@ -1370,12 +1371,11 @@ pub enum BindingExpect { UninitializedAttributeCheck { name: Name, range: TextRange, - /// The owning class when this is an instance attribute. Its `__getattr__` may make a - /// conditionally initialized attribute safe to access. - getattr_class: Option>, - /// Initialization requires all keys in at least one group to have Never type. - /// No groups means the attribute may be uninitialized without any deferred conditions. - termination_key_groups: Vec>>, + class: Idx, + /// Instance attributes can also be provided by `__getattr__`. + is_instance: bool, + /// Any defining method can establish initialization on all its returning paths. + initializations: Vec, }, /// Check for forward reference string literal in union type. /// At runtime, `type.__or__` cannot handle string literals, so expressions @@ -1551,16 +1551,18 @@ impl DisplayWith for BindingExpect { Self::UninitializedAttributeCheck { name, range, - getattr_class, - termination_key_groups, + class, + is_instance, + initializations, } => { write!( f, - "UninitializedAttributeCheck({}, {}, {:?}, {:?})", + "UninitializedAttributeCheck({}, {}, {:?}, {}, {:?})", name, ctx.module().display(range), - getattr_class, - termination_key_groups + class, + is_instance, + initializations ) } Self::ForwardRefUnion { diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index ae8a262a18..bc06c8c0cc 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -214,21 +214,91 @@ impl InitializedInFlow { } } +/// Initialization on every path through a control-flow merge. +#[derive(Debug, Clone)] +pub enum InitializationStatus { + Initialized, + Uninitialized, + /// Every missing path must terminate. Any key in a path can prove termination. + Deferred(Vec>>), +} + +impl InitializationStatus { + /// Combine paths with AND. A trailing Never expression can terminate a missing path. + pub fn merge<'a>( + paths: impl IntoIterator, Option>)>, + ) -> Self { + let mut conditions = Vec::new(); + for (initialized, termination_key) in paths { + match initialized { + Some(Self::Initialized) => {} + Some(Self::Deferred(paths)) => { + for path in paths { + let mut path = path.clone(); + if let Some(key) = termination_key { + path.push(key); + } + path.sort_unstable(); + path.dedup(); + conditions.push(path); + } + } + None | Some(Self::Uninitialized) => { + if let Some(key) = termination_key { + conditions.push(vec![key]); + } else { + return Self::Uninitialized; + } + } + } + } + if conditions.is_empty() { + Self::Initialized + } else { + // Forks can copy the same requirement into several paths. + conditions.sort_unstable(); + conditions.dedup(); + Self::Deferred(conditions) + } + } + + pub fn is_initialized(&self, is_never: impl Fn(Idx) -> bool) -> bool { + match self { + Self::Initialized => true, + Self::Uninitialized => false, + Self::Deferred(paths) => paths + .iter() + .all(|path| path.iter().any(|key| is_never(*key))), + } + } +} + +impl From for InitializationStatus { + fn from(initialized: InitializedInFlow) -> Self { + match initialized { + InitializedInFlow::Yes => Self::Initialized, + InitializedInFlow::Conditionally | InitializedInFlow::No => Self::Uninitialized, + InitializedInFlow::DeferredCheck(keys) => { + Self::Deferred(keys.into_iter().map(|key| vec![key]).collect()) + } + } + } +} + #[derive(Debug, Clone)] pub enum InitializedInClass { Yes, - /// Each group contains the termination keys required by one defining flow. - /// Initialization requires all keys in any one group to have Never type. - /// An empty list of groups means no flow guarantees initialization. - IfAny(Vec>>), + /// Any defining method or the class body can establish initialization. + /// An empty list means no flow guarantees initialization. + IfAny(Vec), } -impl From for InitializedInClass { - fn from(initialized: InitializedInFlow) -> Self { +impl From for InitializedInClass { + fn from(initialized: InitializationStatus) -> Self { match initialized { - InitializedInFlow::Yes => Self::Yes, - InitializedInFlow::Conditionally | InitializedInFlow::No => Self::IfAny(Vec::new()), - InitializedInFlow::DeferredCheck(keys) => Self::IfAny(vec![keys]), + InitializationStatus::Initialized => Self::Yes, + InitializationStatus::Uninitialized => Self::IfAny(Vec::new()), + deferred @ InitializationStatus::Deferred(_) => Self::IfAny(vec![deferred]), } } } diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index a08e618145..a61043bb53 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -31,6 +31,7 @@ use ruff_python_ast::name::Name; use ruff_text_size::Ranged; use ruff_text_size::TextRange; use starlark_map::small_map::SmallMap; +use starlark_map::small_set::SmallSet; use crate::binding::attrs::AttrsDecoratorMethods; use crate::binding::attrs::collect_attrs_decorator_methods; @@ -516,26 +517,61 @@ impl<'a> BindingsBuilder<'a> { ClassFieldDefinition::DefinedInMethod { method, .. } if !method.recognized_attribute_defining_method ); + // Inspect definition bindings here so method provenance does not occupy every flow value. + let is_conditional_method = + if let ClassFieldDefinition::DefinedWithoutAssign { definition } = &definition { + let mut pending = vec![*definition]; + let mut seen = SmallSet::new(); + let mut found_function = false; + let mut only_functions = true; + while let Some(idx) = pending.pop() { + if !seen.insert(idx) { + continue; + } + match self + .get_original_binding(idx) + .and_then(|(_, binding)| binding) + { + Some(Binding::Function { .. }) => found_function = true, + Some(Binding::Phi(_, branches)) => { + pending.extend(branches.iter().map(|branch| branch.value_key)); + } + Some(Binding::Narrow(prior, _, _)) => pending.push(*prior), + Some(Binding::LoopPhi(phi)) => { + pending.push(phi.0); + pending.extend(phi.1.iter().copied()); + } + _ => { + only_functions = false; + break; + } + } + } + found_function && only_functions + } else { + false + }; // Stub files declare attributes without runtime initialization. if !self.module_info.path().is_interface() && check_initialization - && let InitializedInClass::IfAny(termination_key_groups) = initialized + && !is_conditional_method + && let InitializedInClass::IfAny(initializations) = initialized { - let getattr_class = matches!( + let is_instance = matches!( &definition, ClassFieldDefinition::DefinedInMethod { receiver_kind: MethodSelfKind::Instance, .. } - ) - .then_some(class_indices.class_idx); + ); self.insert_binding( KeyExpect::UninitializedAttributeCheck(range), BindingExpect::UninitializedAttributeCheck { name: name.key().clone(), range, - getattr_class, - termination_key_groups, + class: class_indices.class_idx, + is_instance, + initializations, }, ); } diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index 5ac733f250..557e5da5da 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -69,7 +69,7 @@ use crate::binding::binding::ReturnImplicit; use crate::binding::binding::ReturnType; use crate::binding::binding::ReturnTypeKind; use crate::binding::bindings::BindingsBuilder; -use crate::binding::bindings::InitializedInFlow; +use crate::binding::bindings::InitializationStatus; use crate::binding::bindings::LegacyTParamCollector; use crate::binding::classmethod::FunctionDecorators; use crate::binding::expr::Usage; @@ -174,7 +174,7 @@ impl<'a, 'b> SourceOrderVisitor<'a> for SuperMethodCallFinder<'b> { pub struct SelfAssignments { pub method_name: Name, - pub instance_attributes: SmallMap, + pub instance_attributes: SmallMap, pub nn_module_registrations: NNModuleRegistrations, } @@ -267,12 +267,14 @@ impl<'a> SelfAttrNames<'a> { .map(|(n, r)| { ( n, - InstanceAttribute( - vec![ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit))], - None, - r, - MethodSelfKind::Instance, - InitializedInFlow::Yes, + ( + InstanceAttribute( + vec![ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit))], + None, + r, + MethodSelfKind::Instance, + ), + InitializationStatus::Initialized, ), ) }) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index d28f783bde..285300af05 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -71,6 +71,7 @@ use crate::binding::binding::SuppressedException; use crate::binding::bindings::BindingTable; use crate::binding::bindings::BindingsBuilder; use crate::binding::bindings::CurrentIdx; +use crate::binding::bindings::InitializationStatus; use crate::binding::bindings::InitializedInClass; use crate::binding::bindings::InitializedInFlow; use crate::binding::classmethod::ClassmethodReceiverState; @@ -653,6 +654,9 @@ pub struct Flow { info: SmallMap, /// Pairs for which the current flow excludes both values being `None`. excluded_none_pairs: ExcludedNonePairs, + /// Attribute initialization has no type to merge and needs no Phi bindings. + /// An absent entry means the attribute is not initialized on this path. + instance_attribute_initialization: SmallMap, // Have we seen control flow terminate? // // We continue to analyze the rest of the code after a flow terminates, but @@ -787,9 +791,6 @@ struct FlowInfo { struct FlowValue { idx: Idx, style: FlowStyle, - /// True if every reaching value comes from a function definition, including decorated ones. - /// This survives merges that replace `FunctionDef` with an initialization status. - is_function_def: bool, } /// The most recent narrow for a name. @@ -801,11 +802,7 @@ struct FlowNarrow { impl FlowInfo { fn new_value(idx: Idx, style: FlowStyle) -> Self { Self { - value: Some(FlowValue { - idx, - is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), - style, - }), + value: Some(FlowValue { idx, style }), narrow: None, narrow_depth: 0, loop_prior: idx, @@ -823,11 +820,7 @@ impl FlowInfo { fn updated_value(&self, idx: Idx, style: FlowStyle, in_loop: bool) -> Self { Self { - value: Some(FlowValue { - idx, - is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), - style, - }), + value: Some(FlowValue { idx, style }), // Note that any existing narrow is wiped when a new value is bound. narrow: None, narrow_depth: 0, @@ -1106,8 +1099,10 @@ pub struct ClassIndices { struct ScopeClass { name: Identifier, indices: ClassIndices, - attributes_from_recognized_methods: SmallMap>, - attributes_from_other_methods: SmallMap>, + attributes_from_recognized_methods: + SmallMap>, + attributes_from_other_methods: + SmallMap>, classmethod_receivers: Option>, /// Attributes that `__new__` assigns on the instance it creates. attributes_set_on_new_instance: SmallSet, @@ -1132,7 +1127,7 @@ impl ScopeClass { pub fn add_attributes_defined_by_method( &mut self, method_name: Name, - attributes: SmallMap, + attributes: SmallMap, nn_module_registrations: NNModuleRegistrations, ) { if is_attribute_defining_method(&method_name, &self.name.id) { @@ -1161,29 +1156,43 @@ impl ScopeClass { /// unrecognized methods, and that constructors take precedence. pub fn method_defined_attributes( self, - ) -> impl Iterator, MethodThatSetsAttr, InstanceAttribute)> { + ) -> impl Iterator< + Item = ( + Hashed, + MethodThatSetsAttr, + (InstanceAttribute, InitializationStatus), + ), + > { Self::iter_attributes(self.attributes_from_recognized_methods, true).chain( Self::iter_attributes(self.attributes_from_other_methods, false), ) } fn iter_attributes( - attrs: SmallMap>, + attrs: SmallMap>, recognized_attribute_defining_method: bool, - ) -> impl Iterator, MethodThatSetsAttr, InstanceAttribute)> { + ) -> impl Iterator< + Item = ( + Hashed, + MethodThatSetsAttr, + (InstanceAttribute, InitializationStatus), + ), + > { { attrs.into_iter().flat_map(move |(method_name, attrs)| { - attrs.into_iter_hashed().map(move |(name, attr)| { - ( - name, - MethodThatSetsAttr { - method_name: method_name.clone(), - recognized_attribute_defining_method, - instance_or_class: attr.3, - }, - attr, - ) - }) + attrs + .into_iter_hashed() + .map(move |(name, (attr, initialized))| { + ( + name, + MethodThatSetsAttr { + method_name: method_name.clone(), + recognized_attribute_defining_method, + instance_or_class: attr.3, + }, + (attr, initialized), + ) + }) }) } } @@ -1231,15 +1240,8 @@ pub struct InstanceAttribute( pub Option>, pub TextRange, pub MethodSelfKind, - pub InitializedInFlow, ); -fn instance_attribute_flow_name(name: &Name) -> Name { - // This cannot collide with a Python identifier, but lets attribute initialization reuse - // the same branch-merging machinery as local variables. - Name::new(format!("$self.{name}")) -} - #[derive(Clone, Debug)] struct ScopeMethod { name: Identifier, @@ -1249,7 +1251,7 @@ struct ScopeMethod { new_instance_names: SmallSet, instance_attributes: SmallMap, nn_module_registrations: NNModuleRegistrations, - return_attribute_initialization: Vec>, + return_attribute_initialization: Vec>, parameters: SmallMap, yields_and_returns: YieldsAndReturns, is_async: bool, @@ -2329,57 +2331,36 @@ impl Scopes { let scope = self.pop(); let unused_variables = Self::collect_unused_variables(scope.variables.clone()); match scope.kind { - ScopeKind::Method(mut method_scope) => { - for (name, attribute) in method_scope.instance_attributes.iter_mut() { - let mut initialized = if scope.flow.has_terminated { - None - } else { - Some( - scope - .flow - .get_info(&instance_attribute_flow_name(name)) - .map_or(InitializedInFlow::Conditionally, FlowInfo::initialized), - ) - }; - for return_initialization in &method_scope.return_attribute_initialization { - let on_return = return_initialization - .get(name) - .cloned() - .unwrap_or(InitializedInFlow::Conditionally); - initialized = Some(match (initialized, on_return) { - (None, initialized) => initialized, - (Some(InitializedInFlow::Yes), InitializedInFlow::Yes) => { - InitializedInFlow::Yes - } - ( - Some(InitializedInFlow::DeferredCheck(mut existing)), - InitializedInFlow::DeferredCheck(mut new), - ) => { - // All return paths must initialize the attribute. - // Combine their requirements with AND. - existing.append(&mut new); - InitializedInFlow::DeferredCheck(existing) - } - ( - Some(InitializedInFlow::Yes), - InitializedInFlow::DeferredCheck(keys), - ) - | ( - Some(InitializedInFlow::DeferredCheck(keys)), - InitializedInFlow::Yes, - ) => InitializedInFlow::DeferredCheck(keys), - _ => InitializedInFlow::Conditionally, - }); - } - // If every path raises or otherwise terminates without returning an instance, - // there is no successfully initialized object missing this attribute. - attribute.4 = initialized.unwrap_or(InitializedInFlow::Yes); - } + ScopeKind::Method(method_scope) => { + let instance_attributes = method_scope + .instance_attributes + .into_iter() + .map(|(name, attribute)| { + let final_flow = (!scope.flow.has_terminated).then_some(&scope.flow); + let initialized = InitializationStatus::merge( + final_flow + .into_iter() + .map(|flow| { + ( + flow.instance_attribute_initialization.get(&name), + flow.last_stmt_expr, + ) + }) + .chain( + method_scope + .return_attribute_initialization + .iter() + .map(|flow| (flow.get(&name), None)), + ), + ); + (name, (attribute, initialized)) + }) + .collect(); ( method_scope.yields_and_returns, Some(SelfAssignments { method_name: method_scope.name.id, - instance_attributes: method_scope.instance_attributes, + instance_attributes, nn_module_registrations: method_scope.nn_module_registrations, }), Self::collect_unused_parameters(method_scope.parameters), @@ -2419,28 +2400,20 @@ impl Scopes { x: &ExprAttribute, value: ExprOrBinding, annotation: Option>, - idx: Option>, + has_value: bool, ) -> bool { - let in_loop = self.loop_depth() != 0; + let mut in_nested_function = false; for scope in self.iter_rev_mut() { + in_nested_function |= matches!(scope.kind, ScopeKind::Function(_)); if let ScopeKind::Method(method_scope) = &mut scope.kind && let Some(self_name) = &method_scope.self_name && matches!(&*x.value, Expr::Name(name) if name.id == self_name.id) { - if let Some(idx) = idx { - match scope + if has_value && !in_nested_function { + scope .flow - .info - .entry(instance_attribute_flow_name(&x.attr.id)) - { - Entry::Vacant(entry) => { - entry.insert(FlowInfo::new_value(idx, FlowStyle::Other)); - } - Entry::Occupied(mut entry) => { - *entry.get_mut() = - entry.get().updated_value(idx, FlowStyle::Other, in_loop); - } - } + .instance_attribute_initialization + .insert(x.attr.id.clone(), InitializationStatus::Initialized); } if let Some(attr) = method_scope.instance_attributes.get_mut(&x.attr.id) { // Accumulate subsequent assignments in the method. @@ -2458,12 +2431,12 @@ impl Scopes { annotation, x.attr.range(), // Keep the range of the first assignment as the definition location. method_scope.receiver_kind, - InitializedInFlow::Yes, ), ); } return true; } + in_nested_function |= matches!(scope.kind, ScopeKind::Method(_)); } false } @@ -2508,6 +2481,27 @@ impl Scopes { } } + /// A deletion in the method body removes its initialization guarantee. + pub fn record_self_attr_delete(&mut self, x: &ExprAttribute) { + for scope in self.iter_rev_mut() { + match &scope.kind { + ScopeKind::Function(_) => return, + ScopeKind::Method(method) => { + if method.self_name.as_ref().is_some_and(|self_name| { + matches!(&*x.value, Expr::Name(name) if name.id == self_name.id) + }) { + scope.flow.instance_attribute_initialization.insert( + x.attr.id.clone(), + InitializationStatus::Uninitialized, + ); + } + return; + } + _ => {} + } + } + } + /// Record the value of `self.register_buffer(...)` or `self.register_parameter(...)`. /// The enclosing class is checked for `torch.nn.Module` ancestry during solving. pub fn record_nn_module_registration( @@ -2826,7 +2820,6 @@ impl Scopes { scope.flow.excluded_none_pairs.invalidate(name); if let Some(value) = scope.flow.get_value_mut(name) { value.style = FlowStyle::Uninitialized; - value.is_function_def = false; } scope.deleted_names.insert(name.clone()); } @@ -3398,21 +3391,9 @@ impl Scopes { } ScopeKind::Method(method) => { if !is_unreachable { - let mut initialization = - SmallMap::with_capacity(method.instance_attributes.len()); - for name in method.instance_attributes.keys() { - initialization.insert( - name.clone(), - scope - .flow - .get_info(&instance_attribute_flow_name(name)) - .map_or( - InitializedInFlow::Conditionally, - FlowInfo::initialized, - ), - ); - } - method.return_attribute_initialization.push(initialization); + method + .return_attribute_initialization + .push(scope.flow.instance_attribute_initialization.clone()); } method .yields_and_returns @@ -3599,8 +3580,6 @@ impl Scopes { }, }; let initialized = match &value.style { - // Conditional method definitions are exempt from attribute initialization checks. - _ if value.is_function_def => InitializedInFlow::Yes, FlowStyle::PossiblyUninitialized => InitializedInFlow::Conditionally, FlowStyle::MaybeInitialized(keys) => { InitializedInFlow::DeferredCheck(keys.clone()) @@ -3609,7 +3588,11 @@ impl Scopes { }; field_definitions.insert_hashed( name.owned(), - (definition, static_info.range, initialized.into()), + ( + definition, + static_info.range, + InitializationStatus::from(initialized).into(), + ), ); } }); @@ -3620,7 +3603,7 @@ impl Scopes { |( name, method, - InstanceAttribute(values, annotation, range, receiver_kind, initialized), + (InstanceAttribute(values, annotation, range, receiver_kind), initialized), )| { if let Some(( ClassFieldDefinition::DefinedInMethod { @@ -3659,17 +3642,14 @@ impl Scopes { // Any method can establish initialization, so combine their requirements with OR. match (&mut *existing_initialized, initialized) { (InitializedInClass::Yes, _) => {} - (_, InitializedInFlow::Yes) => { + (_, InitializationStatus::Initialized) => { *existing_initialized = InitializedInClass::Yes; } ( InitializedInClass::IfAny(groups), - InitializedInFlow::DeferredCheck(keys), - ) => groups.push(keys), - ( - InitializedInClass::IfAny(_), - InitializedInFlow::Conditionally | InitializedInFlow::No, - ) => {} + deferred @ InitializationStatus::Deferred(_), + ) => groups.push(deferred), + (InitializedInClass::IfAny(_), InitializationStatus::Uninitialized) => {} } } else if !field_definitions.contains_key_hashed(name.as_ref()) { field_definitions.insert_hashed( @@ -4235,55 +4215,6 @@ impl MergeStyle { } } -/// The result of analyzing whether a variable is defined after merging branches. -/// This enum captures the three distinct states: -/// - `Defined`: Variable is defined in all non-terminating branches -/// - `DeferredCheck`: Some branches don't define the variable, but they may terminate -/// (have `Never` type). The check is deferred to solve time. -/// - `NotDefined`: Variable is not defined in some branches that may not terminate -enum DefinitionStatus { - /// Variable is defined in all non-terminating branches. - Defined, - /// Variable may be defined if branches with these keys terminate (have `Never` type). - /// The keys are checked at solve time; if all have `Never` type, the variable is - /// considered initialized. - DeferredCheck(Vec>), - /// Variable is not defined in some branches that may not terminate. - NotDefined, -} - -/// Determines the definition status of a variable after a merge. -/// -/// For `LoopDefinitelyRuns`, the base having a value is also sufficient, since the -/// loop body runs at least once. -fn determine_definition_status( - merge_style: MergeStyle, - base_has_value: bool, - n_values: usize, - n_possibly_set_branches: usize, - n_branches: usize, - n_unterminated_missing_branches: usize, - missing_branch_termination_keys: Vec>, -) -> DefinitionStatus { - match merge_style { - MergeStyle::LoopDefinitelyRuns if base_has_value => DefinitionStatus::Defined, - _ if n_values == n_branches => DefinitionStatus::Defined, - // With no branch that defines the name or may have set it, the name is unset on every - // path that reaches the merge, whichever branches turn out to terminate. - _ if n_values == 0 && n_possibly_set_branches == 0 => DefinitionStatus::NotDefined, - // Every branch that leaves the name unset may still never reach the merge, which only - // its own termination key can tell. - _ if n_unterminated_missing_branches == 0 => { - if missing_branch_termination_keys.is_empty() { - DefinitionStatus::Defined - } else { - DefinitionStatus::DeferredCheck(missing_branch_termination_keys) - } - } - _ => DefinitionStatus::NotDefined, - } -} - /// Information about a single branch being merged, including both the flow info /// for a specific name (if present) and the termination key from the flow this branch came from. struct MergeBranchEntry { @@ -4456,7 +4387,6 @@ impl<'a> BindingsBuilder<'a> { merge_item: MergeItem, phi_key: PhiKey, merge_style: MergeStyle, - n_branches: usize, ) -> FlowInfo { // Only a promised phi can already appear in a branch's flow. let phi_idx = phi_key.promised; @@ -4509,45 +4439,32 @@ impl<'a> BindingsBuilder<'a> { let mut branch_idxs = SmallSet::with_capacity(merge_branches.len()); let mut branch_infos = Vec::with_capacity(merge_branches.len()); let mut styles = Vec::with_capacity(merge_branches.len()); - let mut is_function_def = true; - let mut styled_idxs = SmallSet::with_capacity(merge_branches.len()); - let mut n_values = 0; - let mut n_possibly_set_branches = 0; - // Collect termination keys from branches that don't define the variable. - // These will be used for deferred uninitialized checks at solve time. - let mut missing_branch_termination_keys = Vec::new(); - // A plain loop's base is a path past the loop even when the name is absent from it, in - // which case the base is not among the merged branches. - let mut n_unterminated_missing_branches = - usize::from(matches!(merge_style, MergeStyle::Loop) && !added_base_to_merge); - // The base is merged last, and only for loops. A loop that definitely runs never - // reaches the merge from its base, so the base does not count as a missing branch. - let base_position = (added_base_to_merge - && matches!(merge_style, MergeStyle::LoopDefinitelyRuns)) - .then(|| merge_branches.len() - 1); - for (position, merge_branch) in merge_branches.into_iter().enumerate() { - let style = merge_branch - .flow_info - .as_ref() - .and_then(|flow_info| flow_info.value.as_ref()) - .map(|v| &v.style); - // A branch that may have left the name unset, such as one carrying what a loop that - // may run zero times left behind, leaves it unset at the merge only if the branch - // reaches it. With a termination key, that is decided at solve time like a missing - // branch, and its style does not count. Without one, its style carries the doubt. - let unset_unless_terminated = matches!(style, Some(FlowStyle::PossiblyUninitialized)) - && merge_branch.termination_key.is_some(); - n_possibly_set_branches += usize::from(unset_unless_terminated); - // A branch with FlowStyle::Uninitialized (e.g., after exception variable - // unbinding via mark_as_deleted) does not define the variable. - let defines = style.is_some_and(|style| !matches!(style, FlowStyle::Uninitialized)) - && !unset_unless_terminated; - if !defines && Some(position) != base_position { - match merge_branch.termination_key { - Some(termination_key) => missing_branch_termination_keys.push(termination_key), - None => n_unterminated_missing_branches += 1, + let n_flow_branches = merge_branches.len() - usize::from(added_base_to_merge); + let definition_status = if matches!(merge_style, MergeStyle::LoopDefinitelyRuns) + && base_has_value + { + InitializationStatus::Initialized + } else { + let branches = merge_branches.iter().enumerate().filter_map(|(i, branch)| { + if i == n_flow_branches && matches!(merge_style, MergeStyle::LoopDefinitelyRuns) { + return None; } - } + let initialized = branch.flow_info.as_ref().is_some_and(|flow| { + flow.value + .as_ref() + .is_some_and(|value| !matches!(value.style, FlowStyle::Uninitialized)) + }); + Some(( + initialized.then_some(&InitializationStatus::Initialized), + branch.termination_key, + )) + }); + // A missing pre-loop definition still represents the zero-iteration path. + let missing_base = (matches!(merge_style, MergeStyle::Loop) && !added_base_to_merge) + .then_some((None, None)); + InitializationStatus::merge(branches.chain(missing_base)) + }; + for merge_branch in merge_branches { let Some(flow_info) = merge_branch.flow_info else { continue; }; @@ -4565,52 +4482,30 @@ impl<'a> BindingsBuilder<'a> { } if let Some(v) = flow_info.value { - // A branch that does not define the name still contributes its idx to type - // inference. - if defines { - n_values += 1; - } if Some(v.idx) == phi_idx { continue; } - is_function_def &= v.is_function_def; - value_idxs.insert(v.idx); - // An invariant in Pyrefly is that we only set style when we - // set a value, so duplicate value_idxs always have the same style. - if !unset_unless_terminated && styled_idxs.insert(v.idx) { + if value_idxs.insert(v.idx) { + // An invariant in Pyrefly is that we only set style when we + // set a value, so duplicate value_idxs always have the same style. styles.push(v.style); } } branch_idxs.insert(branch_idx); } - let definition_status = determine_definition_status( - merge_style, - base_has_value, - n_values, - n_possibly_set_branches, - n_branches, - n_unterminated_missing_branches, - missing_branch_termination_keys, - ); - // Helper to compute the final FlowStyle based on definition status. let compute_final_style = |styles: Vec| -> FlowStyle { match &definition_status { - // Every branch that leaves the name unset has a key, so the keys decide those - // branches. A branch that may hold the name without a key keeps its doubt. - DefinitionStatus::DeferredCheck(keys) => FlowStyle::merged( - true, - styles - .into_iter() - .filter(|style| !matches!(style, FlowStyle::Uninitialized)) - .chain([FlowStyle::MaybeInitialized(keys.clone())]), - merge_style, - ), - DefinitionStatus::Defined => { + // Each local branch contributes at most one termination key, so these + // clauses are singletons and can use FlowStyle's existing conjunction. + InitializationStatus::Deferred(paths) => { + FlowStyle::MaybeInitialized(paths.iter().flatten().copied().collect()) + } + InitializationStatus::Initialized => { FlowStyle::merged(true, styles.into_iter(), merge_style) } - DefinitionStatus::NotDefined => { + InitializationStatus::Uninitialized => { FlowStyle::merged(false, styles.into_iter(), merge_style) } } @@ -4650,7 +4545,6 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: *value_idxs.first().unwrap(), style: compute_final_style(styles), - is_function_def, }), narrow: Some(FlowNarrow { idx: merged_idx }), narrow_depth: 1, @@ -4672,7 +4566,6 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: merged_idx, style: compute_final_style(styles), - is_function_def, }), narrow: None, narrow_depth: 0, @@ -4724,16 +4617,6 @@ impl<'a> BindingsBuilder<'a> { || base.is_definitely_unreachable) && flows.iter().all(|f| f.is_definitely_unreachable); - // For a regular loop, we merge the base so there's one extra branch being merged. - // For LoopDefinitelyRuns, we don't count the base as an extra branch because we - // know the loop body will definitely execute at least once. - let n_branches = flows.len() - + if matches!(merge_style, MergeStyle::Loop) { - 1 - } else { - 0 - }; - // Collect all termination keys from flows (for building dense MergeItems) let all_termination_keys: Vec>> = flows.iter().map(|f| f.last_stmt_expr).collect(); @@ -4745,6 +4628,34 @@ impl<'a> BindingsBuilder<'a> { None }; + // Each reachable path must initialize an attribute. A missing assignment can + // instead be justified by a Never-returning call on that path. Ordinary loops + // include the pre-loop path because their bodies may execute zero times. + // If no loop path survives, retain the pre-loop state as the local flow does. + let attribute_flows = flows.iter().map(|flow| (flow, flow.last_stmt_expr)).chain( + (matches!(merge_style, MergeStyle::Loop) + || (merge_style.is_loop() && flows.is_empty())) + .then_some((&base, None)), + ); + let mut attribute_names = SmallSet::new(); + for (flow, _) in attribute_flows.clone() { + for name in flow.instance_attribute_initialization.keys() { + attribute_names.insert(name.clone()); + } + } + let mut instance_attribute_initialization = SmallMap::with_capacity(attribute_names.len()); + for name in attribute_names { + let initialized = InitializationStatus::merge(attribute_flows.clone().map( + |(flow, termination_key)| { + ( + flow.instance_attribute_initialization.get(&name), + termination_key, + ) + }, + )); + instance_attribute_initialization.insert(name, initialized); + } + // Collect all unique names from base + all flows. We need this before we construct merge items // so that we can accurately represent a flow in which some name doesn't appear. let mut all_names: SmallSet = SmallSet::new(); @@ -4793,7 +4704,7 @@ impl<'a> BindingsBuilder<'a> { promised: self.promised_idx(&key), key, }; - let flow_info = self.merged_flow_info(merge_item, phi_key, merge_style, n_branches); + let flow_info = self.merged_flow_info(merge_item, phi_key, merge_style); merged_flow_infos.insert_hashed(name, flow_info); } @@ -4801,6 +4712,7 @@ impl<'a> BindingsBuilder<'a> { let flow = Flow { info: merged_flow_infos, excluded_none_pairs, + instance_attribute_initialization, has_terminated, terminated_by_raise: has_terminated && any_terminated_by_raise, is_definitely_unreachable: all_are_unreachable, @@ -4835,7 +4747,6 @@ impl<'a> BindingsBuilder<'a> { info.value = Some(FlowValue { idx: phi_idx, style: FlowStyle::LoopRecursion, - is_function_def: false, }); info.narrow = None; } diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index f439cf27e1..d6f728bf9f 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -985,6 +985,9 @@ impl<'a> BindingsBuilder<'a> { delete_idx, Binding::Delete(Box::new(target.clone())), ); + if let Expr::Attribute(attribute) = &target { + self.scopes.record_self_attr_delete(attribute); + } if let Expr::Attribute(_) = target && let Some((identifier, _)) = identifier_and_chain_prefix_for_expr(&target) { @@ -1287,14 +1290,12 @@ impl<'a> BindingsBuilder<'a> { None, ), ); - let (value, attr_idx) = match x.value { + let has_value = x.value.is_some(); + let value = match x.value { Some(mut assigned) => { - let (value, idx) = self.bind_attr_assign( - attr.clone(), - &mut assigned, - |_attr, value, _| ExprOrBinding::Expr(value.clone()), - ); - (value, Some(idx)) + self.bind_attr_assign(attr.clone(), &mut assigned, |_attr, value, _| { + ExprOrBinding::Expr(value.clone()) + }) } _ => { self.ensure_expr( @@ -1303,17 +1304,14 @@ impl<'a> BindingsBuilder<'a> { is_annotation: false, }, ); - ( - ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit)), - None, - ) + ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit)) } }; if !self.scopes.record_self_attr_assign( &attr, value.clone(), Some(ann_key), - attr_idx, + has_value, ) { self.error( x.range, diff --git a/pyrefly/lib/binding/target.rs b/pyrefly/lib/binding/target.rs index 30f08f3f67..64c568ea70 100644 --- a/pyrefly/lib/binding/target.rs +++ b/pyrefly/lib/binding/target.rs @@ -193,7 +193,7 @@ impl<'a> BindingsBuilder<'a> { Option>, ) -> ExprOrBinding, ensure_assigned: bool, - ) -> (ExprOrBinding, Idx) { + ) -> ExprOrBinding { let narrowing_identifier = identifier_and_chain_prefix_for_expr(&Expr::Attribute(attr.clone())) .map(|(identifier, _)| identifier); @@ -236,7 +236,7 @@ impl<'a> BindingsBuilder<'a> { if let Some(identifier) = narrowing_identifier { self.narrow_if_name_is_defined(identifier, idx); } - (value, idx) + value } pub fn bind_attr_assign( @@ -248,7 +248,7 @@ impl<'a> BindingsBuilder<'a> { &Expr, Option>, ) -> ExprOrBinding, - ) -> (ExprOrBinding, Idx) { + ) -> ExprOrBinding { self.bind_attr_assign_impl( attr, Some(assigned), @@ -348,7 +348,7 @@ impl<'a> BindingsBuilder<'a> { ); } Expr::Attribute(x) => { - let (attr_value, attr_idx) = self.bind_attr_assign_impl( + let attr_value = self.bind_attr_assign_impl( x.clone(), assigned, |_attr, value, ann| make_assigned_value(value, ann), @@ -357,7 +357,7 @@ impl<'a> BindingsBuilder<'a> { // If this is a self-assignment, record it because we may use it to infer // the existence of an instance-only attribute. self.scopes - .record_self_attr_assign(x, attr_value, None, Some(attr_idx)); + .record_self_attr_assign(x, attr_value, None, true); self.scopes.record_new_instance_attr_assign(x); } Expr::Subscript(x) => { diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index e7003237f2..8baab33fac 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -10,6 +10,326 @@ use crate::test::util::TestEnv; use crate::testcase; +testcase!( + test_attribute_initialization_inherited, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +class Base: + timeout = 10 + def __init__(self) -> None: + self.value = 0 + +class Child(Base): + def __init__(self, t: int | None = None) -> None: + super().__init__() + if t is not None: + self.timeout = t + self.value = t + self.own = t # E: Attribute `own` may be uninitialized + +class Grandchild(Child): + def __init__(self, t: int | None = None) -> None: + if t is not None: + self.timeout = t + self.value = t + +class ClassBody(Base): + if bool(): + timeout = 20 +"#, +); + +testcase!( + test_attribute_initialization_trailing_noreturn, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +from typing import NoReturn + +def stop() -> NoReturn: + raise ValueError() + +def proceed() -> None: ... + +class TrailingCall: + def __init__(self, flag: bool) -> None: + if flag: + self.value = 1 + stop() + +class TrailingRaise: + def __init__(self, flag: bool) -> None: + if flag: + self.value = 1 + raise ValueError() + +class DeferredThenStop: + def __init__(self, flag: bool) -> None: + if flag: + self.value = 1 + else: + proceed() + stop() + +class DeferredThenProceed: + def __init__(self, flag: bool) -> None: + if flag: + self.value = 1 + else: + stop() + proceed() + +class EarlyReturn: + def __init__(self, flag: bool) -> None: + if flag: + return + self.value = 1 # E: Attribute `value` may be uninitialized + stop() +"#, +); + +testcase!( + test_attribute_initialization_while_true_return, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +def connect() -> int: ... + +class Retry: + def __init__(self) -> None: + while True: + try: + self.conn = connect() + return + except OSError: + continue + +class EarlyReturn: + def __init__(self, flag: bool) -> None: + while True: + if flag: + return + self.value = 1 # E: Attribute `value` may be uninitialized + return + +class SuppressedLoopRaise: + def __init__(self) -> None: + from contextlib import suppress + with suppress(ValueError): + while True: + raise ValueError() + self.value = 1 + +class ReturnOrBreak: + def __init__(self, flag: bool) -> None: + while True: + if flag: + self.value = 1 # E: Attribute `value` may be uninitialized + return + break +"#, +); + +testcase!( + test_attribute_initialization_nested_function, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +class NestedOnly: + def __init__(self) -> None: + def later() -> None: + self.value = 1 # E: Attribute `value` may be uninitialized + +class ConditionalOuter: + def __init__(self, flag: bool) -> None: + if flag: + self.value = 1 # E: Attribute `value` may be uninitialized + def later() -> None: + self.value = 2 + +class InitializedOuter: + def __init__(self) -> None: + self.value = 1 + def later() -> None: + del self.value + +class NestedMethod: + def __init__(self) -> None: + class Inner: + def later(inner) -> None: + self.value = 1 # E: Attribute `value` may be uninitialized + +class NestedReturn: + def __init__(self) -> None: + def later() -> None: + return + self.value = 1 +"#, +); + +testcase!( + test_attribute_initialization_control_flow, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +from contextlib import nullcontext +from typing import Literal + +class TryExcept: + def __init__(self, raw: str) -> None: + try: + self.value = int(raw) + except ValueError: + self.value = 0 + +class TryExceptMissing: + def __init__(self, raw: str) -> None: + try: + self.value = int(raw) # E: Attribute `value` may be uninitialized + except ValueError: + pass + +class Finally: + def __init__(self) -> None: + try: + int("1") + finally: + self.value = 1 + +class With: + def __init__(self) -> None: + with nullcontext(): + self.value = 1 + +class WithConditional: + def __init__(self, flag: bool) -> None: + with nullcontext(): + if flag: + self.value = 1 # E: Attribute `value` may be uninitialized + +class Match: + def __init__(self, kind: Literal["a", "b"]) -> None: + match kind: + case "a": + self.value = 1 + case "b": + self.value = 2 + +class MatchMissing: + def __init__(self, kind: str) -> None: + match kind: + case "a": + self.value = 1 # E: Attribute `value` may be uninitialized + +class Deleted: + def __init__(self) -> None: + self.value = 1 # E: Attribute `value` may be uninitialized + del self.value + +class DeletedConditionally: + def __init__(self, flag: bool) -> None: + self.value = 1 # E: Attribute `value` may be uninitialized + if flag: + del self.value + +class Reassigned: + def __init__(self) -> None: + self.value = 1 + del self.value + self.value = 2 +"#, +); + +testcase!( + test_attribute_initialization_class_conditions_and_hooks, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +import sys +from dataclasses import dataclass +from typing import TYPE_CHECKING, assert_type + +class Conditional: + if TYPE_CHECKING: + type_only = 1 + if sys.version_info >= (3, 10): + versioned = 1 + +class VersionedLoop: + while sys.version_info >= (3, 0): + pass + value = 1 + +@dataclass +class PostInit: + flag: bool + def __post_init__(self) -> None: + self.value = 1 + +@dataclass +class ConditionalPostInit: + flag: bool + def __post_init__(self) -> None: + if self.flag: + self.value = 1 # E: Attribute `value` may be uninitialized + +class SubclassHook: + def __init_subclass__(cls) -> None: + cls.value = 1 + +class ConditionalSubclassHook: + def __init_subclass__(cls, flag: bool = False) -> None: + if flag: + cls.value = 1 # E: Attribute `value` may be uninitialized + +class ClassLoop: + for value in [1]: + pass + +assert_type(Conditional.type_only, int) +assert_type(Conditional.versioned, int) +assert_type(ClassLoop.value, int) +"#, +); + +testcase!( + bug = "Suppressed exceptions do not merge attribute initialization", + test_attribute_initialization_suppressed_exception, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +from contextlib import suppress + +class C: + def __init__(self, raw: str) -> None: + with suppress(ValueError): + self.value = int(raw) +"#, +); + +testcase!( + bug = "Return snapshots do not include assignments from finally", + test_attribute_initialization_finally_after_return, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +class C: + def __init__(self) -> None: + try: + return + finally: + self.value = 1 # E: Attribute `value` may be uninitialized +"#, +); + +testcase!( + bug = "For loop targets are treated as assigned before the first iteration", + test_attribute_initialization_class_loop_target, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +from typing import assert_type + +class C: + for value in list[int](): + pass + +assert_type(C.value, int) +"#, +); + testcase!( test_possibly_uninitialized_attribute_default_disabled, r#" @@ -496,6 +816,82 @@ class ConditionalConstructor: "#, ); +testcase!( + test_attribute_initialization_loops, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +from typing import Never, assert_type + +def stop() -> Never: + raise RuntimeError() + +class BeforeLoop: + def __init__(self, values: list[int]) -> None: + self.value = 0 + for value in values: + self.value = value + +class PossiblyEmpty: + def __init__(self, values: list[int]) -> None: + for value in values: + self.value = value # E: Attribute `value` may be uninitialized + +class NonEmpty: + def __init__(self) -> None: + for value in [1]: + self.value = value + +class NoReturningLoop: + def __init__(self) -> None: + self.value = 0 + for value in [1]: + raise RuntimeError() + +class ConditionalContinue: + def __init__(self, flag: bool) -> None: + for value in [1]: + if flag: + continue + self.value = value # E: Attribute `value` may be uninitialized + +class ConditionalBreak: + def __init__(self, flag: bool) -> None: + while True: + if flag: + break + self.value = 1 # E: Attribute `value` may be uninitialized + break + +class LoopElse: + def __init__(self, values: list[int]) -> None: + for value in values: + self.value = value + else: + self.value = 0 + +class DeferredBeforeLoop: + def __init__(self, flag: bool, values: list[int]) -> None: + if flag: + self.value = 1 + else: + stop() + for value in values: + pass + +class DeferredInLoop: + def __init__(self, flag: bool) -> None: + for value in [1]: + if flag: + self.value = value + else: + stop() + +assert_type(BeforeLoop([]).value, int) +assert_type(NonEmpty().value, int) +assert_type(LoopElse([]).value, int) +"#, +); + testcase!( test_attribute_initialization_deferred_return_paths, TestEnv::new().enable_possibly_uninitialized_attribute_error(), diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 6f261ace38..8d1f40f86b 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1873,19 +1873,31 @@ Default severity: `ignore` The `strict` preset enables this check as an error. -An attribute may be uninitialized after class-body execution or instance initialization. -Pyrefly reports this error at the attribute definition, even if the attribute is never read. -A later `hasattr` guard does not suppress this check. -Conditional method definitions are excluded. -This check does not apply to stub (`.pyi`) files because they do not perform runtime initialization. +Pyrefly checks whether an assigned attribute exists after the class body or a recognized initialization method completes. +Recognized methods include `__init__`, `__new__`, `__post_init__`, `__init_subclass__`, and supported test setup methods. +Each path that completes normally, including an explicit `return`, must leave the attribute initialized. +Paths that raise, loop forever, or end in a `Never` or `NoReturn` call do not require an assignment. +Deleting an attribute removes its initialization guarantee. ```python class C: def __init__(self, value: int | None) -> None: if value is not None: self.value = value # possibly-uninitialized-attribute + # The path where value is None completes without assigning self.value. ``` +The check permits an inherited attribute when a base class other than `object` defines the same name. +An instance `__getattr__` fallback also suppresses this error for instance attributes. +Pyrefly combines assignments from different methods leniently: one method that initializes the attribute on every completing path suffices. +This includes ordinary helper methods, even when Pyrefly cannot prove that the constructor calls them. +Assignments inside nested functions do not initialize the enclosing method's attributes. + +Pyrefly reports this error at the attribute definition, even if the attribute is never read. +A later `hasattr` guard does not suppress this check. +Conditional method definitions and stub (`.pyi`) files are excluded. +Bare instance annotations use the separate [`uninitialized-instance-variable`](#uninitialized-instance-variable) check. + To disable this check while keeping other strict checks, use: ```toml @@ -1896,7 +1908,6 @@ possibly-uninitialized-attribute = "ignore" ``` This setting does not disable [`missing-attribute`](#missing-attribute). -The separate [`uninitialized-instance-variable`](#uninitialized-instance-variable) check reports class-body instance declarations that have no initialization. ## potential-bad-keyword-argument From 12314edb28a244296b04acef16d159ac532c7e55 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 22 Oct 2026 12:13:20 +0900 Subject: [PATCH 08/10] Bound attribute initialization proofs and isolate flow changes --- pyrefly/lib/binding/bindings.rs | 67 ++++++++- pyrefly/lib/binding/class.rs | 36 ----- pyrefly/lib/binding/scope.rs | 192 ++++++++++++++++++++------ pyrefly/lib/binding/stmt.rs | 11 -- pyrefly/lib/test/attributes.rs | 33 +++-- pyrefly/lib/test/delayed_inference.rs | 2 +- pyrefly/lib/test/flow_branching.rs | 9 +- pyrefly/lib/test/lsp/hover_type.rs | 2 +- pyrefly/lib/test/paramspec.rs | 2 +- pyrefly/lib/test/perf.rs | 34 +++++ pyrefly/lib/test/scope.rs | 2 +- website/docs/error-kinds.mdx | 3 +- 12 files changed, 283 insertions(+), 110 deletions(-) diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index bc06c8c0cc..397659feb8 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -228,6 +228,7 @@ impl InitializationStatus { pub fn merge<'a>( paths: impl IntoIterator, Option>)>, ) -> Self { + const MAX_DEFERRED_PATHS: usize = 16; let mut conditions = Vec::new(); for (initialized, termination_key) in paths { match initialized { @@ -258,6 +259,16 @@ impl InitializationStatus { // Forks can copy the same requirement into several paths. conditions.sort_unstable(); conditions.dedup(); + if conditions.len() > MAX_DEFERRED_PATHS { + // Retain one key per requirement to prevent exponential growth across joins. + // This strengthens the requirements and can produce false positives above the cap. + // After deduplication, at most one singleton remains per distinct key. + for path in &mut conditions { + path.truncate(1); + } + conditions.sort_unstable(); + conditions.dedup(); + } Self::Deferred(conditions) } } @@ -288,7 +299,8 @@ impl From for InitializationStatus { #[derive(Debug, Clone)] pub enum InitializedInClass { Yes, - /// Any defining method or the class body can establish initialization. + /// Any defining method can establish initialization. + /// Class-body definitions are checked separately and never merge with methods. /// An empty list means no flow guarantees initialization. IfAny(Vec), } @@ -3382,3 +3394,56 @@ impl BindingsBuilder<'_> { ); } } + +#[cfg(test)] +mod initialization_tests { + use pyrefly_graph::index::Idx; + + use super::InitializationStatus; + + #[test] + fn test_attribute_initialization_path_limit() { + let mut status = InitializationStatus::Deferred(vec![vec![Idx::new(0)]]); + for join in 0..5 { + status = InitializationStatus::merge([ + (Some(&status), Some(Idx::new(2 * join + 1))), + (Some(&status), Some(Idx::new(2 * join + 2))), + ]); + if join == 3 { + let InitializationStatus::Deferred(paths) = &status else { + panic!("conditional paths must remain deferred"); + }; + assert_eq!(paths.len(), 16); + assert!(status.is_initialized(|key| key.idx() == 7 || key.idx() == 8)); + } + } + let InitializationStatus::Deferred(paths) = &status else { + panic!("conditional paths must remain deferred"); + }; + assert_eq!(paths.len(), 1); + assert!(!status.is_initialized(|key| key.idx() == 9 || key.idx() == 10)); + assert!(status.is_initialized(|key| key.idx() == 0)); + } + + #[test] + fn test_attribute_initialization_path_growth_is_bounded() { + for initial_paths in [1, 64] { + let mut status = InitializationStatus::Deferred( + (0..initial_paths).map(|key| vec![Idx::new(key)]).collect(), + ); + for join in 0..1000 { + status = InitializationStatus::merge([ + (Some(&status), Some(Idx::new(initial_paths + 2 * join))), + (Some(&status), Some(Idx::new(initial_paths + 2 * join + 1))), + ]); + let InitializationStatus::Deferred(paths) = &status else { + panic!("conditional paths must remain deferred"); + }; + assert!(paths.len() <= initial_paths.max(16)); + assert!(paths.iter().all(|path| path.len() <= 5)); + assert!(!status.is_initialized(|_| false)); + assert!(status.is_initialized(|key| key.idx() < initial_paths)); + } + } + } +} diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index a61043bb53..64d8190506 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -31,7 +31,6 @@ use ruff_python_ast::name::Name; use ruff_text_size::Ranged; use ruff_text_size::TextRange; use starlark_map::small_map::SmallMap; -use starlark_map::small_set::SmallSet; use crate::binding::attrs::AttrsDecoratorMethods; use crate::binding::attrs::collect_attrs_decorator_methods; @@ -517,44 +516,9 @@ impl<'a> BindingsBuilder<'a> { ClassFieldDefinition::DefinedInMethod { method, .. } if !method.recognized_attribute_defining_method ); - // Inspect definition bindings here so method provenance does not occupy every flow value. - let is_conditional_method = - if let ClassFieldDefinition::DefinedWithoutAssign { definition } = &definition { - let mut pending = vec![*definition]; - let mut seen = SmallSet::new(); - let mut found_function = false; - let mut only_functions = true; - while let Some(idx) = pending.pop() { - if !seen.insert(idx) { - continue; - } - match self - .get_original_binding(idx) - .and_then(|(_, binding)| binding) - { - Some(Binding::Function { .. }) => found_function = true, - Some(Binding::Phi(_, branches)) => { - pending.extend(branches.iter().map(|branch| branch.value_key)); - } - Some(Binding::Narrow(prior, _, _)) => pending.push(*prior), - Some(Binding::LoopPhi(phi)) => { - pending.push(phi.0); - pending.extend(phi.1.iter().copied()); - } - _ => { - only_functions = false; - break; - } - } - } - found_function && only_functions - } else { - false - }; // Stub files declare attributes without runtime initialization. if !self.module_info.path().is_interface() && check_initialization - && !is_conditional_method && let InitializedInClass::IfAny(initializations) = initialized { let is_instance = matches!( diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 285300af05..644dc40e08 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -791,6 +791,9 @@ struct FlowInfo { struct FlowValue { idx: Idx, style: FlowStyle, + /// True when every reaching value comes from a function definition. + /// Branch merges preserve this flag when they replace the flow style. + is_function_def: bool, } /// The most recent narrow for a name. @@ -802,7 +805,11 @@ struct FlowNarrow { impl FlowInfo { fn new_value(idx: Idx, style: FlowStyle) -> Self { Self { - value: Some(FlowValue { idx, style }), + value: Some(FlowValue { + idx, + is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), + style, + }), narrow: None, narrow_depth: 0, loop_prior: idx, @@ -820,7 +827,11 @@ impl FlowInfo { fn updated_value(&self, idx: Idx, style: FlowStyle, in_loop: bool) -> Self { Self { - value: Some(FlowValue { idx, style }), + value: Some(FlowValue { + idx, + is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), + style, + }), // Note that any existing narrow is wiped when a new value is bound. narrow: None, narrow_depth: 0, @@ -2820,6 +2831,7 @@ impl Scopes { scope.flow.excluded_none_pairs.invalidate(name); if let Some(value) = scope.flow.get_value_mut(name) { value.style = FlowStyle::Uninitialized; + value.is_function_def = false; } scope.deleted_names.insert(name.clone()); } @@ -3580,6 +3592,7 @@ impl Scopes { }, }; let initialized = match &value.style { + _ if value.is_function_def => InitializedInFlow::Yes, FlowStyle::PossiblyUninitialized => InitializedInFlow::Conditionally, FlowStyle::MaybeInitialized(keys) => { InitializedInFlow::DeferredCheck(keys.clone()) @@ -4215,6 +4228,58 @@ impl MergeStyle { } } +/// The result of analyzing whether a variable is defined after merging branches. +/// This enum captures the three distinct states: +/// - `Defined`: Variable is defined in all non-terminating branches +/// - `DeferredCheck`: Some branches don't define the variable, but they may terminate +/// (have `Never` type). The check is deferred to solve time. +/// - `NotDefined`: Variable is not defined in some branches that may not terminate +enum DefinitionStatus { + /// Variable is defined in all non-terminating branches. + Defined, + /// Variable may be defined if branches with these keys terminate (have `Never` type). + /// The keys are checked at solve time; if all have `Never` type, the variable is + /// considered initialized. + DeferredCheck(Vec>), + /// Variable is not defined in some branches that may not terminate. + NotDefined, +} + +/// Determines the definition status of a variable after a merge. +/// +/// The logic differs slightly for `LoopDefinitelyRuns` (where base having a value +/// or all loop body branches having values is sufficient) vs other merge styles. +fn determine_definition_status( + merge_style: MergeStyle, + base_has_value: bool, + n_values: usize, + n_branches: usize, + n_missing_branches: usize, + n_branches_with_termination_key: usize, + missing_branch_termination_keys: Vec>, +) -> DefinitionStatus { + match merge_style { + MergeStyle::LoopDefinitelyRuns if base_has_value => DefinitionStatus::Defined, + MergeStyle::LoopDefinitelyRuns if n_values == n_branches => DefinitionStatus::Defined, + MergeStyle::LoopDefinitelyRuns if n_missing_branches <= n_branches_with_termination_key => { + if !missing_branch_termination_keys.is_empty() { + DefinitionStatus::DeferredCheck(missing_branch_termination_keys) + } else { + DefinitionStatus::Defined + } + } + _ if n_values == n_branches => DefinitionStatus::Defined, + _ if n_missing_branches <= n_branches_with_termination_key => { + if !missing_branch_termination_keys.is_empty() { + DefinitionStatus::DeferredCheck(missing_branch_termination_keys) + } else { + DefinitionStatus::Defined + } + } + _ => DefinitionStatus::NotDefined, + } +} + /// Information about a single branch being merged, including both the flow info /// for a specific name (if present) and the termination key from the flow this branch came from. struct MergeBranchEntry { @@ -4387,6 +4452,8 @@ impl<'a> BindingsBuilder<'a> { merge_item: MergeItem, phi_key: PhiKey, merge_style: MergeStyle, + n_branches: usize, + n_branches_with_termination_key: usize, ) -> FlowInfo { // Only a promised phi can already appear in a branch's flow. let phi_idx = phi_key.promised; @@ -4439,33 +4506,18 @@ impl<'a> BindingsBuilder<'a> { let mut branch_idxs = SmallSet::with_capacity(merge_branches.len()); let mut branch_infos = Vec::with_capacity(merge_branches.len()); let mut styles = Vec::with_capacity(merge_branches.len()); - let n_flow_branches = merge_branches.len() - usize::from(added_base_to_merge); - let definition_status = if matches!(merge_style, MergeStyle::LoopDefinitelyRuns) - && base_has_value - { - InitializationStatus::Initialized - } else { - let branches = merge_branches.iter().enumerate().filter_map(|(i, branch)| { - if i == n_flow_branches && matches!(merge_style, MergeStyle::LoopDefinitelyRuns) { - return None; - } - let initialized = branch.flow_info.as_ref().is_some_and(|flow| { - flow.value - .as_ref() - .is_some_and(|value| !matches!(value.style, FlowStyle::Uninitialized)) - }); - Some(( - initialized.then_some(&InitializationStatus::Initialized), - branch.termination_key, - )) - }); - // A missing pre-loop definition still represents the zero-iteration path. - let missing_base = (matches!(merge_style, MergeStyle::Loop) && !added_base_to_merge) - .then_some((None, None)); - InitializationStatus::merge(branches.chain(missing_base)) - }; - for merge_branch in merge_branches { + let mut is_function_def = true; + let mut n_values = 0; + // Collect termination keys from branches that don't define the variable. + // These will be used for deferred uninitialized checks at solve time. + let mut missing_branch_termination_keys = Vec::new(); + for merge_branch in merge_branches.into_iter() { + // Handle branches that don't have this name at all (flow_info is None) let Some(flow_info) = merge_branch.flow_info else { + // This branch doesn't have this name - record its termination key if any + if let Some(termination_key) = merge_branch.termination_key { + missing_branch_termination_keys.push(termination_key); + } continue; }; let branch_idx = flow_info.idx(); @@ -4482,30 +4534,67 @@ impl<'a> BindingsBuilder<'a> { } if let Some(v) = flow_info.value { + // A branch with FlowStyle::Uninitialized (e.g., after exception variable + // unbinding via mark_as_deleted) should not count as defining the variable. + // We still track its idx for type inference but don't count it as a value. + let is_uninitialized = matches!(v.style, FlowStyle::Uninitialized); + if !is_uninitialized { + n_values += 1; + } if Some(v.idx) == phi_idx { + // If uninitialized, still track termination key before continuing. + if is_uninitialized && let Some(termination_key) = merge_branch.termination_key + { + missing_branch_termination_keys.push(termination_key); + } continue; } + is_function_def &= v.is_function_def; if value_idxs.insert(v.idx) { // An invariant in Pyrefly is that we only set style when we // set a value, so duplicate value_idxs always have the same style. styles.push(v.style); } + // Treat uninitialized branches like missing branches for termination keys. + if is_uninitialized && let Some(termination_key) = merge_branch.termination_key { + missing_branch_termination_keys.push(termination_key); + } + } else { + // This branch doesn't have a value for the variable. + // If it has a termination key, track it for deferred checking. + if let Some(termination_key) = merge_branch.termination_key { + missing_branch_termination_keys.push(termination_key); + } } branch_idxs.insert(branch_idx); } + // n_total_branches is the actual number of branches we iterated over, which includes + // the base flow for loops (since base is added to merge_branches for type inference). + let n_total_branches = if added_base_to_merge { + n_branches + 1 + } else { + n_branches + }; + let n_missing_branches = n_total_branches - n_values; + let definition_status = determine_definition_status( + merge_style, + base_has_value, + n_values, + n_branches, + n_missing_branches, + n_branches_with_termination_key, + missing_branch_termination_keys, + ); + // Helper to compute the final FlowStyle based on definition status. let compute_final_style = |styles: Vec| -> FlowStyle { match &definition_status { - // Each local branch contributes at most one termination key, so these - // clauses are singletons and can use FlowStyle's existing conjunction. - InitializationStatus::Deferred(paths) => { - FlowStyle::MaybeInitialized(paths.iter().flatten().copied().collect()) - } - InitializationStatus::Initialized => { + DefinitionStatus::DeferredCheck(keys) => FlowStyle::MaybeInitialized(keys.clone()), + DefinitionStatus::Defined => { FlowStyle::merged(true, styles.into_iter(), merge_style) } - InitializationStatus::Uninitialized => { + DefinitionStatus::NotDefined => { FlowStyle::merged(false, styles.into_iter(), merge_style) } } @@ -4545,6 +4634,7 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: *value_idxs.first().unwrap(), style: compute_final_style(styles), + is_function_def, }), narrow: Some(FlowNarrow { idx: merged_idx }), narrow_depth: 1, @@ -4566,6 +4656,7 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: merged_idx, style: compute_final_style(styles), + is_function_def, }), narrow: None, narrow_depth: 0, @@ -4617,6 +4708,20 @@ impl<'a> BindingsBuilder<'a> { || base.is_definitely_unreachable) && flows.iter().all(|f| f.is_definitely_unreachable); + // For a regular loop, we merge the base so there's one extra branch being merged. + // For LoopDefinitelyRuns, we don't count the base as an extra branch because we + // know the loop body will definitely execute at least once. + let n_branches = flows.len() + + if matches!(merge_style, MergeStyle::Loop) { + 1 + } else { + 0 + }; + + // Count how many branches have a last_stmt_expr (potential type-based termination) + let n_branches_with_termination_key = + flows.iter().filter(|f| f.last_stmt_expr.is_some()).count(); + // Collect all termination keys from flows (for building dense MergeItems) let all_termination_keys: Vec>> = flows.iter().map(|f| f.last_stmt_expr).collect(); @@ -4704,7 +4809,13 @@ impl<'a> BindingsBuilder<'a> { promised: self.promised_idx(&key), key, }; - let flow_info = self.merged_flow_info(merge_item, phi_key, merge_style); + let flow_info = self.merged_flow_info( + merge_item, + phi_key, + merge_style, + n_branches, + n_branches_with_termination_key, + ); merged_flow_infos.insert_hashed(name, flow_info); } @@ -4747,6 +4858,7 @@ impl<'a> BindingsBuilder<'a> { info.value = Some(FlowValue { idx: phi_idx, style: FlowStyle::LoopRecursion, + is_function_def: false, }); info.narrow = None; } @@ -4783,7 +4895,6 @@ impl<'a> BindingsBuilder<'a> { is_while_true: bool, loop_definitely_runs: bool, ) { - let loop_raises = self.scopes.terminated_by_raise(); let finished_loop = self.scopes.finish_loop(); let (breaks, other_exits): (Vec, Vec) = finished_loop .exits @@ -4822,13 +4933,6 @@ impl<'a> BindingsBuilder<'a> { // also know that breaks are the only way to exit, so we drop the current flow, // which is actually unreachable. // - if is_while_true && breaks.is_empty() { - self.scopes.mark_flow_termination(if loop_raises { - TerminationKind::Raise - } else { - TerminationKind::Jump - }); - } if let Some(base) = base_if_breaks { if is_while_true { self.merge_flow(base, breaks, other_range, MergeStyle::Exclusive) diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index d6f728bf9f..446abc8cf7 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -1499,7 +1499,6 @@ impl<'a> BindingsBuilder<'a> { ); } Stmt::While(mut x) => { - let was_unreachable = self.scopes.is_definitely_unreachable(); self.setup_loop(x.range, &SmallSet::new()); // Note that it is important we ensure *after* we set up the loop, so that both the // narrowing and type checking are aware that the test might be impacted by changes @@ -1550,16 +1549,6 @@ impl<'a> BindingsBuilder<'a> { is_while_true, is_while_true, ); - if is_while_true - && !test_is_environment_independent - && self.scopes.has_terminated() - && !was_unreachable - { - // Another Python environment can skip the loop. - self.scopes.set_definitely_unreachable(false); - self.scopes - .mark_flow_termination(TerminationKind::StaticTest); - } } Stmt::If(mut x) => { let is_definitely_unreachable = self.scopes.is_definitely_unreachable(); diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 8baab33fac..d10ae507cf 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -10,6 +10,28 @@ use crate::test::util::TestEnv; use crate::testcase; +testcase!( + test_class_body_initialization_is_independent_of_methods, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +def cond() -> bool: ... + +class Constructor: + if cond(): + x = 1 # E: Attribute `x` may be uninitialized + + def __init__(self) -> None: + self.x = 2 + +class Helper: + if cond(): + x = 1 # E: Attribute `x` may be uninitialized + + def initialize(self) -> None: + self.x = 2 +"#, +); + testcase!( test_attribute_initialization_inherited, TestEnv::new().enable_possibly_uninitialized_attribute_error(), @@ -88,6 +110,7 @@ class EarlyReturn: ); testcase!( + bug = "Retry loops retain a spurious fallthrough path", test_attribute_initialization_while_true_return, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -97,7 +120,7 @@ class Retry: def __init__(self) -> None: while True: try: - self.conn = connect() + self.conn = connect() # E: Attribute `conn` may be uninitialized return except OSError: continue @@ -110,14 +133,6 @@ class EarlyReturn: self.value = 1 # E: Attribute `value` may be uninitialized return -class SuppressedLoopRaise: - def __init__(self) -> None: - from contextlib import suppress - with suppress(ValueError): - while True: - raise ValueError() - self.value = 1 - class ReturnOrBreak: def __init__(self, flag: bool) -> None: while True: diff --git a/pyrefly/lib/test/delayed_inference.rs b/pyrefly/lib/test/delayed_inference.rs index 204dc0a83c..b74bdb36dd 100644 --- a/pyrefly/lib/test/delayed_inference.rs +++ b/pyrefly/lib/test/delayed_inference.rs @@ -45,7 +45,7 @@ from typing import assert_type, Literal x = 5 while True: x = x + 1 -y = x # E: This code is unreachable +y = x assert_type(y, int) "#, ); diff --git a/pyrefly/lib/test/flow_branching.rs b/pyrefly/lib/test/flow_branching.rs index e4363480ff..41990ee37f 100644 --- a/pyrefly/lib/test/flow_branching.rs +++ b/pyrefly/lib/test/flow_branching.rs @@ -951,7 +951,6 @@ c = C() while True: if True: c = C() - break assert_type(c, C) "#, @@ -1509,7 +1508,7 @@ for x in []: pass # E: Expected an indented block while True: -pass # E: Expected an indented block # E: This code is unreachable +pass # E: Expected an indented block "#, ); @@ -2907,6 +2906,7 @@ def f() -> int: // elif condition only executes if the first `if` was False — walrus may not run. testcase!( + bug = "In order to fix false positives, we handled narrows differently in if/elif and introduced a false negative here", test_walrus_in_elif, r#" def condition() -> bool: ... @@ -2915,7 +2915,7 @@ def f() -> bool: pass elif (x := condition()): pass - return x # E: `x` may be uninitialized + return x # False negative: x winds up getting applied as if it were in the base flow due to the negative narrow "#, ); @@ -3003,6 +3003,7 @@ def f() -> None: // the walrus may not execute, so x should be possibly-uninitialized. testcase!( + bug = "Should report x as possibly uninitialized since the if branch does not terminate", test_walrus_in_elif_preceding_if_no_terminate, r#" def foo() -> bool: @@ -3016,7 +3017,7 @@ def f() -> None: pass elif (x := bar()) > 1: pass - print(x) # E: `x` may be uninitialized + print(x) # should be an error: x may be uninitialized "#, ); diff --git a/pyrefly/lib/test/lsp/hover_type.rs b/pyrefly/lib/test/lsp/hover_type.rs index c7c75daaf7..eb05592a6b 100644 --- a/pyrefly/lib/test/lsp/hover_type.rs +++ b/pyrefly/lib/test/lsp/hover_type.rs @@ -549,7 +549,7 @@ while True: y = x # ^ "#; - let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report); assert_eq!( r#" # main.py diff --git a/pyrefly/lib/test/paramspec.rs b/pyrefly/lib/test/paramspec.rs index e0128661d9..3a2337e5a9 100644 --- a/pyrefly/lib/test/paramspec.rs +++ b/pyrefly/lib/test/paramspec.rs @@ -487,7 +487,7 @@ def run[**P, R](func: Callable[P, R], *args: P.args, **kwargs: P.kwargs) -> R: return func(*args,**kwargs) except Exception: pass - raise NotImplementedError("Unreachable") # E: This code is unreachable + raise NotImplementedError("Unreachable") "#, ); diff --git a/pyrefly/lib/test/perf.rs b/pyrefly/lib/test/perf.rs index 0059cf3648..8e8155973c 100644 --- a/pyrefly/lib/test/perf.rs +++ b/pyrefly/lib/test/perf.rs @@ -302,3 +302,37 @@ fn test_deeply_nested_expression_is_rejected() { errors[0].msg() ); } + +// Deferred attribute proofs must remain bounded even when the diagnostic is disabled. +#[test] +fn test_attribute_initialization_branch_growth() { + let mut source = String::from("def proceed() -> None: ...\nclass C:\n"); + for method in ["__init__", "helper"] { + source.push_str(&format!(" def {method}(self, flag: bool) -> None:\n")); + for attribute in 0..16 { + source.push_str(&format!( + " if flag:\n self.{method}_value_{attribute} = 1\n else:\n proceed()\n", + )); + } + for _ in 0..64 { + source.push_str( + " if flag:\n proceed()\n else:\n proceed()\n", + ); + } + } + for enabled in [false, true] { + let env = TestEnv::one("main", &source); + let env = if enabled { + env.enable_possibly_uninitialized_attribute_error() + } else { + env + }; + let (state, handle) = env.to_state(); + let errors = state + .transaction() + .get_errors([&handle("main")]) + .collect_errors() + .ordinary; + assert_eq!(errors.len(), if enabled { 16 } else { 0 }, "{errors:#?}"); + } +} diff --git a/pyrefly/lib/test/scope.rs b/pyrefly/lib/test/scope.rs index 7c3897c358..9302ce056e 100644 --- a/pyrefly/lib/test/scope.rs +++ b/pyrefly/lib/test/scope.rs @@ -1237,7 +1237,7 @@ def try_except_twice(): except Exception: e3 # E: `e3` is uninitialized - e3 # E: `e3` is uninitialized + e3 # E: `e3` may be uninitialized def try_except_multiple_finally(): try: diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 8d1f40f86b..81236e9285 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1876,7 +1876,7 @@ The `strict` preset enables this check as an error. Pyrefly checks whether an assigned attribute exists after the class body or a recognized initialization method completes. Recognized methods include `__init__`, `__new__`, `__post_init__`, `__init_subclass__`, and supported test setup methods. Each path that completes normally, including an explicit `return`, must leave the attribute initialized. -Paths that raise, loop forever, or end in a `Never` or `NoReturn` call do not require an assignment. +Paths that raise or end in a `Never` or `NoReturn` call do not require an assignment. Deleting an attribute removes its initialization guarantee. ```python @@ -1891,6 +1891,7 @@ The check permits an inherited attribute when a base class other than `object` d An instance `__getattr__` fallback also suppresses this error for instance attributes. Pyrefly combines assignments from different methods leniently: one method that initializes the attribute on every completing path suffices. This includes ordinary helper methods, even when Pyrefly cannot prove that the constructor calls them. +Class-body definitions are checked separately, even when a method assigns the same name. Assignments inside nested functions do not initialize the enclosing method's attributes. Pyrefly reports this error at the attribute definition, even if the attribute is never read. From 4abd5caff12d31b18104fc6f0bc21dadbe201b9b Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 23 Oct 2026 10:33:38 +0900 Subject: [PATCH 09/10] . --- pyrefly/lib/binding/scope.rs | 8 ++++++++ pyrefly/lib/binding/stmt.rs | 11 +++++++++++ pyrefly/lib/test/attributes.rs | 3 +-- pyrefly/lib/test/delayed_inference.rs | 2 +- pyrefly/lib/test/flow_branching.rs | 3 ++- pyrefly/lib/test/lsp/hover_type.rs | 2 +- pyrefly/lib/test/paramspec.rs | 2 +- 7 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 644dc40e08..e1f573764d 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -4895,6 +4895,7 @@ impl<'a> BindingsBuilder<'a> { is_while_true: bool, loop_definitely_runs: bool, ) { + let loop_raises = self.scopes.terminated_by_raise(); let finished_loop = self.scopes.finish_loop(); let (breaks, other_exits): (Vec, Vec) = finished_loop .exits @@ -4933,6 +4934,13 @@ impl<'a> BindingsBuilder<'a> { // also know that breaks are the only way to exit, so we drop the current flow, // which is actually unreachable. // + if is_while_true && breaks.is_empty() { + self.scopes.mark_flow_termination(if loop_raises { + TerminationKind::Raise + } else { + TerminationKind::Jump + }); + } if let Some(base) = base_if_breaks { if is_while_true { self.merge_flow(base, breaks, other_range, MergeStyle::Exclusive) diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index 446abc8cf7..d6f728bf9f 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -1499,6 +1499,7 @@ impl<'a> BindingsBuilder<'a> { ); } Stmt::While(mut x) => { + let was_unreachable = self.scopes.is_definitely_unreachable(); self.setup_loop(x.range, &SmallSet::new()); // Note that it is important we ensure *after* we set up the loop, so that both the // narrowing and type checking are aware that the test might be impacted by changes @@ -1549,6 +1550,16 @@ impl<'a> BindingsBuilder<'a> { is_while_true, is_while_true, ); + if is_while_true + && !test_is_environment_independent + && self.scopes.has_terminated() + && !was_unreachable + { + // Another Python environment can skip the loop. + self.scopes.set_definitely_unreachable(false); + self.scopes + .mark_flow_termination(TerminationKind::StaticTest); + } } Stmt::If(mut x) => { let is_definitely_unreachable = self.scopes.is_definitely_unreachable(); diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index d10ae507cf..537e033261 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -110,7 +110,6 @@ class EarlyReturn: ); testcase!( - bug = "Retry loops retain a spurious fallthrough path", test_attribute_initialization_while_true_return, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -120,7 +119,7 @@ class Retry: def __init__(self) -> None: while True: try: - self.conn = connect() # E: Attribute `conn` may be uninitialized + self.conn = connect() return except OSError: continue diff --git a/pyrefly/lib/test/delayed_inference.rs b/pyrefly/lib/test/delayed_inference.rs index b74bdb36dd..204dc0a83c 100644 --- a/pyrefly/lib/test/delayed_inference.rs +++ b/pyrefly/lib/test/delayed_inference.rs @@ -45,7 +45,7 @@ from typing import assert_type, Literal x = 5 while True: x = x + 1 -y = x +y = x # E: This code is unreachable assert_type(y, int) "#, ); diff --git a/pyrefly/lib/test/flow_branching.rs b/pyrefly/lib/test/flow_branching.rs index 41990ee37f..c4615ba8fa 100644 --- a/pyrefly/lib/test/flow_branching.rs +++ b/pyrefly/lib/test/flow_branching.rs @@ -951,6 +951,7 @@ c = C() while True: if True: c = C() + break assert_type(c, C) "#, @@ -1508,7 +1509,7 @@ for x in []: pass # E: Expected an indented block while True: -pass # E: Expected an indented block +pass # E: Expected an indented block # E: This code is unreachable "#, ); diff --git a/pyrefly/lib/test/lsp/hover_type.rs b/pyrefly/lib/test/lsp/hover_type.rs index eb05592a6b..c7c75daaf7 100644 --- a/pyrefly/lib/test/lsp/hover_type.rs +++ b/pyrefly/lib/test/lsp/hover_type.rs @@ -549,7 +549,7 @@ while True: y = x # ^ "#; - let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report); + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); assert_eq!( r#" # main.py diff --git a/pyrefly/lib/test/paramspec.rs b/pyrefly/lib/test/paramspec.rs index 3a2337e5a9..e0128661d9 100644 --- a/pyrefly/lib/test/paramspec.rs +++ b/pyrefly/lib/test/paramspec.rs @@ -487,7 +487,7 @@ def run[**P, R](func: Callable[P, R], *args: P.args, **kwargs: P.kwargs) -> R: return func(*args,**kwargs) except Exception: pass - raise NotImplementedError("Unreachable") + raise NotImplementedError("Unreachable") # E: This code is unreachable "#, ); From 7e67b9fa274bda06a001a6900d966eb2befb2baf Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Sat, 24 Oct 2026 01:58:18 +0900 Subject: [PATCH 10/10] Preserve upstream local initialization checks --- pyrefly/lib/binding/class.rs | 1 - pyrefly/lib/binding/scope.rs | 160 +++++++++++++---------------- pyrefly/lib/test/flow_branching.rs | 6 +- pyrefly/lib/test/scope.rs | 2 +- 4 files changed, 74 insertions(+), 95 deletions(-) diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 64d8190506..278bb9673a 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -486,7 +486,6 @@ impl<'a> BindingsBuilder<'a> { let django_field_info = self.extract_django_fields_from_class_body(&field_definitions); let mut fields = SmallMap::with_capacity(field_definitions.len()); let mut django_relation_fields = Vec::new(); - for (name, (definition, range, initialized)) in field_definitions.into_iter_hashed() { if let ClassFieldDefinition::AssignedInBody { value, .. } = &definition && let ExprOrBinding::Expr(e) = value.as_ref() diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index e1f573764d..a7e9b21db6 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -791,9 +791,6 @@ struct FlowInfo { struct FlowValue { idx: Idx, style: FlowStyle, - /// True when every reaching value comes from a function definition. - /// Branch merges preserve this flag when they replace the flow style. - is_function_def: bool, } /// The most recent narrow for a name. @@ -805,11 +802,7 @@ struct FlowNarrow { impl FlowInfo { fn new_value(idx: Idx, style: FlowStyle) -> Self { Self { - value: Some(FlowValue { - idx, - is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), - style, - }), + value: Some(FlowValue { idx, style }), narrow: None, narrow_depth: 0, loop_prior: idx, @@ -827,11 +820,7 @@ impl FlowInfo { fn updated_value(&self, idx: Idx, style: FlowStyle, in_loop: bool) -> Self { Self { - value: Some(FlowValue { - idx, - is_function_def: matches!(style, FlowStyle::FunctionDef { .. }), - style, - }), + value: Some(FlowValue { idx, style }), // Note that any existing narrow is wiped when a new value is bound. narrow: None, narrow_depth: 0, @@ -1873,7 +1862,7 @@ impl Scopes { &mut class_scope.attributes_from_other_methods, ] { if let Some(attributes) = attributes.get_mut(method_name) { - for attribute in attributes.values_mut() { + for (attribute, _) in attributes.values_mut() { if method_range.contains_range(attribute.2) { attribute.3 = MethodSelfKind::Class; } @@ -2831,7 +2820,6 @@ impl Scopes { scope.flow.excluded_none_pairs.invalidate(name); if let Some(value) = scope.flow.get_value_mut(name) { value.style = FlowStyle::Uninitialized; - value.is_function_def = false; } scope.deleted_names.insert(name.clone()); } @@ -3592,7 +3580,7 @@ impl Scopes { }, }; let initialized = match &value.style { - _ if value.is_function_def => InitializedInFlow::Yes, + _ if static_info.function_def_only => InitializedInFlow::Yes, FlowStyle::PossiblyUninitialized => InitializedInFlow::Conditionally, FlowStyle::MaybeInitialized(keys) => { InitializedInFlow::DeferredCheck(keys.clone()) @@ -4247,33 +4235,30 @@ enum DefinitionStatus { /// Determines the definition status of a variable after a merge. /// -/// The logic differs slightly for `LoopDefinitelyRuns` (where base having a value -/// or all loop body branches having values is sufficient) vs other merge styles. +/// For `LoopDefinitelyRuns`, the base having a value is also sufficient, since the +/// loop body runs at least once. fn determine_definition_status( merge_style: MergeStyle, base_has_value: bool, n_values: usize, + n_possibly_set_branches: usize, n_branches: usize, - n_missing_branches: usize, - n_branches_with_termination_key: usize, + n_unterminated_missing_branches: usize, missing_branch_termination_keys: Vec>, ) -> DefinitionStatus { match merge_style { MergeStyle::LoopDefinitelyRuns if base_has_value => DefinitionStatus::Defined, - MergeStyle::LoopDefinitelyRuns if n_values == n_branches => DefinitionStatus::Defined, - MergeStyle::LoopDefinitelyRuns if n_missing_branches <= n_branches_with_termination_key => { - if !missing_branch_termination_keys.is_empty() { - DefinitionStatus::DeferredCheck(missing_branch_termination_keys) - } else { - DefinitionStatus::Defined - } - } _ if n_values == n_branches => DefinitionStatus::Defined, - _ if n_missing_branches <= n_branches_with_termination_key => { - if !missing_branch_termination_keys.is_empty() { - DefinitionStatus::DeferredCheck(missing_branch_termination_keys) - } else { + // With no branch that defines the name or may have set it, the name is unset on every + // path that reaches the merge, whichever branches turn out to terminate. + _ if n_values == 0 && n_possibly_set_branches == 0 => DefinitionStatus::NotDefined, + // Every branch that leaves the name unset may still never reach the merge, which only + // its own termination key can tell. + _ if n_unterminated_missing_branches == 0 => { + if missing_branch_termination_keys.is_empty() { DefinitionStatus::Defined + } else { + DefinitionStatus::DeferredCheck(missing_branch_termination_keys) } } _ => DefinitionStatus::NotDefined, @@ -4453,7 +4438,6 @@ impl<'a> BindingsBuilder<'a> { phi_key: PhiKey, merge_style: MergeStyle, n_branches: usize, - n_branches_with_termination_key: usize, ) -> FlowInfo { // Only a promised phi can already appear in a branch's flow. let phi_idx = phi_key.promised; @@ -4506,18 +4490,45 @@ impl<'a> BindingsBuilder<'a> { let mut branch_idxs = SmallSet::with_capacity(merge_branches.len()); let mut branch_infos = Vec::with_capacity(merge_branches.len()); let mut styles = Vec::with_capacity(merge_branches.len()); - let mut is_function_def = true; + let mut styled_idxs = SmallSet::with_capacity(merge_branches.len()); let mut n_values = 0; + let mut n_possibly_set_branches = 0; // Collect termination keys from branches that don't define the variable. // These will be used for deferred uninitialized checks at solve time. let mut missing_branch_termination_keys = Vec::new(); - for merge_branch in merge_branches.into_iter() { - // Handle branches that don't have this name at all (flow_info is None) - let Some(flow_info) = merge_branch.flow_info else { - // This branch doesn't have this name - record its termination key if any - if let Some(termination_key) = merge_branch.termination_key { - missing_branch_termination_keys.push(termination_key); + // A plain loop's base is a path past the loop even when the name is absent from it, in + // which case the base is not among the merged branches. + let mut n_unterminated_missing_branches = + usize::from(matches!(merge_style, MergeStyle::Loop) && !added_base_to_merge); + // The base is merged last, and only for loops. A loop that definitely runs never + // reaches the merge from its base, so the base does not count as a missing branch. + let base_position = (added_base_to_merge + && matches!(merge_style, MergeStyle::LoopDefinitelyRuns)) + .then(|| merge_branches.len() - 1); + for (position, merge_branch) in merge_branches.into_iter().enumerate() { + let style = merge_branch + .flow_info + .as_ref() + .and_then(|flow_info| flow_info.value.as_ref()) + .map(|v| &v.style); + // A branch that may have left the name unset, such as one carrying what a loop that + // may run zero times left behind, leaves it unset at the merge only if the branch + // reaches it. With a termination key, that is decided at solve time like a missing + // branch, and its style does not count. Without one, its style carries the doubt. + let unset_unless_terminated = matches!(style, Some(FlowStyle::PossiblyUninitialized)) + && merge_branch.termination_key.is_some(); + n_possibly_set_branches += usize::from(unset_unless_terminated); + // A branch with FlowStyle::Uninitialized (e.g., after exception variable + // unbinding via mark_as_deleted) does not define the variable. + let defines = style.is_some_and(|style| !matches!(style, FlowStyle::Uninitialized)) + && !unset_unless_terminated; + if !defines && Some(position) != base_position { + match merge_branch.termination_key { + Some(termination_key) => missing_branch_termination_keys.push(termination_key), + None => n_unterminated_missing_branches += 1, } + } + let Some(flow_info) = merge_branch.flow_info else { continue; }; let branch_idx = flow_info.idx(); @@ -4534,63 +4545,47 @@ impl<'a> BindingsBuilder<'a> { } if let Some(v) = flow_info.value { - // A branch with FlowStyle::Uninitialized (e.g., after exception variable - // unbinding via mark_as_deleted) should not count as defining the variable. - // We still track its idx for type inference but don't count it as a value. - let is_uninitialized = matches!(v.style, FlowStyle::Uninitialized); - if !is_uninitialized { + // A branch that does not define the name still contributes its idx to type + // inference. + if defines { n_values += 1; } if Some(v.idx) == phi_idx { - // If uninitialized, still track termination key before continuing. - if is_uninitialized && let Some(termination_key) = merge_branch.termination_key - { - missing_branch_termination_keys.push(termination_key); - } continue; } - is_function_def &= v.is_function_def; - if value_idxs.insert(v.idx) { - // An invariant in Pyrefly is that we only set style when we - // set a value, so duplicate value_idxs always have the same style. + value_idxs.insert(v.idx); + // An invariant in Pyrefly is that we only set style when we + // set a value, so duplicate value_idxs always have the same style. + if !unset_unless_terminated && styled_idxs.insert(v.idx) { styles.push(v.style); } - // Treat uninitialized branches like missing branches for termination keys. - if is_uninitialized && let Some(termination_key) = merge_branch.termination_key { - missing_branch_termination_keys.push(termination_key); - } - } else { - // This branch doesn't have a value for the variable. - // If it has a termination key, track it for deferred checking. - if let Some(termination_key) = merge_branch.termination_key { - missing_branch_termination_keys.push(termination_key); - } } branch_idxs.insert(branch_idx); } - // n_total_branches is the actual number of branches we iterated over, which includes - // the base flow for loops (since base is added to merge_branches for type inference). - let n_total_branches = if added_base_to_merge { - n_branches + 1 - } else { - n_branches - }; - let n_missing_branches = n_total_branches - n_values; let definition_status = determine_definition_status( merge_style, base_has_value, n_values, + n_possibly_set_branches, n_branches, - n_missing_branches, - n_branches_with_termination_key, + n_unterminated_missing_branches, missing_branch_termination_keys, ); // Helper to compute the final FlowStyle based on definition status. let compute_final_style = |styles: Vec| -> FlowStyle { match &definition_status { - DefinitionStatus::DeferredCheck(keys) => FlowStyle::MaybeInitialized(keys.clone()), + // Every branch that leaves the name unset has a key, so the keys decide those + // branches. A branch that may hold the name without a key keeps its doubt. + DefinitionStatus::DeferredCheck(keys) => FlowStyle::merged( + true, + styles + .into_iter() + .filter(|style| !matches!(style, FlowStyle::Uninitialized)) + .chain([FlowStyle::MaybeInitialized(keys.clone())]), + merge_style, + ), DefinitionStatus::Defined => { FlowStyle::merged(true, styles.into_iter(), merge_style) } @@ -4634,7 +4629,6 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: *value_idxs.first().unwrap(), style: compute_final_style(styles), - is_function_def, }), narrow: Some(FlowNarrow { idx: merged_idx }), narrow_depth: 1, @@ -4656,7 +4650,6 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: merged_idx, style: compute_final_style(styles), - is_function_def, }), narrow: None, narrow_depth: 0, @@ -4718,10 +4711,6 @@ impl<'a> BindingsBuilder<'a> { 0 }; - // Count how many branches have a last_stmt_expr (potential type-based termination) - let n_branches_with_termination_key = - flows.iter().filter(|f| f.last_stmt_expr.is_some()).count(); - // Collect all termination keys from flows (for building dense MergeItems) let all_termination_keys: Vec>> = flows.iter().map(|f| f.last_stmt_expr).collect(); @@ -4809,13 +4798,7 @@ impl<'a> BindingsBuilder<'a> { promised: self.promised_idx(&key), key, }; - let flow_info = self.merged_flow_info( - merge_item, - phi_key, - merge_style, - n_branches, - n_branches_with_termination_key, - ); + let flow_info = self.merged_flow_info(merge_item, phi_key, merge_style, n_branches); merged_flow_infos.insert_hashed(name, flow_info); } @@ -4858,7 +4841,6 @@ impl<'a> BindingsBuilder<'a> { info.value = Some(FlowValue { idx: phi_idx, style: FlowStyle::LoopRecursion, - is_function_def: false, }); info.narrow = None; } diff --git a/pyrefly/lib/test/flow_branching.rs b/pyrefly/lib/test/flow_branching.rs index c4615ba8fa..e4363480ff 100644 --- a/pyrefly/lib/test/flow_branching.rs +++ b/pyrefly/lib/test/flow_branching.rs @@ -2907,7 +2907,6 @@ def f() -> int: // elif condition only executes if the first `if` was False — walrus may not run. testcase!( - bug = "In order to fix false positives, we handled narrows differently in if/elif and introduced a false negative here", test_walrus_in_elif, r#" def condition() -> bool: ... @@ -2916,7 +2915,7 @@ def f() -> bool: pass elif (x := condition()): pass - return x # False negative: x winds up getting applied as if it were in the base flow due to the negative narrow + return x # E: `x` may be uninitialized "#, ); @@ -3004,7 +3003,6 @@ def f() -> None: // the walrus may not execute, so x should be possibly-uninitialized. testcase!( - bug = "Should report x as possibly uninitialized since the if branch does not terminate", test_walrus_in_elif_preceding_if_no_terminate, r#" def foo() -> bool: @@ -3018,7 +3016,7 @@ def f() -> None: pass elif (x := bar()) > 1: pass - print(x) # should be an error: x may be uninitialized + print(x) # E: `x` may be uninitialized "#, ); diff --git a/pyrefly/lib/test/scope.rs b/pyrefly/lib/test/scope.rs index 9302ce056e..7c3897c358 100644 --- a/pyrefly/lib/test/scope.rs +++ b/pyrefly/lib/test/scope.rs @@ -1237,7 +1237,7 @@ def try_except_twice(): except Exception: e3 # E: `e3` is uninitialized - e3 # E: `e3` may be uninitialized + e3 # E: `e3` is uninitialized def try_except_multiple_finally(): try: