[`flake8-pyi`] Fix PYI034 to not trigger on metaclasses (`PYI034`) (#20881)

## Summary

Fixes #20781

---------

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py

@@ -359,3 +359,29 @@ class Generic5(list[PotentialTypeVar]):
     def __new__(cls: type[Generic5]) -> Generic5: ...
     def __enter__(self: Generic5) -> Generic5: ...
 
+
+# Test cases based on issue #20781 - metaclasses that triggers IsMetaclass::Maybe
+class MetaclassInWhichSelfCannotBeUsed5(type(Protocol)):
+    def __new__(
+        cls, name: str, bases: tuple[type[Any], ...], attrs: dict[str, Any], **kwargs: Any
+    ) -> MetaclassInWhichSelfCannotBeUsed5:
+        new_class = super().__new__(cls, name, bases, attrs, **kwargs)
+        return new_class
+
+
+import django.db.models.base
+
+
+class MetaclassInWhichSelfCannotBeUsed6(django.db.models.base.ModelBase):
+    def __new__(cls, name: str, bases: tuple[Any, ...], attrs: dict[str, Any], **kwargs: Any) -> MetaclassInWhichSelfCannotBeUsed6:
+        ...
+
+
+class MetaclassInWhichSelfCannotBeUsed7(django.db.models.base.ModelBase):
+    def __new__(cls, /, name: str, bases: tuple[object, ...], attrs: dict[str, object], **kwds: object) -> MetaclassInWhichSelfCannotBeUsed7:
+        ...
+
+
+class MetaclassInWhichSelfCannotBeUsed8(django.db.models.base.ModelBase):
+    def __new__(cls, name: builtins.str, bases: tuple, attributes: dict, /, **kw) -> MetaclassInWhichSelfCannotBeUsed8:
+        ...

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.pyi

@@ -252,3 +252,28 @@ from some_module import PotentialTypeVar
 class Generic5(list[PotentialTypeVar]):
     def __new__(cls: type[Generic5]) -> Generic5: ...
     def __enter__(self: Generic5) -> Generic5: ...
+
+
+# Test case based on issue #20781 - metaclass that triggers IsMetaclass::Maybe
+class MetaclassInWhichSelfCannotBeUsed5(type(Protocol)):
+    def __new__(
+        cls, name: str, bases: tuple[type[Any], ...], attrs: dict[str, Any], **kwargs: Any
+    ) -> MetaclassInWhichSelfCannotBeUsed5: ...
+
+
+import django.db.models.base
+
+
+class MetaclassInWhichSelfCannotBeUsed6(django.db.models.base.ModelBase):
+    def __new__(cls, name: str, bases: tuple[Any, ...], attrs: dict[str, Any], **kwargs: Any) -> MetaclassInWhichSelfCannotBeUsed6:
+        ...
+
+
+class MetaclassInWhichSelfCannotBeUsed7(django.db.models.base.ModelBase):
+    def __new__(cls, /, name: str, bases: tuple[object, ...], attrs: dict[str, object], **kwds: object) -> MetaclassInWhichSelfCannotBeUsed7:
+        ...
+
+
+class MetaclassInWhichSelfCannotBeUsed8(django.db.models.base.ModelBase):
+    def __new__(cls, name: builtins.str, bases: tuple, attributes: dict, /, **kw) -> MetaclassInWhichSelfCannotBeUsed8:
+        ...

crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs

@@ -50,6 +50,29 @@ use ruff_text_size::Ranged;
 /// 1. `__aiter__` methods that return `AsyncIterator`, despite the class
 ///    inheriting directly from `AsyncIterator`.
 ///
+/// The rule attempts to avoid flagging methods on metaclasses, since
+/// [PEP 673] specifies that `Self` is disallowed in metaclasses. Ruff can
+/// detect a class as being a metaclass if it inherits from a stdlib
+/// metaclass such as `builtins.type` or `abc.ABCMeta`, and additionally
+/// infers that a class may be a metaclass if it has a `__new__` method
+/// with a similar signature to `type.__new__`. The heuristic used to
+/// identify a metaclass-like `__new__` method signature is that it:
+///
+/// 1. Has exactly 5 parameters (including `cls`)
+/// 1. Has a second parameter annotated with `str`
+/// 1. Has a third parameter annotated with a `tuple` type
+/// 1. Has a fourth parameter annotated with a `dict` type
+/// 1. Has a fifth parameter is keyword-variadic (`**kwargs`)
+///
+/// For example, the following class would be detected as a metaclass, disabling
+/// the rule:
+///
+/// ```python
+/// class MyMetaclass(django.db.models.base.ModelBase):
+///     def __new__(cls, name: str, bases: tuple[Any, ...], attrs: dict[str, Any], **kwargs: Any) -> MyMetaclass:
+///         ...
+/// ```
+///
 /// ## Example
 ///
 /// ```pyi
@@ -87,6 +110,8 @@ use ruff_text_size::Ranged;
 ///
 /// ## References
 /// - [Python documentation: `typing.Self`](https://docs.python.org/3/library/typing.html#typing.Self)
+///
+/// [PEP 673]: https://peps.python.org/pep-0673/#valid-locations-for-self
 #[derive(ViolationMetadata)]
 #[violation_metadata(stable_since = "v0.0.271")]
 pub(crate) struct NonSelfReturnType {
@@ -143,7 +168,10 @@ pub(crate) fn non_self_return_type(
     };
 
     // PEP 673 forbids the use of `typing(_extensions).Self` in metaclasses.
-    if analyze::class::is_metaclass(class_def, semantic).is_yes() {
+    if !matches!(
+        analyze::class::is_metaclass(class_def, semantic),
+        analyze::class::IsMetaclass::No
+    ) {
         return;
     }
 

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI034_PYI034.py.snap

@@ -451,6 +451,7 @@ help: Use `Self` as return type
 359 +     def __new__(cls) -> typing.Self: ...
 360 |     def __enter__(self: Generic5) -> Generic5: ...
 361 | 
+362 | 
 note: This is an unsafe fix and may change runtime behavior
 
 PYI034 [*] `__enter__` methods in classes like `Generic5` usually return `self` at runtime
@@ -468,4 +469,6 @@ help: Use `Self` as return type
     -     def __enter__(self: Generic5) -> Generic5: ...
 360 +     def __enter__(self) -> typing.Self: ...
 361 | 
+362 | 
+363 | # Test cases based on issue #20781 - metaclasses that triggers IsMetaclass::Maybe
 note: This is an unsafe fix and may change runtime behavior

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI034_PYI034.pyi.snap

@@ -431,6 +431,8 @@ help: Use `Self` as return type
     -     def __new__(cls: type[Generic5]) -> Generic5: ...
 253 +     def __new__(cls) -> typing.Self: ...
 254 |     def __enter__(self: Generic5) -> Generic5: ...
+255 | 
+256 | 
 note: This is a display-only fix and is likely to be incorrect
 
 PYI034 [*] `__enter__` methods in classes like `Generic5` usually return `self` at runtime
@@ -447,4 +449,7 @@ help: Use `Self` as return type
 253 |     def __new__(cls: type[Generic5]) -> Generic5: ...
     -     def __enter__(self: Generic5) -> Generic5: ...
 254 +     def __enter__(self) -> typing.Self: ...
+255 | 
+256 | 
+257 | # Test case based on issue #20781 - metaclass that triggers IsMetaclass::Maybe
 note: This is a display-only fix and is likely to be incorrect

crates/ruff_python_semantic/src/analyze/class.rs

@@ -317,6 +317,91 @@ impl IsMetaclass {
     }
 }
 
+/// Check if a class has a metaclass-like `__new__` method signature.
+///
+/// A metaclass-like `__new__` method signature has:
+/// 1. Exactly 5 parameters (including cls)
+/// 2. Second parameter annotated with `str`
+/// 3. Third parameter annotated with a `tuple` type
+/// 4. Fourth parameter annotated with a `dict` type
+/// 5. Fifth parameter is keyword-variadic (`**kwargs`)
+///
+/// For example:
+///
+/// ```python
+/// class MyMetaclass(django.db.models.base.ModelBase):
+///     def __new__(cls, name: str, bases: tuple[Any, ...], attrs: dict[str, Any], **kwargs: Any) -> MyMetaclass:
+///         ...
+/// ```
+fn has_metaclass_new_signature(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
+    // Look for a __new__ method in the class body
+    for stmt in &class_def.body {
+        let ast::Stmt::FunctionDef(ast::StmtFunctionDef {
+            name, parameters, ..
+        }) = stmt
+        else {
+            continue;
+        };
+
+        if name != "__new__" {
+            continue;
+        }
+
+        // Check if we have exactly 5 parameters (cls + 4 others)
+        if parameters.len() != 5 {
+            continue;
+        }
+
+        // Check that there is no variadic parameter
+        if parameters.vararg.is_some() {
+            continue;
+        }
+
+        // Check that the last parameter is keyword-variadic (**kwargs)
+        if parameters.kwarg.is_none() {
+            continue;
+        }
+
+        // Check parameter annotations, skipping the first parameter (cls)
+        let mut param_iter = parameters.iter().skip(1);
+
+        // Check second parameter (name: str)
+        let Some(second_param) = param_iter.next() else {
+            continue;
+        };
+        if !second_param
+            .annotation()
+            .is_some_and(|annotation| semantic.match_builtin_expr(map_subscript(annotation), "str"))
+        {
+            continue;
+        }
+
+        // Check third parameter (bases: tuple[...])
+        let Some(third_param) = param_iter.next() else {
+            continue;
+        };
+        if !third_param.annotation().is_some_and(|annotation| {
+            semantic.match_builtin_expr(map_subscript(annotation), "tuple")
+        }) {
+            continue;
+        }
+
+        // Check fourth parameter (attrs: dict[...])
+        let Some(fourth_param) = param_iter.next() else {
+            continue;
+        };
+        if !fourth_param.annotation().is_some_and(|annotation| {
+            semantic.match_builtin_expr(map_subscript(annotation), "dict")
+        }) {
+            continue;
+        }
+
+        return true;
+    }
+
+    false
+}
+
 /// Returns `IsMetaclass::Yes` if the given class is definitely a metaclass,
 /// `IsMetaclass::No` if it's definitely *not* a metaclass, and
 /// `IsMetaclass::Maybe` otherwise.
@@ -349,7 +434,17 @@ pub fn is_metaclass(class_def: &ast::StmtClassDef, semantic: &SemanticModel) ->
     match (is_base_class, maybe) {
         (true, true) => IsMetaclass::Maybe,
         (true, false) => IsMetaclass::Yes,
-        (false, _) => IsMetaclass::No,
+        (false, _) => {
+            // If it has >1 base class and a metaclass-like signature for `__new__`,
+            // then it might be a metaclass.
+            if class_def.bases().is_empty() {
+                IsMetaclass::No
+            } else if has_metaclass_new_signature(class_def, semantic) {
+                IsMetaclass::Maybe
+            } else {
+                IsMetaclass::No
+            }
+        }
     }
 }