Skip to content

Commit

Permalink
[pyupgrade] Add rules to use PEP 695 generics in classes and functi…
Browse files Browse the repository at this point in the history
…ons (`UP046`, `UP047`) (#15565)

## Summary

This PR extends our [PEP 695](https://peps.python.org/pep-0695) handling
from the type aliases handled by `UP040` to generic function and class
parameters, as suggested in the latter two examples from #4617:

```python
# Input
T = TypeVar("T", bound=float)
class A(Generic[T]):
    ...

def f(t: T):
    ...

# Output
class A[T: float]:
    ...

def f[T: float](t: T):
    ...
```

I first implemented this as part of `UP040`, but based on a brief
discussion during a very helpful pairing session with @AlexWaygood, I
opted to split them into rules separate from `UP040` and then also
separate from each other. From a quick look, and based on [this
issue](asottile/pyupgrade#836), I'm pretty
sure neither of these rules is currently in pyupgrade, so I just took
the next available codes, `UP046` and `UP047`.

The last main TODO, noted in the rule file and in the fixture, is to
handle generic method parameters not included in the class itself, `S`
in this case:

```python
T = TypeVar("T")
S = TypeVar("S")

class Foo(Generic[T]):
    def bar(self, x: T, y: S) -> S: ...
```

but Alex mentioned that that might be okay to leave for a follow-up PR.

I also left a TODO about handling multiple subclasses instead of bailing
out when more than one is present. I'm not sure how common that would
be, but I can still handle it here, or follow up on that too.

I think this is unrelated to the PR, but when I ran `cargo dev
generate-all`, it removed the rule code `PLW0101` from
`ruff.schema.json`. It seemed unrelated, so I left that out, but I
wanted to mention it just in case.

## Test Plan

New test fixture, `cargo nextest run`

Closes #4617, closes #12542

---------

Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
ntBre and AlexWaygood authored Jan 22, 2025
1 parent b4877f1 commit bb6fb46
Show file tree
Hide file tree
Showing 17 changed files with 1,276 additions and 229 deletions.
10 changes: 9 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import typing
from typing import TypeAlias
from typing import Any, TypeAlias

# UP040
x: typing.TypeAlias = int
Expand Down Expand Up @@ -43,6 +43,10 @@ class Foo:
T = typing.TypeVar(*args)
x: typing.TypeAlias = list[T]

# `default` should be skipped for now, added in Python 3.13
T = typing.TypeVar("T", default=Any)
x: typing.TypeAlias = list[T]

# OK
x: TypeAlias
x: int = 1
Expand Down Expand Up @@ -85,3 +89,7 @@ class Foo:
PositiveList = TypeAliasType(
"PositiveList2", list[Annotated[T, Gt(0)]], type_params=(T,)
)

# `default` should be skipped for now, added in Python 3.13
T = typing.TypeVar("T", default=Any)
AnyList = TypeAliasType("AnyList", list[T], typep_params=(T,))
90 changes: 90 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_0.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
from typing import Any, AnyStr, Generic, ParamSpec, TypeVar, TypeVarTuple

from somewhere import SupportsRichComparisonT

S = TypeVar("S", str, bytes) # constrained type variable
T = TypeVar("T", bound=float)
Ts = TypeVarTuple("Ts")
P = ParamSpec("P")


class A(Generic[T]):
# Comments in a class body are preserved
var: T


class B(Generic[*Ts]):
var: tuple[*Ts]


class C(Generic[P]):
var: P


class Constrained(Generic[S]):
var: S


# This case gets a diagnostic but not a fix because we can't look up the bounds
# or constraints on the TypeVar imported from another module
class ExternalType(Generic[T, SupportsRichComparisonT]):
var: T
compare: SupportsRichComparisonT


# typing.AnyStr is a common external type variable, so treat it specially as a
# known TypeVar
class MyStr(Generic[AnyStr]):
s: AnyStr


class MultipleGenerics(Generic[S, T, *Ts, P]):
var: S
typ: T
tup: tuple[*Ts]
pep: P


class MultipleBaseClasses(list, Generic[T]):
var: T


# These cases are not handled
class D(Generic[T, T]): # duplicate generic variable, runtime error
pass


# TODO(brent) we should also apply the fix to methods, but it will need a
# little more work. these should be left alone for now but be fixed eventually.
class NotGeneric:
# -> generic_method[T: float](t: T)
def generic_method(t: T) -> T:
return t


# This one is strange in particular because of the mix of old- and new-style
# generics, but according to the PEP, this is okay "if the class, function, or
# type alias does not use the new syntax." `more_generic` doesn't use the new
# syntax, so it can use T from the module and U from the class scope.
class MixedGenerics[U]:
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)


class DefaultTypeVar(Generic[V]): # -> [V: str = Any]
var: V


# nested classes and functions are skipped
class Outer:
class Inner(Generic[T]):
var: T
12 changes: 12 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"""Replacing AnyStr requires specifying the constraints `bytes` and `str`, so
it can't be replaced if these have been shadowed. This test is in a separate
fixture because it doesn't seem possible to restore `str` to its builtin state
"""

from typing import AnyStr, Generic

str = "string"


class BadStr(Generic[AnyStr]):
var: AnyStr
59 changes: 59 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from collections.abc import Callable
from typing import Any, AnyStr, ParamSpec, TypeVar, TypeVarTuple

from somewhere import Something

S = TypeVar("S", str, bytes) # constrained type variable
T = TypeVar("T", bound=float)
Ts = TypeVarTuple("Ts")
P = ParamSpec("P")


def f(t: T) -> T:
return t


def g(ts: tuple[*Ts]) -> tuple[*Ts]:
return ts


def h(
p: Callable[P, T],
# Comment in the middle of a parameter list should be preserved
another_param,
and_another,
) -> Callable[P, T]:
return p


def i(s: S) -> S:
return s


# NOTE this case is the reason the fix is marked unsafe. If we can't confirm
# that one of the type parameters (`Something` in this case) is a TypeVar,
# which we can't do across module boundaries, we will not convert it to a
# generic type parameter. This leads to code that mixes old-style standalone
# TypeVars with the new-style generic syntax and will be rejected by type
# checkers
def broken_fix(okay: T, bad: Something) -> tuple[T, Something]:
return (okay, bad)


def any_str_param(s: AnyStr) -> AnyStr:
return s


# these cases are not handled

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


def default_var(v: V) -> V:
return v


def outer():
def inner(t: T) -> T:
return t
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::PytestParameterWithDefaultArgument) {
flake8_pytest_style::rules::parameter_with_default_argument(checker, function_def);
}
if checker.enabled(Rule::NonPEP695GenericFunction) {
pyupgrade::rules::non_pep695_generic_function(checker, function_def);
}
}
Stmt::Return(_) => {
if checker.enabled(Rule::ReturnOutsideFunction) {
Expand Down Expand Up @@ -554,6 +557,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::DataclassEnum) {
ruff::rules::dataclass_enum(checker, class_def);
}
if checker.enabled(Rule::NonPEP695GenericClass) {
pyupgrade::rules::non_pep695_generic_class(checker, class_def);
}
}
Stmt::Import(ast::StmtImport { names, range: _ }) => {
if checker.enabled(Rule::MultipleImportsOnOneLine) {
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pyupgrade, "043") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryDefaultTypeArgs),
(Pyupgrade, "044") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP646Unpack),
(Pyupgrade, "045") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP604AnnotationOptional),
(Pyupgrade, "046") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP695GenericClass),
(Pyupgrade, "047") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP695GenericFunction),

