From a73bebcf15e078c90cd590f0a8eb53313bbd5ea9 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Fri, 30 Aug 2024 12:39:05 -0400
Subject: [PATCH] Avoid `no-self-use` for `attrs`-style validators (#13166)

## Summary

Closes https://github.com/astral-sh/ruff/issues/12568.
---
 .../test/fixtures/pylint/no_self_use.py       | 20 ++++++++++++++++
 .../src/rules/pylint/rules/no_self_use.rs     |  1 +
 ...pylint__tests__PLR6301_no_self_use.py.snap |  9 +++++++-
 .../src/analyze/visibility.rs                 | 23 ++++++++++++++++++-
 4 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/no_self_use.py b/crates/ruff_linter/resources/test/fixtures/pylint/no_self_use.py
index db74e179c7..bbb966aba4 100644
--- a/crates/ruff_linter/resources/test/fixtures/pylint/no_self_use.py
+++ b/crates/ruff_linter/resources/test/fixtures/pylint/no_self_use.py
@@ -83,3 +83,23 @@ class B(A):
     def baz(self):
         if super().foo():
             ...
+
+
+# See: https://github.com/astral-sh/ruff/issues/12568
+from attrs import define, field
+
+
+@define
+class Foo:
+    x: int = field()
+    y: int
+
+    @x.validator
+    def validate_x(self, attribute, value):
+        if value <= 0:
+            raise ValueError("x must be a positive integer")
+
+    @y.validator
+    def validate_y(self, attribute, value):
+        if value <= 0:
+            raise ValueError("y must be a positive integer")
diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
index 19420e7b4f..62c11b83c0 100644
--- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
+++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
@@ -87,6 +87,7 @@ pub(crate) fn no_self_use(
         || visibility::is_override(decorator_list, semantic)
         || visibility::is_overload(decorator_list, semantic)
         || visibility::is_property(decorator_list, extra_property_decorators, semantic)
+        || visibility::is_validator(decorator_list, semantic)
     {
         return;
     }
diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap
index eef3c04e36..fe3f47b21f 100644
--- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap
+++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap
@@ -27,4 +27,11 @@ no_self_use.py:13:9: PLR6301 Method `greeting_2` could be a function, class meth
 14 |         print("Hi!")
    |
 
-
+no_self_use.py:103:9: PLR6301 Method `validate_y` could be a function, class method, or static method
+    |
+102 |     @y.validator
+103 |     def validate_y(self, attribute, value):
+    |         ^^^^^^^^^^ PLR6301
+104 |         if value <= 0:
+105 |             raise ValueError("y must be a positive integer")
+    |
diff --git a/crates/ruff_python_semantic/src/analyze/visibility.rs b/crates/ruff_python_semantic/src/analyze/visibility.rs
index e28e13d338..a9173f25c7 100644
--- a/crates/ruff_python_semantic/src/analyze/visibility.rs
+++ b/crates/ruff_python_semantic/src/analyze/visibility.rs
@@ -1,4 +1,4 @@
-use ruff_python_ast::{self as ast, Decorator};
+use ruff_python_ast::{self as ast, Decorator, Expr};
 
 use ruff_python_ast::helpers::map_callable;
 use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
@@ -90,6 +90,27 @@ where
     })
 }
 
+/// Returns `true` if a function definition is an `attrs`-like validator based on its decorators.
+pub fn is_validator(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool {
+    decorator_list.iter().any(|decorator| {
+        let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = &decorator.expression else {
+            return false;
+        };
+
+        if attr.as_str() != "validator" {
+            return false;
+        }
+
+        let Expr::Name(value) = value.as_ref() else {
+            return false;
+        };
+
+        semantic
+            .resolve_name(value)
+            .is_some_and(|id| semantic.binding(id).kind.is_assignment())
+    })
+}
+
 /// Returns `true` if a class is an `final`.
 pub fn is_final(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool {
     decorator_list