[pyupgrade] Classes with mixed type variable style (UP050)

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP050.py

@@ -0,0 +1,70 @@
+from typing import Generic, ParamSpec, TypeVar, TypeVarTuple
+
+
+_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)
+
+_As = TypeVarTuple('_As')
+_Bs = TypeVarTuple('_Bs', bound=tuple[int, str])
+_Cs = TypeVarTuple('_Cs', default=tuple[int, str])
+
+
+_P1 = ParamSpec('_P1')
+_P2 = ParamSpec('_P2', infer_variance=True)
+_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): ...
+class C[T](Generic[_E], list[_E]): ...
+class C[T](list[_F], Generic[_F]): ...
+
+class C[*Ts](Generic[_As]): ...
+class C[*Ts](Generic[_Bs], tuple[*Bs]): ...
+class C[*Ts](Callable[[*_Cs], tuple[*Ts]], Generic[_Cs]): ...
+
+
+class C[**P](Generic[_P1]): ...
+class C[**P](Generic[_P2]): ...
+class C[**P](Generic[_P3]): ...
+
+
+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]): ...
+
+
+class C[T](Generic[_C], Generic[_D]): ...
+
+
+class C[
+    T  # Comment
+](Generic[_E]): ...
+
+
+### 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

@@ -563,6 +563,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) {
+                pyupgrade::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

@@ -542,6 +542,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pyupgrade, "045") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP604AnnotationOptional),
         (Pyupgrade, "046") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP695GenericClass),
         (Pyupgrade, "047") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP695GenericFunction),
+        (Pyupgrade, "050") => (RuleGroup::Preview, rules::pyupgrade::rules::ClassWithMixedTypeVars),
 
         // pydocstyle
         (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule),

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

