Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pyupgrade] Handle multiple base classes for PEP 695 generics (UP046) #15659

Merged
merged 19 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_0.py
ntBre marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,50 @@ class MultipleBaseClasses(list, Generic[T]):
var: T


# these are just for the MoreBaseClasses and MultipleBaseAndGenerics cases
class Base1: ...


class Base2: ...


class Base3: ...


class MoreBaseClasses(Base1, Base2, Base3, Generic[T]):
var: T


class MultipleBaseAndGenerics(Base1, Base2, Base3, Generic[S, T, *Ts, P]):
var: S
typ: T
tup: tuple[*Ts]
pep: P


class A(Generic[T]): ...


class B(A[S], Generic[S]):
var: S


class C(A[S], Generic[S, T]):
var: tuple[S, T]


class D(A[int], Generic[T]):
var: T


class NotLast(Generic[T], Base1):
var: T


class Sandwich(Base1, Generic[T], Base2):
var: T


# These cases are not handled
class D(Generic[T, T]): # duplicate generic variable, runtime error
pass
Expand All @@ -71,11 +115,6 @@ def more_generic(u: U, t: T) -> tuple[U, T]:
return (u, t)


# TODO(brent) we should also handle multiple base classes
class Multiple(NotGeneric, Generic[T]):
pass


# TODO(brent) default requires 3.13
V = TypeVar("V", default=Any, bound=str)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
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::StmtClassDef;
use ruff_python_ast::{Expr, ExprSubscript};
use ruff_python_ast::ExprSubscript;
use ruff_python_ast::{Arguments, StmtClassDef};
ntBre marked this conversation as resolved.
Show resolved Hide resolved
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};
Expand All @@ -21,10 +22,9 @@ use super::{check_type_vars, in_nested_context, DisplayTypeVars, TypeVarReferenc
///
/// ## Known problems
///
/// The rule currently skips generic classes with multiple base classes. It also skips
/// generic classes nested inside of other
/// functions or classes. Finally, this rule skips type parameters with the `default` argument
/// introduced in [PEP 696] and implemented in Python 3.13.
/// The rule currently skips generic classes nested inside of other functions or classes. It also
/// skips type parameters with the `default` argument introduced in [PEP 696] and implemented in
/// Python 3.13.
///
/// This rule can only offer a fix if all of the generic types in the class definition are defined
/// in the current module. For external type parameters, a diagnostic is emitted without a suggested
Expand Down Expand Up @@ -65,6 +65,11 @@ use super::{check_type_vars, in_nested_context, DisplayTypeVars, TypeVarReferenc
/// [`unused-private-type-var`](unused-private-type-var.md) for a rule to clean up unused
/// private type variables.
///
/// This rule will correctly handle classes with multiple base classes, as long as the single
/// `Generic` base class is at the end of the argument list, as checked by
/// [`generic-not-last-base-class`](generic-not-last-base-class.md). If a `Generic` base class is
/// found outside of the last position, a diagnostic is emitted without a suggested fix.
///
/// This rule only applies to generic classes and does not include generic functions. See
/// [`non-pep695-generic-function`](non-pep695-generic-function.md) for the function version.
///
Expand Down Expand Up @@ -118,29 +123,29 @@ pub(crate) fn non_pep695_generic_class(checker: &mut Checker, class_def: &StmtCl
return;
};

// TODO(brent) only accept a single, Generic argument for now. I think it should be fine to have
// other arguments, but this simplifies the fix just to delete the argument list for now
let [Expr::Subscript(ExprSubscript {
value,
slice,
range,
..
})] = arguments.args.as_ref()
let Some((generic_idx, generic_expr @ ExprSubscript { slice, range, .. })) =
find_generic(arguments, checker)
else {
return;
};

if !checker.semantic().match_typing_expr(value, "Generic") {
return;
}

let mut diagnostic = Diagnostic::new(
NonPEP695GenericClass {
name: name.to_string(),
},
*range,
);

// only handle the case where Generic is at the end of the argument list, in line with PYI059
// (generic-not-last-base-class). If it comes elsewhere, it results in a runtime error. In stubs
// it's not *strictly* necessary for `Generic` to come last in the bases tuple, but it would
// cause more complication for us to handle stubs specially, and probably isn't worth the
// bother. we still offer a diagnostic here but not a fix
if generic_idx != arguments.len() - 1 {
checker.diagnostics.push(diagnostic);
return;
}

let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
Expand Down Expand Up @@ -179,12 +184,41 @@ pub(crate) fn non_pep695_generic_class(checker: &mut Checker, class_def: &StmtCl
source: checker.source(),
};

diagnostic.set_fix(Fix::unsafe_edit(Edit::replacement(
type_params.to_string(),
name.end(),
arguments.end(),
)));
let Ok(edit) = remove_argument(
generic_expr,
arguments,
Parentheses::Remove,
checker.source(),
) else {
return;
};

diagnostic.set_fix(Fix::unsafe_edits(
Edit::insertion(type_params.to_string(), name.end()),
[edit],
));
ntBre marked this conversation as resolved.
Show resolved Hide resolved
}

checker.diagnostics.push(diagnostic);
}

