diff --git a/pyrefly/lib/binding/class.rs b/pyrefly/lib/binding/class.rs index 781b62315c..af3985e16a 100644 --- a/pyrefly/lib/binding/class.rs +++ b/pyrefly/lib/binding/class.rs @@ -471,8 +471,18 @@ 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, 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( class_indices.synthesized_fields_idx, diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index cda4685dbf..100007a176 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,11 +474,12 @@ impl<'a> BindingsBuilder<'a> { undecorated_idx, class_key, method_self_kind, + is_staticmethod, ignore_annotations, ); 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 @@ -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 17eeb50ca4..42ecc84f1f 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -2343,8 +2343,87 @@ 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 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). + 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. + // 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; + } + } + // 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) { + 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, @@ -2352,34 +2431,35 @@ impl Scopes { value: ExprOrBinding, annotation: Option>, ) -> bool { - for scope in self.iter_rev_mut() { - if let ScopeKind::Method(method_scope) = &mut scope.kind - && let Some(self_name) = &method_scope.self_name - && matches!(&*x.value, Expr::Name(name) if name.id == self_name.id) - { - if let Some(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 Expr::Name(receiver) = &*x.value else { + return false; + }; + 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 @@ -2452,33 +2532,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 @@ -3376,6 +3450,7 @@ impl Scopes { SmallMap, NNModuleRegistrations, SmallMap>>>, + Vec<(Name, Idx, Idx)>, ) { let mut field_definitions = SmallMap::new(); let mut uninitialized_fields = SmallMap::new(); @@ -3504,6 +3579,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(( @@ -3520,6 +3596,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 { @@ -3562,6 +3641,7 @@ impl Scopes { field_definitions, nn_module_registrations, uninitialized_fields, + redefinitions, ) } diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index d4f9d5e570..be3c828c4d 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -1262,6 +1262,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: `foo` cannot be annotated with `int`, it is already defined with type `int | None` + "#, +); + testcase!( test_generic_classvar, r#" @@ -5111,3 +5122,156 @@ C.cached = 1 # E: Cannot set instance variable `cached` from the class assert_type(C.cached, Literal[1]) "#, ); + +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` + "#, +); + +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 + def helper_nonlocal() -> None: + nonlocal self + 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` +"#, +); + +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`")); +}