ruff
3ecea10a - [`pyupgrade`] Make the fix for `UP040` always unsafe (#28526)

Commit
43 days ago
[`pyupgrade`] Make the fix for `UP040` always unsafe (#28526) Summary -- This turned into a bit of a rabbit hole, and the very short summary is that the fix for `UP040` is now always unsafe and documented as such. Read on for more details. `UP040`, `UP046`, and `UP047` all transform code using legacy type variables (`typing.{TypeVar, TypeVarTuple, ParamSpec}`) to use PEP-695 syntax instead (`UP040` also applies to non-generic type aliases but generics are the problem here). This means transformations like this: ```diff from typing import Generic, TypeVar, TypeAlias, TypeAliasType T = TypeVar("T") # UP040 -Alias: TypeAlias = list[T] +type Alias[T] = list[T] # UP040 -AliasType = TypeAliasType("AliasType", set[T], type_params=(T,)) +type AliasType[T] = set[T] # UP046 -class C(Generic[T]): ... +class C[T]: ... # UP047 -def f(x: T): ... +def f[T](x: T): ... ``` This works very well if the type variable is defined in the same file. However, if `T` is imported from another file, it could have bounds, constraints, defaults, or a different type entirely that is mishandled by the autofix. As it turns out, `UP046` and `UP047` already handle this pretty gracefully. Their fixes are _always_ unsafe, and the limitations around imports are documented. `UP047` can't tell if an imported type is generic at all, so it simply doesn't emit a diagnostic for imported annotation types. `UP046` offers a diagnostic but not a fix. We _could_ consider an unsafe or display-only fix, perhaps, but I think we omitted the fix as being even more unsafe than the normally unsafe fix. That leaves `UP040` as the problematic case. Its fix for `TypeAlias`es was also already unsafe in `.py` files but safe in `.pyi` files. Unfortunately, the fix can still cause problems in stub files in a case like this: ```py from typing import TypeAlias from somewhere import MyTypeVar ListOfInt: TypeAlias = list[MyTypeVar] ``` The proposed safe fix is: ```diff -ListOfInt: TypeAlias = list[MyTypeVar] +type ListOfInt = list[MyTypeVar] ``` which is invalid: https://play.ty.dev/6385facd-2a61-4f3e-8961-9124c645643a because it mixes the old-style type variable with the new `type` statement instead of making the `type` statement itself generic: ```py type ListOfInt[MyTypeVar] = list[MyTypeVar] ``` The example here uses `.py` files and blocks for convenience, but the same safe fix is offered for stub files. Similarly, the playground link uses a bounded type variable, but the problem occurs even without bounds or other restraints. Second, the fix for `TypeAliasType` was also safe, even for imported type variables. The problem here is very similar to that above. For imported type variables, the safe fix could lose bounds and constraints or even the kind of type variable. For example: ```diff from typing import TypeAliasType from somewhere import Ts # somewhere.py # from typing import TypeVarTuple # Ts = TypeVarTuple("Ts", bound=int) -MyAliasType = TypeAliasType("MyAliasType", tuple[*Ts], type_params=(Ts,)) +type MyAliasType[Ts] = tuple[*Ts] ``` This presents `Ts` as a regular `TypeVar` with no bound instead of a bounded `TypeVarTuple`. I tried following `UP046` and omitting the fix for imported type variables, but it was a little bit awkward for the current structure of `UP040`. I think it's sufficient and less intrusive into the current implementation to mark it unsafe. I also considered trying to retain safe fixes for `UP040` in stub files when no imported types are involved, but making the fix always unsafe again seemed like the more consistent and simpler approach. From what I remember, none of these rules typically has much ecosystem impact anyway. [#28505]: https://github.com/astral-sh/ruff/pull/28505 [PEP 695]: https://peps.python.org/pep-0695/ Test Plan -- New mdtests and also the ecosystem check here
Author
Parents
Loading