@@ -106,6 +106,7 @@ mod tests {
     #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_0.py"))]
     #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_1.py"))]
     #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047.py"))]
+    #[test_case(Rule::ClassWithMixedTypeVars, Path::new("UP050.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = path.to_string_lossy().to_string();
         let diagnostics = test_path(

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

@@ -0,0 +1,200 @@
+use std::iter;
+
+use crate::checkers::ast::Checker;
+use crate::fix::edits::{remove_argument, Parentheses};
+use crate::rules::pyupgrade::rules::pep695::{
+    expr_name_to_type_var, find_generic, DisplayTypeVars, TypeParamKind, TypeVar,
+};
+use crate::settings::types::PythonVersion;
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::{Arguments, Expr, ExprSubscript, ExprTuple, StmtClassDef, TypeParams};
+use ruff_python_semantic::{Binding, BindingKind, SemanticModel};
+
+/// ## What it does
+/// Checks for classes that both have PEP 695 type parameter list
+/// and inherit from `typing.Generic` or `typing_extensions.Generic`.
+///
+/// ## Why is this bad?
+/// Such classes cause errors at runtime:
+///
+/// ```python
+/// from typing import Generic, TypeVar
+///
+/// U = TypeVar("U")
+///
+/// # TypeError: Cannot inherit from Generic[...] multiple times.
+/// class C[T](Generic[U]): ...
+/// ```
+///
+/// ## Example
+///
+/// ```python
+/// from typing import Generic, ParamSpec, TypeVar, TypeVarTuple
+///
+/// U = TypeVar("U")
+/// P = ParamSpec("P")
+/// Ts = TypeVarTuple("Ts")
+///
+/// class C[T](Generic[U, P, Ts]): ...
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// class C[T, U, **P, *Ts]: ...
+/// ```
+#[derive(ViolationMetadata)]
+pub(crate) struct ClassWithMixedTypeVars;
+
+impl Violation for ClassWithMixedTypeVars {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "Class with type parameter list inherits from `Generic`".to_string()
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Convert to new-style".to_string())
+    }
+}
+
+/// UP050
+pub(crate) fn class_with_mixed_type_vars(checker: &mut Checker, class_def: &StmtClassDef) {
+    if checker.settings.target_version < PythonVersion::Py312 {
+        return;
+    }
+
+    let semantic = checker.semantic();
+    let StmtClassDef {
+        type_params,
+        arguments,
+        ..
+    } = class_def;
+
+    let Some(type_params) = type_params.as_deref() else {
+        return;
+    };
+
+    let Some(arguments) = arguments else {
+        return;
+    };
+
+    let Some((generic_base, old_style_type_vars)) =
+        typing_generic_base_and_arguments(arguments, semantic)
+    else {
+        return;
+    };
+
+    let mut diagnostic = Diagnostic::new(ClassWithMixedTypeVars, generic_base.range);
+
+    if let Some(fix) = convert_type_vars(
+        generic_base,
+        old_style_type_vars,
+        type_params,
+        arguments,
+        checker,
+    ) {
+        diagnostic.set_fix(fix);
+    }
+
+    checker.diagnostics.push(diagnostic);
+}
+
+fn typing_generic_base_and_arguments<'a>(
+    class_arguments: &'a Arguments,
+    semantic: &SemanticModel,
+) -> Option<(&'a ExprSubscript, &'a Expr)> {
+    let (_, base @ ExprSubscript { slice, .. }) = find_generic(class_arguments, semantic)?;
+
+    Some((base, slice.as_ref()))
+}
+
+fn convert_type_vars(
+    generic_base: &ExprSubscript,
+    old_style_type_vars: &Expr,
+    type_params: &TypeParams,
+    class_arguments: &Arguments,
+    checker: &Checker,
+) -> Option<Fix> {
+    let mut type_vars = type_params
+        .type_params
+        .iter()
+        .map(|param| TypeVar::from(param))
+        .collect::<Vec<_>>();
+
+    let mut converted_type_vars = match old_style_type_vars {
+        expr @ Expr::Name(_) => {
+            generic_arguments_to_type_vars(iter::once(expr), type_params, checker)?
+        }
+        Expr::Tuple(ExprTuple { elts, .. }) => {
+            generic_arguments_to_type_vars(elts.iter(), type_params, checker)?
+        }
+        _ => return None,
+    };
+
+    type_vars.append(&mut converted_type_vars);
+
+    let source = checker.source();
+    let new_type_params = DisplayTypeVars {
+        type_vars: &type_vars,
+        source,
+    };
+
+    let remove_generic_base =
+        remove_argument(generic_base, class_arguments, Parentheses::Remove, source).ok()?;
+    let replace_type_params =
+        Edit::range_replacement(new_type_params.to_string(), type_params.range);
+
+    Some(Fix::unsafe_edits(
+        remove_generic_base,
+        [replace_type_params],
+    ))
+}
+
+fn generic_arguments_to_type_vars<'a>(
+    exprs: impl Iterator<Item = &'a Expr>,
+    existing_type_params: &TypeParams,
+    checker: &'a Checker,
+) -> Option<Vec<TypeVar<'a>>> {
+    let is_existing_param_of_same_class = |binding: &Binding| {
+        // This first check should have been unnecessary,
+        // as a type parameter list can only contains type-parameter bindings.
+        // Named expressions, for example, are syntax errors.
+        // However, Ruff doesn't know that yet (#11118),
+        // so here it shall remain.
+        matches!(binding.kind, BindingKind::TypeParam)
+            && existing_type_params.range.contains_range(binding.range)
+    };
+
+    let semantic = checker.semantic();
+    let target_version = checker.settings.target_version;
+
+    let mut type_vars = vec![];
+
+    for expr in exprs {
+        let name = expr.as_name_expr()?;
+        let binding = semantic.only_binding(name).map(|id| semantic.binding(id))?;
+
+        if is_existing_param_of_same_class(binding) {
+            continue;
+        }
+
+        let type_var = expr_name_to_type_var(semantic, name)?;
+
+        match (&type_var.kind, &type_var.restriction) {
+            (TypeParamKind::TypeVarTuple, Some(_)) => return None,
+            (TypeParamKind::ParamSpec, Some(_)) => return None,
+            _ => {}
+        }
+
+        if type_var.default.is_some() && target_version < PythonVersion::Py313 {
+            return None;
+        }
+
+        type_vars.push(type_var);
+    }
+
+    Some(type_vars)
+}

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

@@ -9,18 +9,20 @@ 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;
 use ruff_text_size::{Ranged, TextRange};
 
+pub(crate) use class_with_mixed_type_vars::*;
 pub(crate) use non_pep695_generic_class::*;
 pub(crate) use non_pep695_generic_function::*;
 pub(crate) use non_pep695_type_alias::*;
 
 use crate::checkers::ast::Checker;
 
+mod class_with_mixed_type_vars;
 mod non_pep695_generic_class;
 mod non_pep695_generic_function;
 mod non_pep695_type_alias;
@@ -187,6 +189,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>,
@@ -344,3 +374,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(super) 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))
+        })
+    })
+}