-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
TypeAliasType type_params accepts typing_extensions.TypeVar also #14840
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
Conversation
9779915 to
c015c58
Compare
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
AlexWaygood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Ah shoot, you probably also should have updated the return type of the |
|
…0598) ## Summary This allows us to handle self-referential bounds/constraints/defaults without panicking. Handles more cases from astral-sh/ty#256 This also changes the way we infer the types of legacy TypeVars. Rather than understanding a constructor call to `typing[_extension].TypeVar` inside of any (arbitrarily nested) expression, and having to use a special `assigned_to` field of the semantic index to try to best-effort figure out what name the typevar was assigned to, we instead understand the creation of a legacy `TypeVar` only in the supported syntactic position (RHS of a simple un-annotated assignment with one target). In any other position, we just infer it as creating an opaque instance of `typing.TypeVar`. (This behavior matches all other type checkers.) So we now special-case TypeVar creation in `TypeInferenceBuilder`, as a special case of an assignment definition, rather than deeper inside call binding. This does mean we re-implement slightly more of argument-parsing, but in practice this is minimal and easy to handle correctly. This is easier to implement if we also make the RHS of a simple (no unpacking) one-target assignment statement no longer a standalone expression. Which is fine to do, because simple one-target assignments don't need to infer the RHS more than once. This is a bonus performance (0-3% across various projects) and significant memory-usage win, since most assignment statements are simple one-target assignment statements, meaning we now create many fewer standalone-expression salsa ingredients. This change does mean that inference of manually-constructed `TypeAliasType` instances can no longer find its Definition in `assigned_to`, which regresses go-to-definition for these aliases. In a future PR, `TypeAliasType` will receive the same treatment that `TypeVar` did in this PR (moving its special-case inference into `TypeInferenceBuilder` and supporting it only in the correct syntactic position, and lazily inferring its value type to support recursion), which will also fix the go-to-definition regression. (I decided a temporary edge-case regression is better in this case than doubling the size of this PR.) This PR also tightens up and fixes various aspects of the validation of `TypeVar` creation, as seen in the tests. We still (for now) treat all typevars as instances of `typing.TypeVar`, even if they were created using `typing_extensions.TypeVar`. This means we'll wrongly error on e.g. `T.__default__` on Python 3.11, even if `T` is a `typing_extensions.TypeVar` instance at runtime. We share this wrong behavior with both mypy and pyrefly. It will be easier to fix after we pull in python/typeshed#14840. There are some issues that showed up here with typevar identity and `MarkTypeVarsInferable`; the fix here (using the new `original` field and `is_identical_to` methods on `BoundTypeVarInstance` and `TypeVarInstance`) is a bit kludgy, but it can go away when we eliminate `MarkTypeVarsInferable`. ## Test Plan Added and updated mdtests. ### Conformance suite impact The impact here is all positive: * We now correctly error on a legacy TypeVar with exactly one constraint type given. * We now correctly error on a legacy TypeVar with both an upper bound and constraints specified. ### Ecosystem impact Basically none; in the setuptools case we just issue slightly different errors on an invalid TypeVar definition, due to the modified validation code. --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
The
type_paramsattribute ofTypeAliasTypeshould accept instances oftyping_extensions.TypeVar, as well as oftyping.TypeVar(on those Python versions in which these are different classes.) Same goes forParamSpecandTypeVarTuple.