Support local and dynamic class- and static-method decorators

crates/ruff_linter/resources/test/fixtures/pep8_naming/N805.py

@@ -63,3 +63,33 @@ def good_method_pos_only(self, blah, /, something: str):
 
     def bad_method_pos_only(this, blah, /, self, something: str):
         pass
+
+
+class ModelClass:
+    @hybrid_property
+    def bad(cls):
+        pass
+
+    @bad.expression
+    def bad(self):
+        pass
+
+    @bad.wtf
+    def bad(cls):
+        pass
+
+    @hybrid_property
+    def good(self):
+        pass
+
+    @good.expression
+    def good(cls):
+        pass
+
+    @good.wtf
+    def good(self):
+        pass
+
+    @foobar.thisisstatic
+    def badstatic(foo):
+        pass

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

@@ -96,6 +96,26 @@ mod tests {
                     classmethod_decorators: vec![
                         "classmethod".to_string(),
                         "pydantic.validator".to_string(),
+                        "expression".to_string(),
+                    ],
+                    ..Default::default()
+                },
+                ..settings::LinterSettings::for_rule(Rule::InvalidFirstArgumentNameForMethod)
+            },
+        )?;
+        assert_messages!(diagnostics);
+        Ok(())
+    }
+
+    #[test]
+    fn staticmethod_decorators() -> Result<()> {
+        let diagnostics = test_path(
+            Path::new("pep8_naming").join("N805.py").as_path(),
+            &settings::LinterSettings {
+                pep8_naming: pep8_naming::settings::Settings {
+                    staticmethod_decorators: vec![
+                        "staticmethod".to_string(),
+                        "thisisstatic".to_string(),
                     ],
                     ..Default::default()
                 },

crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap

@@ -43,4 +43,37 @@ N805.py:64:29: N805 First argument of a method should be named `self`
 65 |         pass
    |
 
+N805.py:70:13: N805 First argument of a method should be named `self`
+   |
+68 | class ModelClass:
+69 |     @hybrid_property
+70 |     def bad(cls):
+   |             ^^^ N805
+71 |         pass
+   |
+
+N805.py:78:13: N805 First argument of a method should be named `self`
+   |
+77 |     @bad.wtf
+78 |     def bad(cls):
+   |             ^^^ N805
+79 |         pass
+   |
+
+N805.py:86:14: N805 First argument of a method should be named `self`
+   |
+85 |     @good.expression
+86 |     def good(cls):
+   |              ^^^ N805
+87 |         pass
+   |
+
+N805.py:94:19: N805 First argument of a method should be named `self`
+   |
+93 |     @foobar.thisisstatic
+94 |     def badstatic(foo):
+   |                   ^^^ N805
+95 |         pass
+   |
+
 

crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap

@@ -27,4 +27,29 @@ N805.py:64:29: N805 First argument of a method should be named `self`
 65 |         pass
    |
 
+N805.py:70:13: N805 First argument of a method should be named `self`
+   |
+68 | class ModelClass:
+69 |     @hybrid_property
+70 |     def bad(cls):
+   |             ^^^ N805
+71 |         pass
+   |
+
+N805.py:78:13: N805 First argument of a method should be named `self`
+   |
+77 |     @bad.wtf
+78 |     def bad(cls):
+   |             ^^^ N805
+79 |         pass
+   |
+
+N805.py:94:19: N805 First argument of a method should be named `self`
+   |
+93 |     @foobar.thisisstatic
+94 |     def badstatic(foo):
+   |                   ^^^ N805
+95 |         pass
+   |
+
 

crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap

@@ -0,0 +1,71 @@
+---
+source: crates/ruff_linter/src/rules/pep8_naming/mod.rs
+---
+N805.py:7:20: N805 First argument of a method should be named `self`
+  |
+6 | class Class:
+7 |     def bad_method(this):
+  |                    ^^^^ N805
+8 |         pass
+  |
+
+N805.py:12:30: N805 First argument of a method should be named `self`
+   |
+10 |     if False:
+11 | 
+12 |         def extra_bad_method(this):
+   |                              ^^^^ N805
+13 |             pass
+   |
+
+N805.py:31:15: N805 First argument of a method should be named `self`
+   |
+30 |     @pydantic.validator
+31 |     def lower(cls, my_field: str) -> str:
+   |               ^^^ N805
+32 |         pass
+   |
+
+N805.py:35:15: N805 First argument of a method should be named `self`
+   |
+34 |     @pydantic.validator("my_field")
+35 |     def lower(cls, my_field: str) -> str:
+   |               ^^^ N805
+36 |         pass
+   |
+
+N805.py:64:29: N805 First argument of a method should be named `self`
+   |
+62 |         pass
+63 | 
+64 |     def bad_method_pos_only(this, blah, /, self, something: str):
+   |                             ^^^^ N805
+65 |         pass
+   |
+
+N805.py:70:13: N805 First argument of a method should be named `self`
+   |
+68 | class ModelClass:
+69 |     @hybrid_property
+70 |     def bad(cls):
+   |             ^^^ N805
+71 |         pass
+   |
+
+N805.py:78:13: N805 First argument of a method should be named `self`
+   |
+77 |     @bad.wtf
+78 |     def bad(cls):
+   |             ^^^ N805
+79 |         pass
+   |
+
+N805.py:86:14: N805 First argument of a method should be named `self`
+   |
+85 |     @good.expression
+86 |     def good(cls):
+   |              ^^^ N805
+87 |         pass
+   |
+
+

crates/ruff_python_semantic/src/analyze/function_type.rs

@@ -1,3 +1,4 @@
+use ruff_python_ast::call_path::collect_call_path;
 use ruff_python_ast::call_path::from_qualified_name;
 use ruff_python_ast::helpers::map_callable;
 use ruff_python_ast::Decorator;
@@ -25,20 +26,10 @@ pub fn classify(
     let ScopeKind::Class(class_def) = &scope.kind else {
         return FunctionType::Function;
     };
-    if decorator_list.iter().any(|decorator| {
-        // The method is decorated with a static method decorator (like
-        // `@staticmethod`).
-        semantic
-            .resolve_call_path(map_callable(&decorator.expression))
-            .is_some_and(|call_path| {
-                matches!(
-                    call_path.as_slice(),
-                    ["", "staticmethod"] | ["abc", "abstractstaticmethod"]
-                ) || staticmethod_decorators
-                    .iter()
-                    .any(|decorator| call_path == from_qualified_name(decorator))
-            })
-    }) {
+    if decorator_list
+        .iter()
+        .any(|decorator| is_static_method(decorator, semantic, staticmethod_decorators))
+    {
         FunctionType::StaticMethod
     } else if matches!(name, "__new__" | "__init_subclass__" | "__class_getitem__")
     // Special-case class method, like `__new__`.
@@ -50,23 +41,91 @@ pub fn classify(
                     matches!(call_path.as_slice(), ["", "type"] | ["abc", "ABCMeta"])
                 })
         })
-        || decorator_list.iter().any(|decorator| {
-            // The method is decorated with a class method decorator (like `@classmethod`).
-            semantic
-                .resolve_call_path(map_callable(&decorator.expression))
-                .is_some_and( |call_path| {
-                    matches!(
-                        call_path.as_slice(),
-                        ["", "classmethod"] | ["abc", "abstractclassmethod"]
-                    ) || classmethod_decorators
-                        .iter()
-                        .any(|decorator| call_path == from_qualified_name(decorator))
-                })
-        })
+        || decorator_list.iter().any(|decorator| is_class_method(decorator, semantic, classmethod_decorators))
     {
         FunctionType::ClassMethod
     } else {
         // It's an instance method.
         FunctionType::Method
     }
 }
+
+/// Return `true` if a [`Decorator`] is indicative of a static method.
+fn is_static_method(
+    decorator: &Decorator,
+    semantic: &SemanticModel,
+    staticmethod_decorators: &[String],
+) -> bool {
+    let decorator = map_callable(&decorator.expression);
+
+    // The decorator is an import, so should match against a qualified path.
+    if semantic
+        .resolve_call_path(decorator)
+        .is_some_and(|call_path| {
+            matches!(
+                call_path.as_slice(),
+                ["", "staticmethod"] | ["abc", "abstractstaticmethod"]
+            ) || staticmethod_decorators
+                .iter()
+                .any(|decorator| call_path == from_qualified_name(decorator))
+        })
+    {
+        return true;
+    }
+
+    // We do not have a resolvable call path, most likely from a decorator like
+    // `@someproperty.setter`. Instead, match on the last element.
+    if !staticmethod_decorators.is_empty() {
+        if collect_call_path(decorator).is_some_and(|call_path| {
+            call_path.last().is_some_and(|tail| {
+                staticmethod_decorators
+                    .iter()
+                    .any(|decorator| tail == decorator)
+            })
+        }) {
+            return true;
+        }
+    }
+
+    false
+}
+
+/// Return `true` if a [`Decorator`] is indicative of a class method.
+fn is_class_method(
+    decorator: &Decorator,
+    semantic: &SemanticModel,
+    classmethod_decorators: &[String],
+) -> bool {
+    let decorator = map_callable(&decorator.expression);
+
+    // The decorator is an import, so should match against a qualified path.
+    if semantic
+        .resolve_call_path(decorator)
+        .is_some_and(|call_path| {
+            matches!(
+                call_path.as_slice(),
+                ["", "classmethod"] | ["abc", "abstractclassmethod"]
+            ) || classmethod_decorators
+                .iter()
+                .any(|decorator| call_path == from_qualified_name(decorator))
+        })
+    {
+        return true;
+    }
+
+    // We do not have a resolvable call path, most likely from a decorator like
+    // `@someproperty.setter`. Instead, match on the last element.
+    if !classmethod_decorators.is_empty() {
+        if collect_call_path(decorator).is_some_and(|call_path| {
+            call_path.last().is_some_and(|tail| {
+                classmethod_decorators
+                    .iter()
+                    .any(|decorator| tail == decorator)
+            })
+        }) {
+            return true;
+        }
+    }
+
+    false
+}

crates/ruff_workspace/src/options.rs

@@ -2242,13 +2242,20 @@ pub struct Pep8NamingOptions {
     /// in this list takes a `cls` argument as its first argument.
     ///
     /// Expects to receive a list of fully-qualified names (e.g., `pydantic.validator`,
-    /// rather than `validator`).
+    /// rather than `validator`) or alternatively a plain name which is then matched against
+    /// the last segment in case the decorator itself consists of a dotted name.
     #[option(
         default = r#"[]"#,
         value_type = "list[str]",
         example = r#"
-            # Allow Pydantic's `@validator` decorator to trigger class method treatment.
-            classmethod-decorators = ["pydantic.validator"]
+            classmethod-decorators = [
+                # Allow Pydantic's `@validator` decorator to trigger class method treatment.
+                "pydantic.validator",
+                # Allow SQLAlchemy's dynamic decorators, like `@field.expression`, to trigger class method treatment.
+                "declared_attr",
+                "expression",
+                "comparator",
+            ]
         "#
     )]
     pub classmethod_decorators: Option<Vec<String>>,
@@ -2261,7 +2268,8 @@ pub struct Pep8NamingOptions {
     /// in this list has no `self` or `cls` argument.
     ///
     /// Expects to receive a list of fully-qualified names (e.g., `belay.Device.teardown`,
-    /// rather than `teardown`).
+    /// rather than `teardown`) or alternatively a plain name which is then matched against
+    /// the last segment in case the decorator itself consists of a dotted name.
     #[option(
         default = r#"[]"#,
         value_type = "list[str]",