Report redefined instance attributes across methods

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,

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<KeyDecorator>]>,
 }
@@ -290,11 +291,12 @@ impl<'a> BindingsBuilder<'a> {
         undecorated_idx: Idx<KeyUndecoratedFunction>,
         class_key: Option<Idx<KeyClass>>,
         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<Idx<KeyClass>>,
         is_async: bool,
         method_self_kind: MethodSelfKind,
+        is_staticmethod: bool,
     ) -> (
         YieldsAndReturns,
         Option<SelfAssignments>,
@@ -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<Idx<KeyClass>>,
         is_async: bool,
         method_self_kind: MethodSelfKind,
+        is_staticmethod: bool,
         ignore_annotations: bool,
     ) -> Option<SelfAssignments> {
         // 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.<attr>` 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);

pyrefly/lib/binding/scope.rs

@@ -2343,43 +2343,123 @@ 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,
         x: &ExprAttribute,
         value: ExprOrBinding,
         annotation: Option<Idx<KeyAnnotation>>,
     ) -> 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<MethodThatSetsAttr> {
-        let mut method_name: Option<Name> = 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<Name, (ClassFieldDefinition, TextRange)>,
         NNModuleRegistrations,
         SmallMap<Name, Option<Vec<Idx<Key>>>>,
+        Vec<(Name, Idx<KeyAnnotation>, Idx<KeyAnnotation>)>,
     ) {
         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,
         )
     }