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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,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
Expand Up @@ -21,10 +21,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 +64,10 @@ 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).
///
/// 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,9 +121,9 @@ 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 {
// only handle the case where Generic is at the end of the argument list, in line with PYI059
// (generic-not-last-base-class)
let [base_classes @ .., Expr::Subscript(ExprSubscript {
value,
slice,
range,
Expand Down Expand Up @@ -179,11 +182,32 @@ 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(),
)));
if base_classes.is_empty() {
// avoid debug_assert for empty range_replacement by using range_deletion here
diagnostic.set_fix(Fix::unsafe_edits(
Edit::insertion(type_params.to_string(), name.end()),
[Edit::range_deletion(arguments.range)],
));
} else {
// also build the replacement argument list as a String, if there are any remaining base
// classes
let base_classes = {
let mut s = String::from("(");
for (i, arg) in base_classes.iter().enumerate() {
s.push_str(&checker.source()[arg.range()]);
if i < base_classes.len() - 1 {
s.push_str(", ");
}
}
s.push(')');
s
};

diagnostic.set_fix(Fix::unsafe_edits(
Edit::insertion(type_params.to_string(), name.end()),
[Edit::range_replacement(base_classes, arguments.range)],
));
};
}

checker.diagnostics.push(diagnostic);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,21 @@ 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 |
Loading