// pydocstyle
(Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule),
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/rules/pyupgrade/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ mod tests {
#[test_case(Rule::YieldInForLoop, Path::new("UP028_1.py"))]
#[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.py"))]
#[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.pyi"))]
#[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"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = path.to_string_lossy().to_string();
let diagnostics = test_path(
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub(crate) use native_literals::*;
pub(crate) use open_alias::*;
pub(crate) use os_error_alias::*;
pub(crate) use outdated_version_block::*;
pub(crate) use pep695::*;
pub(crate) use printf_string_formatting::*;
pub(crate) use quoted_annotation::*;
pub(crate) use redundant_open_modes::*;
Expand All @@ -36,7 +37,6 @@ pub(crate) use use_pep585_annotation::*;
pub(crate) use use_pep604_annotation::*;
pub(crate) use use_pep604_isinstance::*;
pub(crate) use use_pep646_unpack::*;
pub(crate) use use_pep695_type_alias::*;
pub(crate) use useless_metaclass_type::*;
pub(crate) use useless_object_inheritance::*;
pub(crate) use yield_in_for_loop::*;
Expand All @@ -57,6 +57,7 @@ mod native_literals;
mod open_alias;
mod os_error_alias;
mod outdated_version_block;
mod pep695;
mod printf_string_formatting;
mod quoted_annotation;
mod redundant_open_modes;
Expand All @@ -79,7 +80,6 @@ mod use_pep585_annotation;
mod use_pep604_annotation;
mod use_pep604_isinstance;
mod use_pep646_unpack;
mod use_pep695_type_alias;
mod useless_metaclass_type;
mod useless_object_inheritance;
mod yield_in_for_loop;
Loading

0 comments on commit bb6fb46

Please sign in to comment.