[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() {
-        return;
-    }
-
-    // Ignore non-private accesses.
-    if !attr.starts_with('_') {
-        return;
-    }
+    let semantic = checker.semantic();
+    let current_scope = semantic.current_scope();
 
-    // Ignore dunder accesses.
-    let is_dunder = attr.starts_with("__") && attr.ends_with("__");
-    if is_dunder {
+    if semantic.in_annotation() {
         return;
     }
 
-    // Ignore sunder accesses.
-    let is_sunder = attr.starts_with('_')
-        && attr.ends_with('_')
-        && !attr.starts_with("__")
-        && !attr.ends_with("__");
-    if is_sunder {
+    if !attr.starts_with('_') || is_dunder(attr) || is_sunder(attr) {
         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, .. }) =
-        checker.semantic().current_scope().kind
-    {
-        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__"
-        ) {
+    if let ScopeKind::Function(ast::StmtFunctionDef { name, .. }) = current_scope.kind {
+        if is_dunder_operator_method(name) {
             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
-            .semantic()
+        if 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| {
-                checker
-                    .semantic()
-                    .current_scope_ids()
-                    .any(|parent| scope == parent)
-            })
+            .is_some_and(|scope| 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
+---
+