diff --git a/crates/pyrefly_config/src/base.rs b/crates/pyrefly_config/src/base.rs index 60b914899d..07dcb07dcc 100644 --- a/crates/pyrefly_config/src/base.rs +++ b/crates/pyrefly_config/src/base.rs @@ -203,6 +203,7 @@ impl Preset { (ErrorKind::MethodAssign, Severity::Error), (ErrorKind::MissingOverrideDecorator, Severity::Error), (ErrorKind::OpenUnpacking, Severity::Error), + (ErrorKind::PossiblyUninitializedAttribute, Severity::Error), (ErrorKind::PotentialBadKeywordArgument, Severity::Error), (ErrorKind::UnusedIgnore, Severity::Error), ]); diff --git a/crates/pyrefly_config/src/config.rs b/crates/pyrefly_config/src/config.rs index 482bb609be..e9ef927cad 100644 --- a/crates/pyrefly_config/src/config.rs +++ b/crates/pyrefly_config/src/config.rs @@ -3960,6 +3960,38 @@ output-format = "omit-errors" assert_eq!(config.root.strict_partial_subtyping, Some(true)); } + #[test] + fn test_possibly_uninitialized_attribute_configuration() { + for (source, initialization_severity, missing_severity) in [ + ("", Severity::Ignore, Severity::Error), + ("preset = 'strict'", Severity::Error, Severity::Error), + ( + "preset = 'strict'\n[errors]\npossibly-uninitialized-attribute = 'ignore'", + Severity::Ignore, + Severity::Error, + ), + ( + "preset = 'strict'\n[errors]\nmissing-attribute = 'ignore'", + Severity::Error, + Severity::Ignore, + ), + ] { + let mut config = ConfigFile::parse_config(source).unwrap(); + config.configure(); + let errors = config.root.errors.as_ref().unwrap(); + assert_eq!( + errors.severity(ErrorKind::PossiblyUninitializedAttribute), + initialization_severity, + "configuration: {source}", + ); + assert_eq!( + errors.severity(ErrorKind::MissingAttribute), + missing_severity, + "configuration: {source}", + ); + } + } + #[test] fn test_preset_off_silences_all_errors() { // The `off` preset silences every error kind and leaves all other diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index ddff1e2eaa..9f48799a62 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -377,6 +377,8 @@ 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. + 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 /// guaranteed. This is a separate error code from BadKeywordArgument to allow @@ -653,6 +655,7 @@ impl ErrorKind { ErrorKind::NotRequiredKeyAccess => Severity::Ignore, ErrorKind::OpenUnpacking => Severity::Ignore, ErrorKind::OrderedDataclassInheritance => Severity::Warn, + ErrorKind::PossiblyUninitializedAttribute => Severity::Ignore, ErrorKind::PotentialBadKeywordArgument => Severity::Ignore, ErrorKind::PytorchEfficiencyLintCudaCall => Severity::Ignore, ErrorKind::PytorchEfficiencyLintItemCall => Severity::Ignore, diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index ef271571c2..60266e3132 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3199,6 +3199,35 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { .emit(); } } + BindingExpect::UninitializedAttributeCheck { + name, + range, + class, + termination_keys, + } => { + let initialized = 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 { + errors + .error_builder( + *range, + ErrorKind::PossiblyUninitializedAttribute, + format!("Attribute `{name}` may be uninitialized"), + ) + .emit(); + } + } BindingExpect::ForwardRefUnion { left, right, diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index b4963a3929..0b7a021de4 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1133,6 +1133,8 @@ pub enum KeyExpect { PrivateAttributeAccess(TextRange), /// Deferred uninitialized variable check. UninitializedCheck(TextRange), + /// Check that an attribute declaration is initialized on every control-flow path. + UninitializedAttributeCheck(TextRange), /// Forward reference string literal in union type check. ForwardRefUnion(TextRange), /// A name used in annotation position that may be an invalid implicit alias. @@ -1159,6 +1161,7 @@ impl Ranged for KeyExpect { | KeyExpect::ExceptClauseReachability(range) | KeyExpect::PrivateAttributeAccess(range) | KeyExpect::UninitializedCheck(range) + | KeyExpect::UninitializedAttributeCheck(range) | KeyExpect::ForwardRefUnion(range) | KeyExpect::ImplicitAliasCheck(range) | KeyExpect::ValidateImplicitReturn(range) @@ -1182,6 +1185,7 @@ impl DisplayWith for KeyExpect { KeyExpect::ExceptClauseReachability(r) => ("ExceptClauseReachability", r), KeyExpect::PrivateAttributeAccess(r) => ("PrivateAttributeAccess", r), KeyExpect::UninitializedCheck(r) => ("UninitializedCheck", r), + KeyExpect::UninitializedAttributeCheck(r) => ("UninitializedAttributeCheck", r), KeyExpect::ForwardRefUnion(r) => ("ForwardRefUnion", r), KeyExpect::ImplicitAliasCheck(r) => ("ImplicitAliasCheck", r), KeyExpect::ValidateImplicitReturn(r) => ("ValidateImplicitReturn", r), @@ -1355,6 +1359,14 @@ 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. + UninitializedAttributeCheck { + name: Name, + range: TextRange, + class: Idx, + /// All missing paths must terminate. None means a path lacks termination evidence. + termination_keys: Option>>, + }, /// Check for forward reference string literal in union type. /// At runtime, `type.__or__` cannot handle string literals, so expressions /// like `int | "str"` will raise a TypeError. @@ -1527,6 +1539,21 @@ impl DisplayWith for BindingExpect { termination_keys ) } + Self::UninitializedAttributeCheck { + name, + range, + class, + termination_keys, + } => { + write!( + f, + "UninitializedAttributeCheck({}, {}, {:?}, {:?})", + name, + ctx.module().display(range), + class, + termination_keys + ) + } Self::ForwardRefUnion { left, right, diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 86b5e08095..f6449c5db8 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -462,7 +462,7 @@ impl<'a> BindingsBuilder<'a> { body, &NestingContext::class(ShortIdentifier::new(&x.name), parent.dupe()), ); - let (field_definitions, nn_module_registrations) = + let (field_definitions, nn_module_registrations, uninitialized_fields) = self.scopes.finish_class_and_get_field_definitions(); self.insert_binding_idx( class_indices.synthesized_fields_idx, @@ -500,6 +500,21 @@ 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(), + }, + ); + } + fields.insert_hashed( name.clone(), ClassFieldProperties::new( diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 51c63f00a1..e2f9f06d10 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -746,6 +746,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. @@ -757,7 +760,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, @@ -775,7 +782,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, @@ -2601,6 +2612,7 @@ impl Scopes { let scope = self.current_mut(); 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()); } @@ -3209,13 +3221,18 @@ 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, 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 { @@ -3325,6 +3342,18 @@ impl Scopes { definition: value.idx, }, }; + if !value.is_function_def { + match &value.style { + FlowStyle::PossiblyUninitialized => { + uninitialized_fields.insert_hashed(name.owned(), None); + } + FlowStyle::MaybeInitialized(keys) => { + uninitialized_fields + .insert_hashed(name.owned(), Some(keys.clone())); + } + _ => {} + } + } field_definitions.insert_hashed(name.owned(), (definition, static_info.range)); } }); @@ -3385,7 +3414,11 @@ impl Scopes { } }, ); - (field_definitions, nn_module_registrations) + ( + field_definitions, + nn_module_registrations, + uninitialized_fields, + ) } /// Return a pair Some((method_name, class_key)) if we are currently in a method @@ -4201,6 +4234,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; @@ -4264,6 +4298,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. @@ -4340,6 +4375,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, @@ -4361,6 +4397,7 @@ impl<'a> BindingsBuilder<'a> { value: Some(FlowValue { idx: merged_idx, style: compute_final_style(styles), + is_function_def, }), narrow: None, narrow_depth: 0, @@ -4514,6 +4551,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 a7733fafa1..52347e763d 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -10,6 +10,122 @@ use crate::test::util::TestEnv; use crate::testcase; +testcase!( + test_class_body_initialization_branches, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +import sys +from typing import TYPE_CHECKING, Never, assert_type + +def coin() -> bool: ... +def stop() -> Never: ... + +class Partial: + if coin(): + value = 1 # E: Attribute `value` may be uninitialized + +class Complete: + if coin(): + value = 1 + else: + value = 2 + +class Terminates: + if coin(): + value = 1 + else: + stop() + +class Base: + value = 0 + +class Child(Base): + if coin(): + value = 1 + +class StaticConditions: + if TYPE_CHECKING: + type_only = 1 + if sys.version_info >= (3, 10): + versioned = 1 + +assert_type(Partial.value, int) +assert_type(Child.value, int) +assert_type(StaticConditions.type_only, int) +assert_type(StaticConditions.versioned, int) +"#, +); + +testcase!( + test_class_body_initialization_methods, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +def coin() -> bool: ... +def callback() -> int: ... + +class C: + if coin(): + value = 1 # E: Attribute `value` may be uninitialized + def method(self) -> int: ... + @staticmethod + def static() -> int: ... + @property + def prop(self) -> int: ... + + def __init__(self) -> None: + self.value = 2 + + if coin(): + callable_value = callback # E: Attribute `callable_value` may be uninitialized + def replaced(self) -> int: ... + replaced = callback # E: Attribute `replaced` may be uninitialized +"#, +); + +testcase!( + test_class_body_initialization_default_disabled, + r#" +def coin() -> bool: ... +class C: + if coin(): + value = 1 +C.missing # E: Class `C` has no class attribute `missing` +"#, +); + +testcase!( + test_class_body_initialization_suppression, + TestEnv::new().enable_possibly_uninitialized_attribute_error(), + r#" +def coin() -> bool: ... +class C: + if coin(): + suppressed = 1 # pyrefly: ignore[possibly-uninitialized-attribute] + reported = 1 # pyrefly: ignore[missing-attribute] # E: Attribute `reported` may be uninitialized +C.missing # pyrefly: ignore[possibly-uninitialized-attribute] # E: Class `C` has no class attribute `missing` +"#, +); + +testcase!( + test_class_body_initialization_stub, + TestEnv::one_with_path( + "foo", + "foo.pyi", + r#" +def coin() -> bool: ... +class C: + if coin(): + value: int = 1 +"#, + ) + .enable_possibly_uninitialized_attribute_error(), + r#" +from typing import assert_type +from foo import C +assert_type(C.value, int) +"#, +); + // 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/util.rs b/pyrefly/lib/test/util.rs index 823b38f5a6..4a3663a35b 100644 --- a/pyrefly/lib/test/util.rs +++ b/pyrefly/lib/test/util.rs @@ -258,6 +258,7 @@ pub struct TestEnv { site_package_path: Vec, implicitly_defined_attribute_error: bool, uninitialized_instance_variable_error: bool, + possibly_uninitialized_attribute_error: bool, explicit_any_error: bool, explicit_any_use_error: bool, implicit_any_error: bool, @@ -332,6 +333,7 @@ impl TestEnv { site_package_path: Vec::new(), implicitly_defined_attribute_error: false, uninitialized_instance_variable_error: false, + possibly_uninitialized_attribute_error: false, explicit_any_error: false, explicit_any_use_error: false, implicit_any_error: false, @@ -489,6 +491,11 @@ impl TestEnv { self } + pub fn enable_possibly_uninitialized_attribute_error(mut self) -> Self { + self.possibly_uninitialized_attribute_error = true; + self + } + pub fn enable_explicit_any_error(mut self) -> Self { self.explicit_any_error = true; self @@ -873,6 +880,9 @@ impl TestEnv { if self.uninitialized_instance_variable_error { errors.set_error_severity(ErrorKind::UninitializedInstanceVariable, Severity::Error); } + if self.possibly_uninitialized_attribute_error { + errors.set_error_severity(ErrorKind::PossiblyUninitializedAttribute, Severity::Error); + } if self.explicit_any_error { errors.set_error_severity(ErrorKind::ExplicitAny, Severity::Error); } diff --git a/scripts/error_presets.json b/scripts/error_presets.json index 16e766e18d..96e04f2ea0 100644 --- a/scripts/error_presets.json +++ b/scripts/error_presets.json @@ -111,6 +111,7 @@ "ordered-dataclass-inheritance": ["legacy", "default", "strict", "all"], "parse-error": ["basic", "legacy", "default", "strict", "all"], "partially-unknown-type": ["all"], + "possibly-uninitialized-attribute": ["strict", "all"], "potential-bad-keyword-argument": ["strict", "all"], "protocol-implicitly-defined-attribute": ["legacy", "default", "strict", "all"], "pytorch-efficiency-lint-cuda-call": ["all"], diff --git a/website/docs/configuration.mdx b/website/docs/configuration.mdx index a644348c99..5653b57013 100644 --- a/website/docs/configuration.mdx +++ b/website/docs/configuration.mdx @@ -847,7 +847,7 @@ The default Pyrefly configuration. Equivalent to having no preset at all. Enables additional error codes on top of the default for stricter checking. - Sets [`strict-callable-subtyping`](#strict-callable-subtyping) `= true` -- Enables (as errors): [`direct-abstract-base-instantiation`](./error-kinds.mdx#direct-abstract-base-instantiation), [`implicit-any`](./error-kinds.mdx#implicit-any) (covers every implicit-`Any` sub-kind), [`method-assign`](./error-kinds.mdx#method-assign), [`missing-override-decorator`](./error-kinds.mdx#missing-override-decorator), [`open-unpacking`](./error-kinds.mdx#open-unpacking), [`potential-bad-keyword-argument`](./error-kinds.mdx#potential-bad-keyword-argument), [`unused-ignore`](./error-kinds.mdx#unused-ignore) +- Enables (as errors): [`direct-abstract-base-instantiation`](./error-kinds.mdx#direct-abstract-base-instantiation), [`implicit-any`](./error-kinds.mdx#implicit-any) (covers every implicit-`Any` sub-kind), [`method-assign`](./error-kinds.mdx#method-assign), [`missing-override-decorator`](./error-kinds.mdx#missing-override-decorator), [`open-unpacking`](./error-kinds.mdx#open-unpacking), [`possibly-uninitialized-attribute`](./error-kinds.mdx#possibly-uninitialized-attribute), [`potential-bad-keyword-argument`](./error-kinds.mdx#potential-bad-keyword-argument), [`unused-ignore`](./error-kinds.mdx#unused-ignore) #### Preset: `all` diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 7ccd2c95cf..e0d3ce5806 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1867,6 +1867,38 @@ def untyped(p): # the parameter is an implicit `Any` v = [untyped(1)] # `v` has type `list[Unknown]`: partially unknown ``` +## possibly-uninitialized-attribute + +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. + +```python +class C: + if enabled(): + value = 1 # 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: + +```toml +preset = "strict" + +[errors] +possibly-uninitialized-attribute = "ignore" +``` + +This setting does not disable [`missing-attribute`](#missing-attribute). + ## potential-bad-keyword-argument Default severity: `ignore`