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/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 dcb9c0ebb1..ee559c9f16 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3212,22 +3212,27 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { name, range, class, - termination_keys, + is_instance, + initializations, } => { - let initialized = termination_keys - .as_ref() - .is_some_and(|keys| keys.iter().all(|key| self.get_idx(*key).ty().is_never())); - let inherited = !initialized + let initialized = initializations.iter().any(|initialized| { + initialized.is_initialized(|key| self.get_idx(key).ty().is_never()) + }); + let inherited_or_dynamic = !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() - }) + (*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 { + if !initialized && !inherited_or_dynamic { errors .error_builder( *range, diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index b4e8bf7b30..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; @@ -1366,13 +1367,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 attribute initialization from the class body or any defining method. UninitializedAttributeCheck { name: Name, range: TextRange, class: Idx, - /// All missing paths must terminate. None means a path lacks termination evidence. - termination_keys: Option>>, + /// 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 @@ -1549,15 +1552,17 @@ impl DisplayWith for BindingExpect { name, range, class, - termination_keys, + is_instance, + initializations, } => { write!( f, - "UninitializedAttributeCheck({}, {}, {:?}, {:?})", + "UninitializedAttributeCheck({}, {}, {:?}, {}, {:?})", name, ctx.module().display(range), class, - termination_keys + is_instance, + initializations ) } Self::ForwardRefUnion { diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index 49fc7a787f..397659feb8 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -214,6 +214,107 @@ 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 { + const MAX_DEFERRED_PATHS: usize = 16; + 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(); + 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) + } + } + + 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, + /// 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), +} + +impl From for InitializedInClass { + fn from(initialized: InitializationStatus) -> Self { + match initialized { + InitializationStatus::Initialized => Self::Yes, + InitializationStatus::Uninitialized => Self::IfAny(Vec::new()), + deferred @ InitializationStatus::Deferred(_) => Self::IfAny(vec![deferred]), + } + } +} + pub type BindingEntry = (Index, IndexMap::Value>); table! { @@ -3293,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 12e5089095..278bb9673a 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::InitializedInClass; 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,7 @@ 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,17 +510,31 @@ impl<'a> BindingsBuilder<'a> { let attrs_field_specifier = self.attrs_field_specifier(&definition, name.key(), range, &attrs_decorators); + let check_initialization = !matches!( + &definition, + ClassFieldDefinition::DefinedInMethod { method, .. } + if !method.recognized_attribute_defining_method + ); // Stub files declare attributes without runtime initialization. if !self.module_info.path().is_interface() - && let Some(termination_keys) = uninitialized_fields.get(name.key()) + && check_initialization + && let InitializedInClass::IfAny(initializations) = initialized { + let is_instance = matches!( + &definition, + ClassFieldDefinition::DefinedInMethod { + receiver_kind: MethodSelfKind::Instance, + .. + } + ); self.insert_binding( KeyExpect::UninitializedAttributeCheck(range), BindingExpect::UninitializedAttributeCheck { name: name.key().clone(), range, class: class_indices.class_idx, - termination_keys: termination_keys.clone(), + is_instance, + initializations, }, ); } diff --git a/pyrefly/lib/binding/django.rs b/pyrefly/lib/binding/django.rs index a5ca5b4e97..3b2b67f83e 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::InitializedInClass; 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..557e5da5da 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::InitializationStatus; use crate::binding::bindings::LegacyTParamCollector; use crate::binding::classmethod::FunctionDecorators; use crate::binding::expr::Usage; @@ -173,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, } @@ -266,11 +267,14 @@ impl<'a> SelfAttrNames<'a> { .map(|(n, r)| { ( n, - InstanceAttribute( - vec![ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit))], - None, - r, - MethodSelfKind::Instance, + ( + 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 17eeb50ca4..a7e9b21db6 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -71,6 +71,8 @@ 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; use crate::binding::expr::Usage; @@ -652,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 @@ -1094,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, @@ -1120,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) { @@ -1149,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), + ) + }) }) } } @@ -1230,6 +1251,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 +1325,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, @@ -1839,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; } @@ -2308,16 +2331,42 @@ 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(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, + 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 +2400,21 @@ impl Scopes { x: &ExprAttribute, value: ExprOrBinding, annotation: Option>, + has_value: bool, ) -> bool { + 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 has_value && !in_nested_function { + scope + .flow + .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. attr.0.push(value); @@ -2378,6 +2436,7 @@ impl Scopes { } return true; } + in_nested_function |= matches!(scope.kind, ScopeKind::Method(_)); } false } @@ -2422,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( @@ -3300,15 +3380,31 @@ 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 { + method + .return_attribute_initialization + .push(scope.flow.instance_attribute_initialization.clone()); + } + 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 +3463,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 +3579,33 @@ 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 { + _ if static_info.function_def_only => InitializedInFlow::Yes, + 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, + InitializationStatus::from(initialized).into(), + ), + ); } }); // 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 +3615,7 @@ impl Scopes { has_instance_write: existing_instance_write, }, _, + existing_initialized, )) = field_definitions.get_mut(name.key()) { if matches!(receiver_kind, MethodSelfKind::Instance) { @@ -3523,8 +3624,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. @@ -3538,6 +3639,19 @@ impl Scopes { *existing_receiver = MethodSelfKind::Class; } } + // Initialization is merged independently of definition priority. + // Any method can establish initialization, so combine their requirements with OR. + match (&mut *existing_initialized, initialized) { + (InitializedInClass::Yes, _) => {} + (_, InitializationStatus::Initialized) => { + *existing_initialized = InitializedInClass::Yes; + } + ( + InitializedInClass::IfAny(groups), + deferred @ InitializationStatus::Deferred(_), + ) => groups.push(deferred), + (InitializedInClass::IfAny(_), InitializationStatus::Uninitialized) => {} + } } else if !field_definitions.contains_key_hashed(name.as_ref()) { field_definitions.insert_hashed( name, @@ -3553,16 +3667,13 @@ impl Scopes { ), }, range, + initialized.into(), ), ); } }, ); - ( - 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 @@ -4611,6 +4722,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(); @@ -4667,6 +4806,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, diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index fd2e62d8ed..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,6 +1290,7 @@ impl<'a> BindingsBuilder<'a> { None, ), ); + let has_value = x.value.is_some(); let value = match x.value { Some(mut assigned) => { self.bind_attr_assign(attr.clone(), &mut assigned, |_attr, value, _| { @@ -1303,10 +1307,12 @@ impl<'a> BindingsBuilder<'a> { ExprOrBinding::Binding(Binding::Any(AnyStyle::Implicit)) } }; - 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), + has_value, + ) { self.error( x.range, ErrorKind::BadAssignment, diff --git a/pyrefly/lib/binding/target.rs b/pyrefly/lib/binding/target.rs index bb3191fa12..64c568ea70 100644 --- a/pyrefly/lib/binding/target.rs +++ b/pyrefly/lib/binding/target.rs @@ -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, 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 273c2d0523..537e033261 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -33,7 +33,6 @@ class Helper: ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_attribute_initialization_inherited, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -48,7 +47,7 @@ class Child(Base): if t is not None: self.timeout = t self.value = t - self.own = t + self.own = t # E: Attribute `own` may be uninitialized class Grandchild(Child): def __init__(self, t: int | None = None) -> None: @@ -63,7 +62,6 @@ class ClassBody(Base): ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_attribute_initialization_trailing_noreturn, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -106,13 +104,12 @@ class EarlyReturn: def __init__(self, flag: bool) -> None: if flag: return - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized 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#" @@ -132,33 +129,32 @@ class EarlyReturn: while True: if flag: return - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized return class ReturnOrBreak: def __init__(self, flag: bool) -> None: while True: if flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized 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 + self.value = 1 # E: Attribute `value` may be uninitialized class ConditionalOuter: def __init__(self, flag: bool) -> None: if flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized def later() -> None: self.value = 2 @@ -172,7 +168,7 @@ class NestedMethod: def __init__(self) -> None: class Inner: def later(inner) -> None: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized class NestedReturn: def __init__(self) -> None: @@ -183,7 +179,6 @@ class NestedReturn: ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_attribute_initialization_control_flow, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -200,7 +195,7 @@ class TryExcept: class TryExceptMissing: def __init__(self, raw: str) -> None: try: - self.value = int(raw) + self.value = int(raw) # E: Attribute `value` may be uninitialized except ValueError: pass @@ -220,7 +215,7 @@ class WithConditional: def __init__(self, flag: bool) -> None: with nullcontext(): if flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized class Match: def __init__(self, kind: Literal["a", "b"]) -> None: @@ -234,16 +229,16 @@ class MatchMissing: def __init__(self, kind: str) -> None: match kind: case "a": - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized class Deleted: def __init__(self) -> None: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized del self.value class DeletedConditionally: def __init__(self, flag: bool) -> None: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized if flag: del self.value @@ -256,7 +251,6 @@ class Reassigned: ); 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#" @@ -286,7 +280,7 @@ class ConditionalPostInit: flag: bool def __post_init__(self) -> None: if self.flag: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized class SubclassHook: def __init_subclass__(cls) -> None: @@ -295,7 +289,7 @@ class SubclassHook: class ConditionalSubclassHook: def __init_subclass__(cls, flag: bool = False) -> None: if flag: - cls.value = 1 + cls.value = 1 # E: Attribute `value` may be uninitialized class ClassLoop: for value in [1]: @@ -322,6 +316,7 @@ class C: ); testcase!( + bug = "Return snapshots do not include assignments from finally", test_attribute_initialization_finally_after_return, TestEnv::new().enable_possibly_uninitialized_attribute_error(), r#" @@ -330,7 +325,7 @@ class C: try: return finally: - self.value = 1 + self.value = 1 # E: Attribute `value` may be uninitialized "#, ); @@ -367,7 +362,6 @@ 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#" @@ -375,14 +369,13 @@ 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] + 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` "#, ); testcase!( - bug = "Attributes assigned in methods are not checked for initialization", test_possibly_uninitialized_attribute_stub, TestEnv::one_with_path( "foo", @@ -421,7 +414,7 @@ class Implementation: def __init__(self, flag: bool) -> None: if flag: - self.instance_value = 1 + self.instance_value = 1 # E: Attribute `instance_value` may be uninitialized "#, ); @@ -702,7 +695,6 @@ 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#" @@ -730,7 +722,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: @@ -765,7 +757,6 @@ 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#" @@ -805,7 +796,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() @@ -840,7 +831,6 @@ 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#" @@ -858,7 +848,7 @@ class BeforeLoop: class PossiblyEmpty: def __init__(self, values: list[int]) -> None: for value in values: - self.value = value + self.value = value # E: Attribute `value` may be uninitialized class NonEmpty: def __init__(self) -> None: @@ -876,14 +866,14 @@ class ConditionalContinue: for value in [1]: if flag: continue - self.value = value + 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 + self.value = 1 # E: Attribute `value` may be uninitialized break class LoopElse: @@ -917,7 +907,6 @@ 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#" @@ -945,7 +934,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 @@ -958,7 +947,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 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 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: diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 500e451de8..81236e9285 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1873,22 +1873,33 @@ 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. +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 or end in a `Never` or `NoReturn` call do not require an assignment. +Deleting an attribute removes its initialization guarantee. ```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 + # The path where value is None completes without assigning self.value. ``` -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. +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. +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. -To disable this check while retaining other strict checks, use: +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 preset = "strict"