From 7493584789e46b0501202201f977699e2423f497 Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Mon, 26 Oct 2026 19:35:20 +0530 Subject: [PATCH 1/7] Test reannotated instance attributes across methods --- pyrefly/lib/test/attributes.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index a7733fafa1..d8e1d6a6bf 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -467,6 +467,17 @@ def f(a: A): "#, ); +testcase!( + test_self_attribute_redefinition_across_methods, + r#" +class A: + def __init__(self) -> None: + self.foo: int | None = None + def calc_foo(self) -> None: + self.foo: int = 1 # E: Attribute `foo` already defined + "#, +); + testcase!( test_generic_classvar, r#" From c5d32434ad16d44eaba96c57ef87a503c7360958 Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Mon, 26 Oct 2026 19:35:34 +0530 Subject: [PATCH 2/7] Report duplicate instance attribute annotations Compare per-method annotations in source order before method fields are merged. --- pyrefly/lib/binding/class.rs | 15 ++++++++++++++- pyrefly/lib/binding/scope.rs | 19 ++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 13c3e716c5..3492b60841 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; @@ -462,8 +463,20 @@ 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, mut instance_attribute_annotations) = self.scopes.finish_class_and_get_field_definitions(); + instance_attribute_annotations + .sort_by_key(|(_, annotation)| self.idx_to_key(*annotation).range().start()); + let mut seen_annotations = SmallSet::new(); + for (name, annotation) in instance_attribute_annotations { + if !seen_annotations.insert(name.clone()) { + self.error( + self.idx_to_key(annotation).range(), + ErrorKind::Redefinition, + format!("Attribute `{name}` already defined"), + ); + } + } self.insert_binding_idx( class_indices.synthesized_fields_idx, BindingClassSynthesizedFields { diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index bd48eebe38..b4f241ffca 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -3163,6 +3163,7 @@ impl Scopes { ) -> ( SmallMap, NNModuleRegistrations, + Vec<(Name, Idx)>, ) { let mut field_definitions = SmallMap::new(); let class_body = self.pop(); @@ -3181,6 +3182,18 @@ impl Scopes { // appears in such a method. let nn_module_registrations = mem::take(&mut class_scope.nn_module_registrations); let method_attrs: Vec<_> = class_scope.method_defined_attributes().collect(); + // Preserve annotations before merging method fields, which prioritizes constructors. + let instance_attribute_annotations = method_attrs + .iter() + .filter_map(|(name, method, InstanceAttribute(_, annotation, _, _))| { + match method.instance_or_class { + MethodSelfKind::Instance => { + annotation.map(|annotation| (name.key().clone(), annotation)) + } + MethodSelfKind::Class => None, + } + }) + .collect(); let recognized_instance_attrs: SmallSet = method_attrs .iter() .filter_map(|(name, method, _)| { @@ -3322,7 +3335,11 @@ impl Scopes { } }, ); - (field_definitions, nn_module_registrations) + ( + field_definitions, + nn_module_registrations, + instance_attribute_annotations, + ) } /// Return a pair Some((method_name, class_key)) if we are currently in a method From 915a1d0f6b561af20707468ab7b881d1834a5565 Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Mon, 26 Oct 2026 22:47:42 +0530 Subject: [PATCH 3/7] fix(test): portable coverage pipe-close on macOS Same as #5390: BSD head rejects -n 0; use dd bs=1 count=0. --- test/coverage.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/coverage.md b/test/coverage.md index 1119decf95..c89d934b7b 100644 --- a/test/coverage.md +++ b/test/coverage.md @@ -56,7 +56,7 @@ IDENTICAL ```scrut $ touch $TMPDIR/pyrefly.toml && \ > for i in $(seq 1 1000); do echo "x$i: int = 1"; done > $TMPDIR/large.py && \ -> set -o pipefail && $PYREFLY coverage report $TMPDIR/large.py 2>/dev/null | head -n 0 +> set -o pipefail && $PYREFLY coverage report $TMPDIR/large.py 2>/dev/null | dd bs=1 count=0 2>/dev/null [0] ``` @@ -65,6 +65,6 @@ $ touch $TMPDIR/pyrefly.toml && \ ```scrut $ touch $TMPDIR/pyrefly.toml && \ > for i in $(seq 1 1000); do echo "x$i: int = 1"; done > $TMPDIR/large.py && \ -> set -o pipefail && $PYREFLY report $TMPDIR/large.py 2>/dev/null | head -n 0 +> set -o pipefail && $PYREFLY report $TMPDIR/large.py 2>/dev/null | dd bs=1 count=0 2>/dev/null [0] ``` From 2587542a36418c3b006d5c86bc6663a3636558e7 Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Sun, 1 Nov 2026 20:14:47 +0530 Subject: [PATCH 4/7] style: format multi-line tuple pattern in class.rs --- pyrefly/lib/binding/class.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 151a2f96df..d0513a18f9 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -468,8 +468,12 @@ impl<'a> BindingsBuilder<'a> { body, &NestingContext::class(ShortIdentifier::new(&x.name), parent.dupe()), ); - let (field_definitions, nn_module_registrations, uninitialized_fields, mut instance_attribute_annotations) = - self.scopes.finish_class_and_get_field_definitions(); + let ( + field_definitions, + nn_module_registrations, + uninitialized_fields, + mut instance_attribute_annotations, + ) = self.scopes.finish_class_and_get_field_definitions(); instance_attribute_annotations .sort_by_key(|(_, annotation)| self.idx_to_key(*annotation).range().start()); let mut seen_annotations = SmallSet::new(); From bc826b45bc79028d0ade0883b5196cdcc7de2b1e Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Mon, 2 Nov 2026 07:11:58 +0000 Subject: [PATCH 5/7] Compare repeated self annotations and skip non-receiver targets A repeated self.x annotation is only a redefinition when it differs from the earlier method annotation. Fields declared in the class body are left to the existing declaration checks. Staticmethod parameters and names rebound in a nested scope no longer count as the class receiver. --- pyrefly/lib/binding/class.rs | 35 ++++++++++++++++++++++++++++++--- pyrefly/lib/binding/function.rs | 15 +++++++++++++- pyrefly/lib/binding/scope.rs | 12 ++++++++++- pyrefly/lib/test/attributes.rs | 34 ++++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index e187987bbd..e827ff1bcb 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -27,11 +27,11 @@ use ruff_python_ast::Identifier; use ruff_python_ast::Keyword; use ruff_python_ast::Stmt; use ruff_python_ast::StmtClassDef; +use ruff_python_ast::comparable::ComparableExpr; 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; @@ -478,9 +478,26 @@ impl<'a> BindingsBuilder<'a> { ) = self.scopes.finish_class_and_get_field_definitions(); instance_attribute_annotations .sort_by_key(|(_, annotation)| self.idx_to_key(*annotation).range().start()); - let mut seen_annotations = SmallSet::new(); + // A field declared in the class body is checked against that declaration elsewhere. + // Among method annotations, repeating the same annotation is fine; only a different + // one is reported as a redefinition. + let mut seen_annotations: SmallMap> = SmallMap::new(); for (name, annotation) in instance_attribute_annotations { - if !seen_annotations.insert(name.clone()) { + if matches!( + field_definitions.get(&name), + Some(( + ClassFieldDefinition::DeclaredByAnnotation { .. } + | ClassFieldDefinition::AssignedInBody { .. }, + _ + )) + ) { + continue; + } + let Some(previous) = seen_annotations.get(&name).copied() else { + seen_annotations.insert(name, annotation); + continue; + }; + if !self.same_annotation_expr(previous, annotation) { self.error( self.idx_to_key(annotation).range(), ErrorKind::Redefinition, @@ -1675,3 +1692,15 @@ impl<'a> BindingsBuilder<'a> { } } } + +impl<'a> BindingsBuilder<'a> { + fn same_annotation_expr(&self, a: Idx, b: Idx) -> bool { + match (self.idx_to_binding(a), self.idx_to_binding(b)) { + ( + Some(BindingAnnotation::AnnotateExpr(_, a, _)), + Some(BindingAnnotation::AnnotateExpr(_, b, _)), + ) => ComparableExpr::from(a) == ComparableExpr::from(b), + _ => false, + } + } +} diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index cda4685dbf..60fec1df9f 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -90,6 +90,7 @@ struct Decorators { is_abstract_method: bool, is_override: bool, is_classmethod: bool, + is_staticmethod: bool, decorator_kind: FunctionDecorators, decorators: Box<[Idx]>, } @@ -290,11 +291,12 @@ impl<'a> BindingsBuilder<'a> { undecorated_idx: Idx, class_key: Option>, method_self_kind: MethodSelfKind, + is_staticmethod: bool, ignore_annotations: bool, ) { let mut self_name = None; for x in x.iter_non_variadic_params() { - if class_key.is_some() && self_name.is_none() { + if class_key.is_some() && !is_staticmethod && self_name.is_none() { self_name = Some(x.parameter.name.clone()); } self.bind_function_param( @@ -402,6 +404,7 @@ impl<'a> BindingsBuilder<'a> { class_key: Option>, is_async: bool, method_self_kind: MethodSelfKind, + is_staticmethod: bool, ) -> ( YieldsAndReturns, Option, @@ -415,6 +418,7 @@ impl<'a> BindingsBuilder<'a> { undecorated_idx, class_key, method_self_kind, + is_staticmethod, false, ); self.init_static_scope(&body, false); @@ -459,6 +463,7 @@ impl<'a> BindingsBuilder<'a> { class_key: Option>, is_async: bool, method_self_kind: MethodSelfKind, + is_staticmethod: bool, ignore_annotations: bool, ) -> Option { // Push a scope to create the parameter keys (but do nothing else with it). @@ -469,6 +474,7 @@ impl<'a> BindingsBuilder<'a> { undecorated_idx, class_key, method_self_kind, + is_staticmethod, ignore_annotations, ); self.scopes.pop(); @@ -662,6 +668,7 @@ impl<'a> BindingsBuilder<'a> { let mut has_no_type_check = false; let mut is_abstract_method = false; let mut is_classmethod = false; + let mut is_staticmethod = false; let mut decorator_kind = FunctionDecorators::Undecorated; let mut all_receiver_overload = true; for d in &decorator_list { @@ -691,6 +698,8 @@ impl<'a> BindingsBuilder<'a> { special_export, Some(SpecialExport::ClassMethod | SpecialExport::AbstractClassMethod) ); + is_staticmethod = + is_staticmethod || matches!(special_export, Some(SpecialExport::StaticMethod)); } if is_overload { decorator_kind = if all_receiver_overload { @@ -708,6 +717,7 @@ impl<'a> BindingsBuilder<'a> { is_abstract_method, is_override, is_classmethod, + is_staticmethod, decorator_kind, decorators, } @@ -804,6 +814,7 @@ impl<'a> BindingsBuilder<'a> { class_key, is_async, method_self_kind, + decorators.is_staticmethod, decorators.has_no_type_check, ); (false, self_assignments) @@ -819,6 +830,7 @@ impl<'a> BindingsBuilder<'a> { class_key, is_async, method_self_kind, + decorators.is_staticmethod, ); self.analyze_return_type( func_name, @@ -846,6 +858,7 @@ impl<'a> BindingsBuilder<'a> { class_key, is_async, method_self_kind, + decorators.is_staticmethod, ); if !ignore_unused_parameters { self.record_unused_parameters(unused_parameters); diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 0607c446bd..ff8dda7abe 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -2352,10 +2352,20 @@ impl Scopes { value: ExprOrBinding, annotation: Option>, ) -> bool { + let Expr::Name(receiver) = &*x.value else { + return false; + }; for scope in self.iter_rev_mut() { + if !matches!(scope.kind, ScopeKind::Method(_)) + && scope.stat.0.contains_key(&receiver.id) + { + // A nested scope rebinds the receiver name (e.g. an inner `def f(self): ...`), + // so this is not an assignment to the enclosing method's receiver. + return false; + } 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) + && receiver.id == self_name.id { if let Some(attr) = method_scope.instance_attributes.get_mut(&x.attr.id) { // Accumulate subsequent assignments in the method. diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 7765be93ac..06447a0a9d 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -4651,3 +4651,37 @@ class DeletedThenDefined: def value(self) -> int: ... "#, ); + +testcase!( + test_self_attribute_repeated_compatible_annotation, + r#" +class A: + x: int + def __init__(self) -> None: + self.x: int = 0 + def reset(self) -> None: + self.x: int = 1 +class B: + def __init__(self) -> None: + self.y: int = 0 + def reset(self) -> None: + self.y: int = 1 + "#, +); + +testcase!( + test_self_attribute_redefinition_ignores_other_receivers, + r#" +class Other: + x: str +class A: + def __init__(self) -> None: + self.x: int = 0 + @staticmethod + def build(other: Other) -> None: + other.x: str = "" # E: Cannot annotate non-self attribute `other.x` + def nested(self) -> None: + def inner(self: Other) -> None: + self.x: str = "" # E: Cannot annotate non-self attribute `self.x` + "#, +); From c34809394f70af1dcc279e030de45ade6f62807f Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Tue, 3 Nov 2026 02:02:35 +0530 Subject: [PATCH 6/7] Solve-time redefinition check and consistent receiver scoping Instance attribute redefinitions across methods are now compared at solve time rather than using syntactic AST equality, comparing against the declaration selected by constructor-prioritized field merging. Receiver ownership now distinguishes local/non-receiver parameters from captured outer receivers, treats intervening class scopes as transparent, and forbids Final-write permissions across nested method boundaries. Finally, staticmethods skip field inference in unannotated or @no_type_check bodies. --- pyrefly/lib/binding/class.rs | 58 ++---- pyrefly/lib/binding/function.rs | 2 +- pyrefly/lib/binding/scope.rs | 204 ++++++++++++++-------- pyrefly/lib/test/attributes.rs | 116 +++++++++++- pyrefly/lib/test/untyped_def_behaviors.rs | 26 +++ 5 files changed, 282 insertions(+), 124 deletions(-) diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 9ac90264c3..af3985e16a 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -27,7 +27,6 @@ use ruff_python_ast::Identifier; use ruff_python_ast::Keyword; use ruff_python_ast::Stmt; use ruff_python_ast::StmtClassDef; -use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::name::Name; use ruff_text_size::Ranged; use ruff_text_size::TextRange; @@ -472,40 +471,17 @@ impl<'a> BindingsBuilder<'a> { &NestingContext::class(ShortIdentifier::new(&x.name), parent.dupe()), ); self.finish_classmethod_receivers(); - let ( - field_definitions, - nn_module_registrations, - uninitialized_fields, - mut instance_attribute_annotations, - ) = self.scopes.finish_class_and_get_field_definitions(); - instance_attribute_annotations - .sort_by_key(|(_, annotation)| self.idx_to_key(*annotation).range().start()); - // A field declared in the class body is checked against that declaration elsewhere. - // Among method annotations, repeating the same annotation is fine; only a different - // one is reported as a redefinition. - let mut seen_annotations: SmallMap> = SmallMap::new(); - for (name, annotation) in instance_attribute_annotations { - if matches!( - field_definitions.get(&name), - Some(( - ClassFieldDefinition::DeclaredByAnnotation { .. } - | ClassFieldDefinition::AssignedInBody { .. }, - _ - )) - ) { - continue; - } - let Some(previous) = seen_annotations.get(&name).copied() else { - seen_annotations.insert(name, annotation); - continue; - }; - if !self.same_annotation_expr(previous, annotation) { - self.error( - self.idx_to_key(annotation).range(), - ErrorKind::Redefinition, - format!("Attribute `{name}` already defined"), - ); - } + let (field_definitions, nn_module_registrations, uninitialized_fields, redefinitions) = + self.scopes.finish_class_and_get_field_definitions(); + for (name, new, existing) in redefinitions { + self.insert_binding( + KeyExpect::Redefinition(self.idx_to_key(new).range()), + BindingExpect::Redefinition { + new, + existing, + name, + }, + ); } self.track_classmethod_uses = previous_tracking; self.insert_binding_idx( @@ -1695,15 +1671,3 @@ impl<'a> BindingsBuilder<'a> { } } } - -impl<'a> BindingsBuilder<'a> { - fn same_annotation_expr(&self, a: Idx, b: Idx) -> bool { - match (self.idx_to_binding(a), self.idx_to_binding(b)) { - ( - Some(BindingAnnotation::AnnotateExpr(_, a, _)), - Some(BindingAnnotation::AnnotateExpr(_, b, _)), - ) => ComparableExpr::from(a) == ComparableExpr::from(b), - _ => false, - } - } -} diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index 60fec1df9f..100007a176 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -479,7 +479,7 @@ impl<'a> BindingsBuilder<'a> { ); self.scopes.pop(); // If we are in a class, use a simple visitor to find `self.` assignments. - if class_key.is_some() { + if class_key.is_some() && !is_staticmethod { SelfAttrNames::find(func_name, parameters, body) } else { None diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index ff8dda7abe..e6f5f8df2b 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -2343,8 +2343,85 @@ impl Scopes { /// often complain if an attribute is implicitly defined outside of methods /// (like constructors) that we recognize as always being called. /// + /// Finds the method scope in `self.scopes` that owns `receiver` as its receiver parameter. + /// Returns `Some((method_index, is_nested))` where: + /// - `method_index` is the index in `self.scopes` of the `ScopeKind::Method` + /// - `is_nested` is true if the receiver was reached across a nested function/method boundary. + /// + /// Returns `None` if `receiver` is not attributed to any method's receiver (including when + /// shadowed by a local variable or non-receiver parameter in an inner scope). + fn find_receiver_method_index(&self, receiver: &Name) -> Option<(usize, bool)> { + let mut crossed_nested = false; + for (i, node) in self.scopes.iter().enumerate().rev() { + let scope = &node.scope; + match &scope.kind { + ScopeKind::Class(_) | ScopeKind::Annotation { .. } | ScopeKind::TypeAlias => { + // Intervening class namespaces and annotation/typealias scopes do not shadow + // lexical receiver variables in enclosed methods. + } + ScopeKind::Function(_) | ScopeKind::Comprehension { .. } => { + if let Some(static_info) = scope.stat.0.get(receiver) { + if matches!( + static_info.style, + StaticStyle::MutableCapture(MutableCapture { + kind: MutableCaptureKind::Nonlocal, + .. + }) + ) { + // Genuine `nonlocal receiver` capture: continues to outer scope. + crossed_nested = true; + } else { + // The name is bound locally in this function/comprehension (as a parameter + // or local variable), so it shadows the enclosing receiver. + return None; + } + } else { + crossed_nested = true; + } + } + ScopeKind::Method(method_scope) => { + if let Some(static_info) = scope.stat.0.get(receiver) { + if matches!( + static_info.style, + StaticStyle::MutableCapture(MutableCapture { + kind: MutableCaptureKind::Nonlocal, + .. + }) + ) { + // Genuine `nonlocal receiver` capture: continues to outer scope. + crossed_nested = true; + } else if method_scope + .self_name + .as_ref() + .is_some_and(|name| &name.id == receiver) + { + return Some((i, crossed_nested)); + } else { + // The name is a non-receiver parameter (e.g. a staticmethod parameter named + // self, or a secondary parameter) or a local binding in this method. It + // shadows any outer receiver. + return None; + } + } else if method_scope + .self_name + .as_ref() + .is_some_and(|name| &name.id == receiver) + { + return Some((i, crossed_nested)); + } else { + crossed_nested = true; + } + } + ScopeKind::Module => { + return None; + } + } + } + None + } + /// Returns `true` if the attribute was a self attribute. - /// Record a self attribute assignment (e.g., `self.x = value`) inside the current method scope. + /// Record a self attribute assignment (e.g., `self.x = value`) inside the owning method scope. /// We accumulate all assignments to the same attribute within the method so they can later be unioned. pub fn record_self_attr_assign( &mut self, @@ -2355,41 +2432,32 @@ impl Scopes { let Expr::Name(receiver) = &*x.value else { return false; }; - for scope in self.iter_rev_mut() { - if !matches!(scope.kind, ScopeKind::Method(_)) - && scope.stat.0.contains_key(&receiver.id) - { - // A nested scope rebinds the receiver name (e.g. an inner `def f(self): ...`), - // so this is not an assignment to the enclosing method's receiver. - return false; - } - if let ScopeKind::Method(method_scope) = &mut scope.kind - && let Some(self_name) = &method_scope.self_name - && receiver.id == self_name.id - { - if let Some(attr) = method_scope.instance_attributes.get_mut(&x.attr.id) { - // Accumulate subsequent assignments in the method. - attr.0.push(value); - // Keep the first type annotation encountered in the method. - if attr.1.is_none() { - attr.1 = annotation; - } - } else { - // First time seeing this attribute in this method: record it. - method_scope.instance_attributes.insert( - x.attr.id.clone(), - InstanceAttribute( - vec![value], - annotation, - x.attr.range(), // Keep the range of the first assignment as the definition location. - method_scope.receiver_kind, - ), - ); - } - return true; + let Some((method_idx, _)) = self.find_receiver_method_index(&receiver.id) else { + return false; + }; + let ScopeKind::Method(method_scope) = &mut self.scopes[method_idx].scope.kind else { + return false; + }; + if let Some(attr) = method_scope.instance_attributes.get_mut(&x.attr.id) { + // Accumulate subsequent assignments in the method. + attr.0.push(value); + // Keep the first type annotation encountered in the method. + if attr.1.is_none() { + attr.1 = annotation; } + } else { + // First time seeing this attribute in this method: record it. + method_scope.instance_attributes.insert( + x.attr.id.clone(), + InstanceAttribute( + vec![value], + annotation, + x.attr.range(), // Keep the range of the first assignment as the definition location. + method_scope.receiver_kind, + ), + ); } - false + true } /// Record a local bound to the result of a `__new__` call inside `__new__`, such as @@ -2462,33 +2530,27 @@ impl Scopes { } pub fn method_that_sets_attr(&self, x: &ExprAttribute) -> Option { - let mut method_name: Option = None; - let mut receiver_kind = MethodSelfKind::Instance; - for scope in self.iter_rev() { - match &scope.kind { - ScopeKind::Method(method_scope) if method_name.is_none() => { - if let Some(self_name) = &method_scope.self_name - && matches!(&*x.value, Expr::Name(name) if name.id == self_name.id) - { - method_name = Some(method_scope.name.id.clone()); - receiver_kind = method_scope.receiver_kind; - } else { - return None; - } - } - ScopeKind::Class(class_scope) => { - if let Some(method_name) = &method_name { - return Some(MethodThatSetsAttr { - method_name: method_name.clone(), - recognized_attribute_defining_method: is_attribute_defining_method( - method_name, - &class_scope.name.id, - ), - instance_or_class: receiver_kind, - }); - } - } - _ => {} + let Expr::Name(receiver) = &*x.value else { + return None; + }; + let (method_idx, is_nested) = self.find_receiver_method_index(&receiver.id)?; + if is_nested { + // Permissions to write Final fields must not cross a nested-method boundary. + return None; + } + let ScopeKind::Method(method_scope) = &self.scopes[method_idx].scope.kind else { + return None; + }; + for node in self.scopes[..method_idx].iter().rev() { + if let ScopeKind::Class(class_scope) = &node.scope.kind { + return Some(MethodThatSetsAttr { + method_name: method_scope.name.id.clone(), + recognized_attribute_defining_method: is_attribute_defining_method( + &method_scope.name.id, + &class_scope.name.id, + ), + instance_or_class: method_scope.receiver_kind, + }); } } None @@ -3386,7 +3448,7 @@ impl Scopes { SmallMap, NNModuleRegistrations, SmallMap>>>, - Vec<(Name, Idx)>, + Vec<(Name, Idx, Idx)>, ) { let mut field_definitions = SmallMap::new(); let mut uninitialized_fields = SmallMap::new(); @@ -3408,18 +3470,6 @@ impl Scopes { let attributes_set_on_new_instance = mem::take(&mut class_scope.attributes_set_on_new_instance); let method_attrs: Vec<_> = class_scope.method_defined_attributes().collect(); - // Preserve annotations before merging method fields, which prioritizes constructors. - let instance_attribute_annotations = method_attrs - .iter() - .filter_map(|(name, method, InstanceAttribute(_, annotation, _, _))| { - match method.instance_or_class { - MethodSelfKind::Instance => { - annotation.map(|annotation| (name.key().clone(), annotation)) - } - MethodSelfKind::Class => None, - } - }) - .collect(); let recognized_instance_attrs: SmallSet = method_attrs .iter() .filter_map(|(name, method, _)| { @@ -3527,6 +3577,7 @@ impl Scopes { // Merge assignments from different methods. // `method_attrs` yields attributes from recognized constructor methods first (e.g. __init__), // followed by other helper methods. + let mut redefinitions = Vec::new(); method_attrs.into_iter().for_each( |(name, method, InstanceAttribute(values, annotation, range, receiver_kind))| { if let Some(( @@ -3543,6 +3594,9 @@ impl Scopes { if matches!(receiver_kind, MethodSelfKind::Instance) { *existing_instance_write = true; } + if let (Some(new_ann), Some(existing_ann)) = (annotation, *existing_annot) { + redefinitions.push((name.key().clone(), new_ann, existing_ann)); + } if existing_method.recognized_attribute_defining_method && !method.recognized_attribute_defining_method { @@ -3585,7 +3639,7 @@ impl Scopes { field_definitions, nn_module_registrations, uninitialized_fields, - instance_attribute_annotations, + redefinitions, ) } diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 4dce9c51bf..5b0fb4f5c1 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -1269,7 +1269,7 @@ class A: def __init__(self) -> None: self.foo: int | None = None def calc_foo(self) -> None: - self.foo: int = 1 # E: Attribute `foo` already defined + self.foo: int = 1 # E: `foo` cannot be annotated with `int`, it is already defined with type `int | None` "#, ); @@ -5157,3 +5157,117 @@ class A: "#, ); +testcase!( + test_self_attribute_redefinition_constructor_ordering, + r#" +class A: + def reset(self) -> None: + self.x: int = 1 # E: `x` cannot be annotated with `int`, it is already defined with type `int | None` + def __init__(self) -> None: + self.x: int | None = None +"#, +); + +testcase!( + test_self_attribute_compatible_unions_and_aliases, + r#" +type AliasInt = int +class A: + def __init__(self) -> None: + self.x: int | str = 0 + self.y: AliasInt = 0 + def reset(self) -> None: + self.x: str | int = "" + self.y: int = 1 +"#, +); + +testcase!( + test_self_attribute_scoped_type_alias_redefinition, + r#" +class T1: ... +class T2: ... +class A: + def __init__(self) -> None: + T = T1 + self.x: T + def reset(self) -> None: + T = T2 + self.x: T # E: `x` cannot be annotated with `T2`, it is already defined with type `T1` +"#, +); + +testcase!( + test_self_attribute_class_body_reannotation_and_literal_narrowing, + r#" +from typing import Literal +class A: + x: int + y: Literal[0] | Literal[1] + def __init__(self) -> None: + self.x: int = 0 + self.y: Literal[1] = 1 + def reset(self) -> None: + self.x: int = 1 + self.y: Literal[0] = 0 +"#, +); + +testcase!( + test_nested_method_receiver_ownership, + r#" +class Outer: + def __init__(self) -> None: + self.x: int = 0 + class Inner: + @staticmethod + def static_helper(self: str) -> None: + self.x = "" # E: Object of class `str` has no attribute `x` + def second_param(other, self: str) -> None: + self.x = "" # E: Object of class `str` has no attribute `x` + def captured(other) -> None: + self.x = 1 + def local_capture() -> None: + nonlocal self + self.x = 2 +"#, +); + +testcase!( + test_final_attribute_nested_permissions, + r#" +from typing import Final +class A: + x: Final[int] + def __init__(self) -> None: + self.x = 0 + def helper() -> None: + self.x = 1 # E: Cannot set field `x` + def helper_nonlocal() -> None: + nonlocal self + self.x = 2 # E: Cannot set field `x` + class Inner: + @staticmethod + def static_fn(self: A) -> None: + self.x = 3 # E: Cannot set field `x` +"#, +); + +testcase!( + test_no_type_check_staticmethod_skipped_body, + r#" +from typing import no_type_check +class A: + @no_type_check + @staticmethod + def static_func(self) -> None: + self.phantom = 1 + @no_type_check + def normal_func(self) -> None: + self.real = 2 + +a = A() +_ = a.phantom # E: Object of class `A` has no attribute `phantom` +_ = a.real +"#, +); diff --git a/pyrefly/lib/test/untyped_def_behaviors.rs b/pyrefly/lib/test/untyped_def_behaviors.rs index f001658d37..e9ad308ffc 100644 --- a/pyrefly/lib/test/untyped_def_behaviors.rs +++ b/pyrefly/lib/test/untyped_def_behaviors.rs @@ -647,3 +647,29 @@ def unannotated(): "Expected errors in IDE mode (Require::Everything)" ); } + +#[test] +fn test_unannotated_staticmethod_batch_skipped_body() { + use crate::state::errors::Errors; + use crate::state::require::Require; + use crate::test::util::TestEnv; + + let code = r#" +class C: + @staticmethod + def f(self): + self.phantom = 1 + +c = C() +_ = c.phantom +"#; + let mut batch_env = TestEnv::new_skip_check_no_infer().with_run_require(Require::Errors); + batch_env.add("main", code); + let (batch_state, batch_handle) = batch_env.to_state(); + let batch_errors: Errors = batch_state + .transaction() + .get_errors([&batch_handle("main")]); + let errs = batch_errors.collect_errors().ordinary; + assert_eq!(errs.len(), 1); + assert!(errs[0].msg().contains("has no attribute `phantom`")); +} From ff5acb107f05fc98770d9b15d333967a827bfdf5 Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Tue, 3 Nov 2026 02:28:19 +0530 Subject: [PATCH 7/7] Keep Final-write permission in closures inside constructors Only nested methods drop the enclosing receiver's permissions; plain closures defined in __init__ keep them, matching the existing test_assign_final_attr_in_nested_function_in_constructor behavior. --- pyrefly/lib/binding/scope.rs | 10 ++++++---- pyrefly/lib/test/attributes.rs | 8 ++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index e6f5f8df2b..42ecc84f1f 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -2346,7 +2346,8 @@ impl Scopes { /// Finds the method scope in `self.scopes` that owns `receiver` as its receiver parameter. /// Returns `Some((method_index, is_nested))` where: /// - `method_index` is the index in `self.scopes` of the `ScopeKind::Method` - /// - `is_nested` is true if the receiver was reached across a nested function/method boundary. + /// - `is_nested` is true if the receiver was reached across a nested method boundary + /// (plain closures do not count). /// /// Returns `None` if `receiver` is not attributed to any method's receiver (including when /// shadowed by a local variable or non-receiver parameter in an inner scope). @@ -2369,15 +2370,16 @@ impl Scopes { }) ) { // Genuine `nonlocal receiver` capture: continues to outer scope. - crossed_nested = true; + // Plain closures keep the enclosing method's permissions. } else { // The name is bound locally in this function/comprehension (as a parameter // or local variable), so it shadows the enclosing receiver. return None; } - } else { - crossed_nested = true; } + // A plain closure is not a method boundary: it keeps the enclosing + // method's permissions (e.g. assigning `Final` fields from a helper + // defined inside `__init__`, which mypy also allows). } ScopeKind::Method(method_scope) => { if let Some(static_info) = scope.stat.0.get(receiver) { diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 5b0fb4f5c1..be3c828c4d 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -5242,11 +5242,15 @@ class A: def __init__(self) -> None: self.x = 0 def helper() -> None: - self.x = 1 # E: Cannot set field `x` + self.x = 1 def helper_nonlocal() -> None: nonlocal self - self.x = 2 # E: Cannot set field `x` + self.x = 2 class Inner: + def method(inner_self) -> None: + def f() -> None: + self.x = 4 # E: Cannot set field `x` + f() @staticmethod def static_fn(self: A) -> None: self.x = 3 # E: Cannot set field `x`