[`flake8-self`] Ignore attribute accesses on instance-like variables (`SLF001`) (#16149)

crates/ruff_linter/resources/test/fixtures/flake8_self/SLF001_1.py

@@ -0,0 +1,45 @@
+from __future__ import annotations
+
+from typing import Annotated
+
+# https://github.com/astral-sh/ruff/issues/9022
+
+class Lorem[T]:
+    def f(self):
+        lorem_1 = Lorem()
+        lorem_1._value = 1  # fine
+
+        lorem_2 = Lorem[bytes]()
+        lorem_2._value = 1  # fine
+
+
+class Ipsum:
+    def __new__(cls):
+        instance = super().__new__(cls)
+        instance._value = 1  # fine
+
+
+class Dolor[T]:
+    def f(
+        self,
+        a: Dolor,
+        b: Dolor[int],
+        c: Annotated[Dolor, ...],
+        d: Annotated[Dolor[str], ...]
+    ):
+        a._value = 1  # fine
+        b._value = 1  # fine
+        c._value = 1  # fine
+        d._value = 1  # fine
+
+    @classmethod
+    def m(cls):
+        instance = cls()
+        instance._value = 1  # fine
+
+
+class M(type):
+    @classmethod
+    def f(mcs):
+        cls = mcs()
+        cls._value = 1

crates/ruff_linter/src/rules/flake8_self/mod.rs

@@ -16,6 +16,7 @@ mod tests {
     use test_case::test_case;
 
     #[test_case(Rule::PrivateMemberAccess, Path::new("SLF001.py"))]
+    #[test_case(Rule::PrivateMemberAccess, Path::new("SLF001_1.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/flake8_self/rules/private_member_access.rs

@@ -1,11 +1,15 @@
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::helpers::{is_dunder, is_sunder};
 use ruff_python_ast::name::UnqualifiedName;
 use ruff_python_ast::{self as ast, Expr};
-use ruff_python_semantic::{BindingKind, ScopeKind};
+use ruff_python_semantic::analyze::typing;
+use ruff_python_semantic::analyze::typing::TypeChecker;
+use ruff_python_semantic::{BindingKind, ScopeKind, SemanticModel};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
+use crate::rules::pylint::helpers::is_dunder_operator_method;
 
 /// ## What it does
 /// Checks for accesses on "private" class members.
@@ -69,27 +73,14 @@ pub(crate) fn private_member_access(checker: &Checker, expr: &Expr) {
         return;
     };
 
-    if checker.semantic().in_annotation() {
+    let semantic = checker.semantic();
+    let current_scope = semantic.current_scope();
+
+    if semantic.in_annotation() {
         return;
     }
 
-    // Ignore non-private accesses.
+    if !attr.starts_with('_') || is_dunder(attr) || is_sunder(attr) {
-    if !attr.starts_with('_') {
-        return;
-    }
-
-    // Ignore dunder accesses.
-    let is_dunder = attr.starts_with("__") && attr.ends_with("__");
-    if is_dunder {
-        return;
-    }
-
-    // Ignore sunder accesses.
-    let is_sunder = attr.starts_with('_')
-        && attr.ends_with('_')
-        && !attr.starts_with("__")
-        && !attr.ends_with("__");
-    if is_sunder {
         return;
     }
 
@@ -103,65 +94,14 @@ pub(crate) fn private_member_access(checker: &Checker, expr: &Expr) {
     }
 
     // Ignore accesses on instances within special methods (e.g., `__eq__`).
-    if let ScopeKind::Function(ast::StmtFunctionDef { name, .. }) =
+    if let ScopeKind::Function(ast::StmtFunctionDef { name, .. }) = current_scope.kind {
-        checker.semantic().current_scope().kind
+        if is_dunder_operator_method(name) {
-    {
-        if matches!(
-            name.as_str(),
-            "__lt__"
-                | "__le__"
-                | "__eq__"
-                | "__ne__"
-                | "__gt__"
-                | "__ge__"
-                | "__add__"
-                | "__sub__"
-                | "__mul__"
-                | "__matmul__"
-                | "__truediv__"
-                | "__floordiv__"
-                | "__mod__"
-                | "__divmod__"
-                | "__pow__"
-                | "__lshift__"
-                | "__rshift__"
-                | "__and__"
-                | "__xor__"
-                | "__or__"
-                | "__radd__"
-                | "__rsub__"
-                | "__rmul__"
-                | "__rmatmul__"
-                | "__rtruediv__"
-                | "__rfloordiv__"
-                | "__rmod__"
-                | "__rdivmod__"
-                | "__rpow__"
-                | "__rlshift__"
-                | "__rrshift__"
-                | "__rand__"
-                | "__rxor__"
-                | "__ror__"
-                | "__iadd__"
-                | "__isub__"
-                | "__imul__"
-                | "__imatmul__"
-                | "__itruediv__"
-                | "__ifloordiv__"
-                | "__imod__"
-                | "__ipow__"
-                | "__ilshift__"
-                | "__irshift__"
-                | "__iand__"
-                | "__ixor__"
-                | "__ior__"
-        ) {
             return;
         }
     }
 
     // Allow some documented private methods, like `os._exit()`.
-    if let Some(qualified_name) = checker.semantic().resolve_qualified_name(expr) {
+    if let Some(qualified_name) = semantic.resolve_qualified_name(expr) {
         if matches!(qualified_name.segments(), ["os", "_exit"]) {
             return;
         }
@@ -185,25 +125,23 @@ pub(crate) fn private_member_access(checker: &Checker, expr: &Expr) {
 
     if let Expr::Name(name) = value.as_ref() {
         // Ignore accesses on class members from _within_ the class.
-        if checker
+        if semantic
-            .semantic()
             .resolve_name(name)
             .and_then(|id| {
-                if let BindingKind::ClassDefinition(scope) = checker.semantic().binding(id).kind {
+                if let BindingKind::ClassDefinition(scope) = semantic.binding(id).kind {
                     Some(scope)
                 } else {
                     None
                 }
             })
-            .is_some_and(|scope| {
+            .is_some_and(|scope| semantic.current_scope_ids().any(|parent| scope == parent))
-                checker
-                    .semantic()
-                    .current_scope_ids()
-                    .any(|parent| scope == parent)
-            })
         {
             return;
         }
+
+        if is_same_class_instance(name, semantic) {
+            return;
+        }
     }
 
     checker.report_diagnostic(Diagnostic::new(
@@ -213,3 +151,110 @@ pub(crate) fn private_member_access(checker: &Checker, expr: &Expr) {
         expr.range(),
     ));
 }
+
+/// Check for the following cases:
+///
+/// * Parameter annotation:
+///
+///     ```python
+///     class C[T]:
+///         def f(self, other: C): ...
+///         def f(self, other: C[...]): ...
+///         def f(self, other: Annotated[C, ...]): ...
+///     ```
+///
+/// * `super().__new__`/`cls` call:
+///
+///     ```python
+///     class C:
+///         def __new__(cls): ...
+///             instance = super().__new__(cls)
+///         @classmethod
+///         def m(cls):
+///             instance = cls()
+///     ```
+///
+/// This function is intentionally naive and does not handle more complex cases.
+/// It is expected to be expanded overtime, possibly when type-aware APIs are available.
+fn is_same_class_instance(name: &ast::ExprName, semantic: &SemanticModel) -> bool {
+    let Some(binding_id) = semantic.resolve_name(name) else {
+        return false;
+    };
+
+    let binding = semantic.binding(binding_id);
+    typing::check_type::<SameClassInstanceChecker>(binding, semantic)
+}
+
+struct SameClassInstanceChecker;
+
+impl SameClassInstanceChecker {
+    /// Whether `name` resolves to a class which the semantic model is traversing.
+    fn is_current_class_name(name: &ast::ExprName, semantic: &SemanticModel) -> bool {
+        semantic.current_scopes().any(|scope| {
+            let ScopeKind::Class(class) = scope.kind else {
+                return false;
+            };
+
+            class.name.id == name.id
+        })
+    }
+}
+
+impl TypeChecker for SameClassInstanceChecker {
+    /// `C`, `C[T]`, `Annotated[C, ...]`, `Annotated[C[T], ...]`
+    fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool {
+        let Some(class_name) = find_class_name(annotation, semantic) else {
+            return false;
+        };
+
+        Self::is_current_class_name(class_name, semantic)
+    }
+
+    /// `cls()`, `C()`, `C[T]()`, `super().__new__()`
+    fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool {
+        let Expr::Call(call) = initializer else {
+            return false;
+        };
+
+        match &*call.func {
+            Expr::Subscript(_) => Self::match_annotation(&call.func, semantic),
+
+            Expr::Name(name) => {
+                matches!(&*name.id, "cls" | "mcs") || Self::is_current_class_name(name, semantic)
+            }
+
+            Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => {
+                let Expr::Call(ast::ExprCall { func, .. }) = &**value else {
+                    return false;
+                };
+
+                let Expr::Name(ast::ExprName { id: func, .. }) = &**func else {
+                    return false;
+                };
+
+                func == "super" && attr == "__new__"
+            }
+
+            _ => false,
+        }
+    }
+}
+
+/// Convert `Annotated[C[T], ...]` to `C` (and similar) to `C` recursively.
+fn find_class_name<'a>(expr: &'a Expr, semantic: &'a SemanticModel) -> Option<&'a ast::ExprName> {
+    match expr {
+        Expr::Name(name) => Some(name),
+        Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
+            if semantic.match_typing_expr(value, "Annotated") {
+                let [expr, ..] = &slice.as_tuple_expr()?.elts[..] else {
+                    return None;
+                };
+
+                return find_class_name(expr, semantic);
+            }
+
+            find_class_name(value, semantic)
+        }
+        _ => None,
+    }
+}

crates/ruff_linter/src/rules/flake8_self/snapshots/ruff_linter__rules__flake8_self__tests__private-member-access_SLF001_1.py.snap

@@ -0,0 +1,4 @@
+---
+source: crates/ruff_linter/src/rules/flake8_self/mod.rs
+---
+

crates/ruff_linter/src/rules/pylint/helpers.rs

@@ -166,16 +166,68 @@ impl Visitor<'_> for SequenceIndexVisitor<'_> {
     }
 }
 
-/// Returns `true` if a method is a known dunder method.
+pub(crate) fn is_dunder_operator_method(method: &str) -> bool {
-pub(super) fn is_known_dunder_method(method: &str) -> bool {
     matches!(
         method,
-        "__abs__"
+        "__lt__"
+            | "__le__"
+            | "__eq__"
+            | "__ne__"
+            | "__gt__"
+            | "__ge__"
             | "__add__"
+            | "__sub__"
+            | "__mul__"
+            | "__matmul__"
+            | "__truediv__"
+            | "__floordiv__"
+            | "__mod__"
+            | "__divmod__"
+            | "__pow__"
+            | "__lshift__"
+            | "__rshift__"
+            | "__and__"
+            | "__xor__"
+            | "__or__"
+            | "__radd__"
+            | "__rsub__"
+            | "__rmul__"
+            | "__rmatmul__"
+            | "__rtruediv__"
+            | "__rfloordiv__"
+            | "__rmod__"
+            | "__rdivmod__"
+            | "__rpow__"
+            | "__rlshift__"
+            | "__rrshift__"
+            | "__rand__"
+            | "__rxor__"
+            | "__ror__"
+            | "__iadd__"
+            | "__isub__"
+            | "__imul__"
+            | "__imatmul__"
+            | "__itruediv__"
+            | "__ifloordiv__"
+            | "__imod__"
+            | "__ipow__"
+            | "__ilshift__"
+            | "__irshift__"
+            | "__iand__"
+            | "__ixor__"
+            | "__ior__"
+    )
+}
+
+/// Returns `true` if a method is a known dunder method.
+pub(super) fn is_known_dunder_method(method: &str) -> bool {
+    is_dunder_operator_method(method)
+        || matches!(
+            method,
+            "__abs__"
             | "__aenter__"
             | "__aexit__"
             | "__aiter__"
-            | "__and__"
             | "__anext__"
             | "__attrs_init__"
             | "__attrs_post_init__"
@@ -198,17 +250,13 @@ pub(super) fn is_known_dunder_method(method: &str) -> bool {
             | "__delitem__"
             | "__dict__"
             | "__dir__"
-            | "__divmod__"
             | "__doc__"
             | "__enter__"
-            | "__eq__"
             | "__exit__"
             | "__float__"
             | "__floor__"
-            | "__floordiv__"
             | "__format__"
             | "__fspath__"
-            | "__ge__"
             | "__get__"
             | "__getattr__"
             | "__getattribute__"
@@ -216,71 +264,33 @@ pub(super) fn is_known_dunder_method(method: &str) -> bool {
             | "__getnewargs__"
             | "__getnewargs_ex__"
             | "__getstate__"
-            | "__gt__"
             | "__hash__"
             | "__html__"
-            | "__iadd__"
-            | "__iand__"
-            | "__ifloordiv__"
-            | "__ilshift__"
-            | "__imatmul__"
-            | "__imod__"
-            | "__imul__"
             | "__index__"
             | "__init__"
             | "__init_subclass__"
             | "__instancecheck__"
             | "__int__"
             | "__invert__"
-            | "__ior__"
-            | "__ipow__"
-            | "__irshift__"
-            | "__isub__"
             | "__iter__"
-            | "__itruediv__"
-            | "__ixor__"
-            | "__le__"
             | "__len__"
             | "__length_hint__"
-            | "__lshift__"
-            | "__lt__"
-            | "__matmul__"
             | "__missing__"
-            | "__mod__"
             | "__module__"
             | "__mro_entries__"
-            | "__mul__"
-            | "__ne__"
             | "__neg__"
             | "__new__"
             | "__next__"
-            | "__or__"
             | "__pos__"
             | "__post_init__"
-            | "__pow__"
             | "__prepare__"
-            | "__radd__"
-            | "__rand__"
-            | "__rdivmod__"
             | "__reduce__"
             | "__reduce_ex__"
             | "__release_buffer__"
             | "__replace__"
             | "__repr__"
             | "__reversed__"
-            | "__rfloordiv__"
-            | "__rlshift__"
-            | "__rmatmul__"
-            | "__rmod__"
-            | "__rmul__"
-            | "__ror__"
             | "__round__"
-            | "__rpow__"
-            | "__rrshift__"
-            | "__rshift__"
-            | "__rsub__"
-            | "__rtruediv__"
-            | "__rxor__"
             | "__set__"
             | "__set_name__"
             | "__setattr__"
@@ -288,14 +298,11 @@ pub(super) fn is_known_dunder_method(method: &str) -> bool {
             | "__setstate__"
             | "__sizeof__"
             | "__str__"
-            | "__sub__"
             | "__subclasscheck__"
             | "__subclasses__"
             | "__subclasshook__"
-            | "__truediv__"
             | "__trunc__"
             | "__weakref__"
-            | "__xor__"
             // Overridable sunder names from the `Enum` class.
             // See: https://docs.python.org/3/library/enum.html#supported-sunder-names
             | "_add_alias_"

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -1,5 +1,5 @@
 //! Rules from [Pylint](https://pypi.org/project/pylint/).
-mod helpers;
+pub(crate) mod helpers;
 pub(crate) mod rules;
 pub mod settings;
 

crates/ruff_python_ast/src/helpers.rs

@@ -548,6 +548,13 @@ pub fn is_dunder(id: &str) -> bool {
     id.starts_with("__") && id.ends_with("__")
 }
 
+/// Whether a name starts and ends with a single underscore.
+///
+/// `_a__` is considered neither a dunder nor a sunder name.
+pub fn is_sunder(id: &str) -> bool {
+    id.starts_with('_') && id.ends_with('_') && !id.starts_with("__") && !id.ends_with("__")
+}
+
 /// Return `true` if the [`Stmt`] is an assignment to a dunder (like `__all__`).
 pub fn is_assignment_to_a_dunder(stmt: &Stmt) -> bool {
     // Check whether it's an assignment to a dunder, with or without a type

crates/ruff_python_semantic/src/analyze/typing.rs

@@ -572,7 +572,7 @@ pub trait TypeChecker {
 /// NOTE: this function doesn't perform more serious type inference, so it won't be able
 ///       to understand if the value gets initialized from a call to a function always returning
 ///       lists. This also implies no interfile analysis.
-fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bool {
+pub fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bool {
     match binding.kind {
         BindingKind::Assignment => match binding.statement(semantic) {
             // Given: