[ruff] Classes with mixed type variable style (RUF053) (#15841)

crates/ruff_linter/resources/test/fixtures/ruff/RUF053.py

@@ -0,0 +1,109 @@
+from typing import Generic, ParamSpec, TypeVar, TypeVarTuple, Unpack
+
+
+_A = TypeVar('_A')
+_B = TypeVar('_B', bound=int)
+_C = TypeVar('_C', str, bytes)
+_D = TypeVar('_D', default=int)
+_E = TypeVar('_E', bound=int, default=int)
+_F = TypeVar('_F', str, bytes, default=str)
+_G = TypeVar('_G', str, a := int)
+
+_As = TypeVarTuple('_As')
+_Bs = TypeVarTuple('_Bs', bound=tuple[int, str])
+_Cs = TypeVarTuple('_Cs', default=tuple[int, str])
+
+_P1 = ParamSpec('_P1')
+_P2 = ParamSpec('_P2', bound=[bytes, bool])
+_P3 = ParamSpec('_P3', default=[int, str])
+
+
+### Errors
+
+class C[T](Generic[_A]): ...
+class C[T](Generic[_B], str): ...
+class C[T](int, Generic[_C]): ...
+class C[T](bytes, Generic[_D], bool): ...  # TODO: Type parameter defaults
+class C[T](Generic[_E], list[_E]): ...  # TODO: Type parameter defaults
+class C[T](list[_F], Generic[_F]): ...  # TODO: Type parameter defaults
+
+class C[*Ts](Generic[*_As]): ...
+class C[*Ts](Generic[Unpack[_As]]): ...
+class C[*Ts](Generic[Unpack[_Bs]], tuple[*Bs]): ...
+class C[*Ts](Callable[[*_Cs], tuple[*Ts]], Generic[_Cs]): ...  # TODO: Type parameter defaults
+
+
+class C[**P](Generic[_P1]): ...
+class C[**P](Generic[_P2]): ...
+class C[**P](Generic[_P3]): ...  # TODO: Type parameter defaults
+
+
+class C[T](Generic[T, _A]): ...
+
+
+# See `is_existing_param_of_same_class`.
+# `expr_name_to_type_var` doesn't handle named expressions,
+# only simple assignments, so there is no fix.
+class C[T: (_Z := TypeVar('_Z'))](Generic[_Z]): ...
+
+
+class C(Generic[_B]):
+    class D[T](Generic[_B, T]): ...
+
+
+class C[T]:
+    class D[U](Generic[T, U]): ...
+
+
+# In a single run, only the first is reported.
+# Others will be reported/fixed in following iterations.
+class C[T](Generic[_C], Generic[_D]): ...
+class C[T, _C: (str, bytes)](Generic[_D]): ...  # TODO: Type parameter defaults
+
+
+class C[
+T  # Comment
+](Generic[_E]): ...  # TODO: Type parameter defaults
+
+
+class C[T](Generic[Generic[_F]]): ...
+class C[T](Generic[Unpack[_A]]): ...
+class C[T](Generic[Unpack[_P1]]): ...
+class C[T](Generic[Unpack[Unpack[_P2]]]): ...
+class C[T](Generic[Unpack[*_As]]): ...
+class C[T](Generic[Unpack[_As, _Bs]]): ...
+
+
+class C[T](Generic[_A, _A]): ...
+class C[T](Generic[_A, Unpack[_As]]): ...
+class C[T](Generic[*_As, _A]): ...
+
+
+from somewhere import APublicTypeVar
+class C[T](Generic[APublicTypeVar]): ...
+class C[T](Generic[APublicTypeVar, _A]): ...
+
+
+# `_G` has two constraints: `str` and `a := int`.
+# The latter cannot be used as a PEP 695 constraint,
+# as named expressions are forbidden within type parameter lists.
+# See also the `_Z` example above.
+class C[T](Generic[_G]): ...  # Should be moved down below eventually
+
+
+# Single-element constraints should not be converted to a bound.
+class C[T: (str,)](Generic[_A]): ...
+class C[T: [a]](Generic[_A]): ...
+
+
+# Existing bounds should not be deparenthesized.
+# class C[T: (_Y := int)](Generic[_A]): ...  # TODO: Uncomment this
+# class C[T: (*a,)](Generic[_A]): ...        # TODO: Uncomment this
+
+
+### No errors
+
+class C(Generic[_A]): ...
+class C[_A]: ...
+class C[_A](list[_A]): ...
+class C[_A](list[Generic[_A]]): ...

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -554,6 +554,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::NonPEP695GenericClass) {
                 pyupgrade::rules::non_pep695_generic_class(checker, class_def);
             }
+            if checker.enabled(Rule::ClassWithMixedTypeVars) {
+                ruff::rules::class_with_mixed_type_vars(checker, class_def);
+            }
         }
         Stmt::Import(ast::StmtImport { names, range: _ }) => {
             if checker.enabled(Rule::MultipleImportsOnOneLine) {

crates/ruff_linter/src/codes.rs

@@ -1005,6 +1005,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "049") => (RuleGroup::Preview, rules::ruff::rules::DataclassEnum),
         (Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel),
         (Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable),
+        (Ruff, "053") => (RuleGroup::Preview, rules::ruff::rules::ClassWithMixedTypeVars),
         (Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),
         (Ruff, "056") => (RuleGroup::Preview, rules::ruff::rules::FalsyDictGetFallback),
         (Ruff, "057") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRound),

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

@@ -57,7 +57,7 @@ mod native_literals;
 mod open_alias;
 mod os_error_alias;
 mod outdated_version_block;
-mod pep695;
+pub(crate) mod pep695;
 mod printf_string_formatting;
 mod quoted_annotation;
 mod redundant_open_modes;

crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs

@@ -9,7 +9,7 @@ use ruff_python_ast::{
     self as ast,
     name::Name,
     visitor::{self, Visitor},
-    Expr, ExprCall, ExprName, ExprSubscript, Identifier, Stmt, StmtAssign, TypeParam,
+    Arguments, Expr, ExprCall, ExprName, ExprSubscript, Identifier, Stmt, StmtAssign, TypeParam,
     TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple,
 };
 use ruff_python_semantic::SemanticModel;
@@ -28,7 +28,7 @@ mod non_pep695_type_alias;
 mod private_type_parameter;
 
 #[derive(Debug)]
-enum TypeVarRestriction<'a> {
+pub(crate) enum TypeVarRestriction<'a> {
     /// A type variable with a bound, e.g., `TypeVar("T", bound=int)`.
     Bound(&'a Expr),
     /// A type variable with constraints, e.g., `TypeVar("T", int, str)`.
@@ -39,25 +39,25 @@ enum TypeVarRestriction<'a> {
 }
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-enum TypeParamKind {
+pub(crate) enum TypeParamKind {
     TypeVar,
     TypeVarTuple,
     ParamSpec,
 }
 
 #[derive(Debug)]
-struct TypeVar<'a> {
-    name: &'a str,
-    restriction: Option<TypeVarRestriction<'a>>,
-    kind: TypeParamKind,
-    default: Option<&'a Expr>,
+pub(crate) struct TypeVar<'a> {
+    pub(crate) name: &'a str,
+    pub(crate) restriction: Option<TypeVarRestriction<'a>>,
+    pub(crate) kind: TypeParamKind,
+    pub(crate) default: Option<&'a Expr>,
 }
 
 /// Wrapper for formatting a sequence of [`TypeVar`]s for use as a generic type parameter (e.g. `[T,
 /// *Ts, **P]`). See [`DisplayTypeVar`] for further details.
-struct DisplayTypeVars<'a> {
-    type_vars: &'a [TypeVar<'a>],
-    source: &'a str,
+pub(crate) struct DisplayTypeVars<'a> {
+    pub(crate) type_vars: &'a [TypeVar<'a>],
+    pub(crate) source: &'a str,
 }
 
 impl Display for DisplayTypeVars<'_> {
@@ -81,7 +81,7 @@ impl Display for DisplayTypeVars<'_> {
 
 /// Used for displaying `type_var`. `source` is the whole file, which will be sliced to recover the
 /// `TypeVarRestriction` values for generic bounds and constraints.
-struct DisplayTypeVar<'a> {
+pub(crate) struct DisplayTypeVar<'a> {
     type_var: &'a TypeVar<'a>,
     source: &'a str,
 }
@@ -192,6 +192,34 @@ impl<'a> From<&'a TypeVar<'a>> for TypeParam {
     }
 }
 
+impl<'a> From<&'a TypeParam> for TypeVar<'a> {
+    fn from(param: &'a TypeParam) -> Self {
+        let (kind, restriction) = match param {
+            TypeParam::TypeVarTuple(_) => (TypeParamKind::TypeVarTuple, None),
+            TypeParam::ParamSpec(_) => (TypeParamKind::ParamSpec, None),
+
+            TypeParam::TypeVar(param) => {
+                let restriction = match param.bound.as_deref() {
+                    None => None,
+                    Some(Expr::Tuple(constraints)) => Some(TypeVarRestriction::Constraint(
+                        constraints.elts.iter().collect::<Vec<_>>(),
+                    )),
+                    Some(bound) => Some(TypeVarRestriction::Bound(bound)),
+                };
+
+                (TypeParamKind::TypeVar, restriction)
+            }
+        };
+
+        Self {
+            name: param.name(),
+            kind,
+            restriction,
+            default: param.default(),
+        }
+    }
+}
+
 struct TypeVarReferenceVisitor<'a> {
     vars: Vec<TypeVar<'a>>,
     semantic: &'a SemanticModel<'a>,
@@ -242,7 +270,7 @@ impl<'a> Visitor<'a> for TypeVarReferenceVisitor<'a> {
     }
 }
 
-fn expr_name_to_type_var<'a>(
+pub(crate) fn expr_name_to_type_var<'a>(
     semantic: &'a SemanticModel,
     name: &'a ExprName,
 ) -> Option<TypeVar<'a>> {
@@ -349,3 +377,18 @@ fn check_type_vars(vars: Vec<TypeVar<'_>>) -> Option<Vec<TypeVar<'_>>> {
         == vars.len())
     .then_some(vars)
 }
+
+/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if
+/// any), along with its index in the class's bases tuple.
+pub(crate) fn find_generic<'a>(
+    class_bases: &'a Arguments,
+    semantic: &SemanticModel,
+) -> Option<(usize, &'a ExprSubscript)> {
+    class_bases.args.iter().enumerate().find_map(|(idx, expr)| {
+        expr.as_subscript_expr().and_then(|sub_expr| {
+            semantic
+                .match_typing_expr(&sub_expr.value, "Generic")
+                .then_some((idx, sub_expr))
+        })
+    })
+}

crates/ruff_linter/src/rules/pyupgrade/rules/pep695/non_pep695_generic_class.rs

@@ -1,15 +1,16 @@
 use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast::visitor::Visitor;
-use ruff_python_ast::{Arguments, ExprSubscript, StmtClassDef};
-use ruff_python_semantic::SemanticModel;
+use ruff_python_ast::{ExprSubscript, StmtClassDef};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 use crate::fix::edits::{remove_argument, Parentheses};
 use crate::settings::types::PythonVersion;
 
-use super::{check_type_vars, in_nested_context, DisplayTypeVars, TypeVarReferenceVisitor};
+use super::{
+    check_type_vars, find_generic, in_nested_context, DisplayTypeVars, TypeVarReferenceVisitor,
+};
 
 /// ## What it does
 ///
@@ -211,18 +212,3 @@ pub(crate) fn non_pep695_generic_class(checker: &mut Checker, class_def: &StmtCl
 
     checker.diagnostics.push(diagnostic);
 }
-
-/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if
-/// any), along with its index in the class's bases tuple.
-fn find_generic<'a>(
-    class_bases: &'a Arguments,
-    semantic: &SemanticModel,
-) -> Option<(usize, &'a ExprSubscript)> {
-    class_bases.args.iter().enumerate().find_map(|(idx, expr)| {
-        expr.as_subscript_expr().and_then(|sub_expr| {
-            semantic
-                .match_typing_expr(&sub_expr.value, "Generic")
-                .then_some((idx, sub_expr))
-        })
-    })
-}

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

@@ -435,6 +435,7 @@ mod tests {
     #[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))]
     #[test_case(Rule::StarmapZip, Path::new("RUF058_0.py"))]
     #[test_case(Rule::StarmapZip, Path::new("RUF058_1.py"))]
+    #[test_case(Rule::ClassWithMixedTypeVars, Path::new("RUF053.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",