/// Search `arguments` for a `typing.Generic` base class. Returns the `Generic` expression (if any),
/// along with its index in `arguments`.
fn find_generic<'a>(
arguments: &'a Arguments,
checker: &mut Checker,
) -> Option<(usize, &'a ExprSubscript)> {
arguments
.args
.as_ref()
.iter()
.enumerate()
.find_map(|(idx, expr)| {
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
expr.as_subscript_expr().and_then(|sub_expr| {
checker
.semantic()
.match_typing_expr(&sub_expr.value, "Generic")
.then_some((idx, sub_expr))
})
})
}
ntBre marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,145 @@ UP046_0.py:41:24: UP046 [*] Generic class `MultipleGenerics` uses `Generic` subc
42 42 | var: S
43 43 | typ: T
44 44 | tup: tuple[*Ts]

UP046_0.py:48:33: UP046 [*] Generic class `MultipleBaseClasses` uses `Generic` subclass instead of type parameters
|
48 | class MultipleBaseClasses(list, Generic[T]):
| ^^^^^^^^^^ UP046
49 | var: T
|
= help: Use type parameters

ℹ Unsafe fix
45 45 | pep: P
46 46 |
47 47 |
48 |-class MultipleBaseClasses(list, Generic[T]):
48 |+class MultipleBaseClasses[T: float](list):
49 49 | var: T
50 50 |
51 51 |

UP046_0.py:62:44: UP046 [*] Generic class `MoreBaseClasses` uses `Generic` subclass instead of type parameters
|
62 | class MoreBaseClasses(Base1, Base2, Base3, Generic[T]):
| ^^^^^^^^^^ UP046
63 | var: T
|
= help: Use type parameters

ℹ Unsafe fix
59 59 | class Base3: ...
60 60 |
61 61 |
62 |-class MoreBaseClasses(Base1, Base2, Base3, Generic[T]):
62 |+class MoreBaseClasses[T: float](Base1, Base2, Base3):
63 63 | var: T
64 64 |
65 65 |

UP046_0.py:66:52: UP046 [*] Generic class `MultipleBaseAndGenerics` uses `Generic` subclass instead of type parameters
|
66 | class MultipleBaseAndGenerics(Base1, Base2, Base3, Generic[S, T, *Ts, P]):
| ^^^^^^^^^^^^^^^^^^^^^ UP046
67 | var: S
68 | typ: T
|
= help: Use type parameters

ℹ Unsafe fix
63 63 | var: T
64 64 |
65 65 |
66 |-class MultipleBaseAndGenerics(Base1, Base2, Base3, Generic[S, T, *Ts, P]):
66 |+class MultipleBaseAndGenerics[S: (str, bytes), T: float, *Ts, **P](Base1, Base2, Base3):
67 67 | var: S
68 68 | typ: T
69 69 | tup: tuple[*Ts]

UP046_0.py:73:9: UP046 [*] Generic class `A` uses `Generic` subclass instead of type parameters
|
73 | class A(Generic[T]): ...
| ^^^^^^^^^^ UP046
|
= help: Use type parameters

ℹ Unsafe fix
70 70 | pep: P
71 71 |
72 72 |
73 |-class A(Generic[T]): ...
73 |+class A[T: float]: ...
74 74 |
75 75 |
76 76 | class B(A[S], Generic[S]):

UP046_0.py:76:15: UP046 [*] Generic class `B` uses `Generic` subclass instead of type parameters
|
76 | class B(A[S], Generic[S]):
| ^^^^^^^^^^ UP046
77 | var: S
|
= help: Use type parameters

ℹ Unsafe fix
73 73 | class A(Generic[T]): ...
74 74 |
75 75 |
76 |-class B(A[S], Generic[S]):
76 |+class B[S: (str, bytes)](A[S]):
77 77 | var: S
78 78 |
79 79 |

UP046_0.py:80:15: UP046 [*] Generic class `C` uses `Generic` subclass instead of type parameters
|
80 | class C(A[S], Generic[S, T]):
| ^^^^^^^^^^^^^ UP046
81 | var: tuple[S, T]
|
= help: Use type parameters

ℹ Unsafe fix
77 77 | var: S
78 78 |
79 79 |
80 |-class C(A[S], Generic[S, T]):
80 |+class C[S: (str, bytes), T: float](A[S]):
81 81 | var: tuple[S, T]
82 82 |
83 83 |

UP046_0.py:84:17: UP046 [*] Generic class `D` uses `Generic` subclass instead of type parameters
|
84 | class D(A[int], Generic[T]):
| ^^^^^^^^^^ UP046
85 | var: T
|
= help: Use type parameters

ℹ Unsafe fix
81 81 | var: tuple[S, T]
82 82 |
83 83 |
84 |-class D(A[int], Generic[T]):
84 |+class D[T: float](A[int]):
85 85 | var: T
86 86 |
87 87 |

UP046_0.py:88:15: UP046 Generic class `NotLast` uses `Generic` subclass instead of type parameters
|
88 | class NotLast(Generic[T], Base1):
| ^^^^^^^^^^ UP046
89 | var: T
|
= help: Use type parameters

UP046_0.py:92:23: UP046 Generic class `Sandwich` uses `Generic` subclass instead of type parameters
|
92 | class Sandwich(Base1, Generic[T], Base2):
| ^^^^^^^^^^ UP046
93 | var: T
|
= help: Use type parameters
Loading