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

Support ParamSpec variables in type aliases #14159

Merged
merged 15 commits into from
Nov 24, 2022

Conversation

ilevkivskyi
Copy link
Member

Fixes #11855
Fixes #7084
Fixes #10445
Should fix #4987

After thinking about this for some time, it looks like the best way to implement this is by switching type aliases from unbound to bound type variables. Then I can essentially simply share (or copy in one small place, to avoid cyclic imports) all the logic that currently exists for ParamSpec and Concatenate in expand_type() etc.

This will also address a big piece of tech debt, and will get some benefits (almost) for free, such as checking bounds/values for alias type variables, and much tighter handling of unbound type variables.

Note that in this PR I change logic for emitting some errors, I try to avoid showing multiple errors for the same location/reason. But this is not an essential part of this PR (it is just some test cases would otherwise fail with even more error messages), I can reconsider if there are objections.

@ilevkivskyi
Copy link
Member Author

Oh it looks like there is a merge race with one of my other PRs, should be easy to rebase.

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

ilevkivskyi commented Nov 21, 2022

On the mypy_primer:

  • Some remaining errors in discord.py look like mypy is struggling with type inference for unions, doesn't look like a bug in this PR (other errors are correct).
  • psycopg is doing Foo: "TypeAlias" = Bar[T] in two places, where TypeAlias is never imported, so technically the errors are correct
  • There are two things in Expression. One is exposed existing bug that "skip-level" type variables are not bound correctly in functions nested in generic functions. Second one is less clear, but unlikely to caused by this PR, probably also existing issue.

I will try to see if there are easy fixes to the things in Expression to minimize the fallout of this PR.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

@jhance @JukkaL @hauntsaninja Can one of you please take a look at this? It would be great to have a second pair of eyes, maybe I missed something (this PR includes a significant refactoring in how type aliases internals).

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 23, 2022

I did some manual testing, and this seems to generate a false positive when defining a type alias:

from typing import ParamSpec, Callable

P = ParamSpec("P")

# Error: The first argument to Callable must be a list of types, parameter specification, or "..."
A = list[Callable[P, None]]

f: A[[int, str]]
reveal_type(f)  # builtins.list[def (builtins.int, builtins.str)] (seems fine)

@gandhis1
Copy link

As @erictraut mentioned in this comment, the tests that he put together for pyright may be useful for PRs like this one to test edge cases of ParamSpec and Concatenate. Probably only a handful of repos in mypy primer scope have even begun to use this feature. Maybe it makes sense to bring them over, see which ones fail, and then selectively disable test cases that are known to be unsupported?

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 23, 2022

We can now leak type lists into inferred types, which could cause problems (e.g. crashes) since type lists are not generally expected as types, I think:

from typing import ParamSpec, Callable, Concatenate, TypeVar

T = TypeVar("T")
P = ParamSpec("P")

A = Callable[Concatenate[T, P], None]

# Type argument "[int, str]" of "A" must be a subtype of "object"
f: A[[int, str], [str, float]]
reveal_type(f)  # def ([builtins.int, builtins.str], builtins.str, builtins.float)

@ilevkivskyi
Copy link
Member Author

@JukkaL Thanks! I fixed the first one (and also a similar one with Concatenate), will push soon. For the second one there should be an easy fix, we just need to replace invalid args with Any.

@ilevkivskyi
Copy link
Member Author

Btw this second issue affects regular instances as well (I am fixing both anyway)

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 23, 2022

Maybe it makes sense to bring them over, see which ones fail, and then selectively disable test cases that are known to be unsupported?

This could be helpful, since we've had a bunch of ParamSpec edge cases that we've been fixing gradually. We can do it in a separate PR, except maybe for test cases that involve type aliases.

@ilevkivskyi Do you want to have a look if there are any relevant test cases you could include in this PR?

@ilevkivskyi
Copy link
Member Author

TBH I don't have desire to go through them, I would rather spend time working on default arguments in generics. I made this PR because I know type aliases, not because I know ParamSpec :-)

FWIW I think the best way may be if a volunteer would run that cases (and not just ParamSpec ones), and opens issues for failing ones (where it makes sense, e.g. not a duplicate).

@ilevkivskyi
Copy link
Member Author

@JukkaL

We can now leak type lists into inferred types

Btw those are not type lists they are Parameters with a nice repr. Actually it looks like trying to replace them in semanal_typeargs conflicts with TypeVarTuples. So since it is not a new bug, I will leave it as is.

Btw @jhance it looks like semanal_typeargs is already broken for TypeVarTuples, the loop in visit_instance will compare type vars with wrong arguments and vice versa.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray-dataclasses (https://github.com/astropenguin/xarray-dataclasses)
- xarray_dataclasses/datamodel.py:35: error: ParamSpec "PInit" is unbound  [valid-type]
- xarray_dataclasses/datamodel.py:173: error: Bad number of arguments for type alias, expected: 0, given: 1  [type-arg]
- xarray_dataclasses/datamodel.py:173: error: Invalid location for ParamSpec "PInit"  [valid-type]
- xarray_dataclasses/datamodel.py:173: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[PInit, int]'
- xarray_dataclasses/datamodel.py:189: error: Bad number of arguments for type alias, expected: 0, given: 1  [type-arg]
- xarray_dataclasses/datamodel.py:189: error: Invalid location for ParamSpec "PInit"  [valid-type]
- xarray_dataclasses/datamodel.py:189: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[PInit, int]'
- xarray_dataclasses/dataarray.py:29: error: ParamSpec "PInit" is unbound  [valid-type]
- xarray_dataclasses/dataset.py:29: error: ParamSpec "PInit" is unbound  [valid-type]

psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/_compat.py:17: error: Type variable "psycopg._compat.T" is unbound  [valid-type]
+ psycopg/psycopg/_compat.py:17: note: (Hint: Use "Generic[T]" or "Protocol[T]" base class to bind "T" inside a class)
+ psycopg/psycopg/_compat.py:17: note: (Hint: Use "T" in function signature to bind "T" inside a function)
+ psycopg_pool/psycopg_pool/_compat.py:14: error: Type variable "psycopg_pool._compat.T" is unbound  [valid-type]
+ psycopg_pool/psycopg_pool/_compat.py:14: note: (Hint: Use "Generic[T]" or "Protocol[T]" base class to bind "T" inside a class)
+ psycopg_pool/psycopg_pool/_compat.py:14: note: (Hint: Use "T" in function signature to bind "T" inside a function)

discord.py (https://github.com/Rapptz/discord.py)
- discord/ext/commands/_types.py:40: error: The first argument to Callable must be a list of types, parameter specification, or "..."  [valid-type]
- discord/ext/commands/_types.py:40: note: See https://mypy.readthedocs.io/en/stable/kinds_of_types.html#callable-types-and-lambdas
- discord/utils.py:152: error: The first argument to Callable must be a list of types, parameter specification, or "..."  [valid-type]
- discord/utils.py:152: note: See https://mypy.readthedocs.io/en/stable/kinds_of_types.html#callable-types-and-lambdas
- discord/utils.py:659: error: Bad number of arguments for type alias, expected: 0, given: 2  [type-arg]
- discord/utils.py:659: error: Invalid location for ParamSpec "P"  [valid-type]
- discord/utils.py:659: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
- discord/app_commands/tree.py:843: error: Bad number of arguments for type alias, expected: 0, given: 3  [type-arg]
- discord/app_commands/tree.py:843: error: Invalid location for ParamSpec "P"  [valid-type]
- discord/app_commands/tree.py:843: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
- discord/app_commands/tree.py:877: error: Bad number of arguments for type alias, expected: 0, given: 3  [type-arg]
- discord/app_commands/tree.py:877: error: Invalid location for ParamSpec "P"  [valid-type]
- discord/app_commands/tree.py:877: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
- discord/app_commands/tree.py:889: error: Need type annotation for "command"  [var-annotated]
- discord/app_commands/commands.py:113: error: The first argument to Callable must be a list of types, parameter specification, or "..."  [valid-type]
- discord/app_commands/commands.py:113: note: See https://mypy.readthedocs.io/en/stable/kinds_of_types.html#callable-types-and-lambdas
- discord/app_commands/commands.py:113: error: The last parameter to Concatenate needs to be a ParamSpec  [valid-type]
- discord/app_commands/commands.py:113: error: ParamSpec "P" is unbound  [valid-type]
- discord/app_commands/commands.py:114: error: The first argument to Callable must be a list of types, parameter specification, or "..."  [valid-type]
- discord/app_commands/commands.py:114: note: See https://mypy.readthedocs.io/en/stable/kinds_of_types.html#callable-types-and-lambdas
- discord/app_commands/commands.py:114: error: The last parameter to Concatenate needs to be a ParamSpec  [valid-type]
- discord/app_commands/commands.py:114: error: ParamSpec "P" is unbound  [valid-type]
- discord/app_commands/commands.py:139: error: Unexpected "..."  [misc]
- discord/app_commands/commands.py:139: error: Bad number of arguments for type alias, expected: 0, given: 3  [type-arg]
+ discord/app_commands/commands.py:680: error: Item "function" of "Union[Callable[[GroupT, Interaction, **P], Coroutine[Any, Any, T]], Callable[[Interaction, **P], Coroutine[Any, Any, T]]]" has no attribute "__self__"  [union-attr]
+ discord/app_commands/commands.py:681: error: Item "function" of "Union[Callable[[GroupT, Interaction, **P], Coroutine[Any, Any, T]], Callable[[Interaction, **P], Coroutine[Any, Any, T]]]" has no attribute "__func__"  [union-attr]
- discord/app_commands/commands.py:656: error: Bad number of arguments for type alias, expected: 0, given: 3  [type-arg]
- discord/app_commands/commands.py:656: error: Invalid location for ParamSpec "P"  [valid-type]
- discord/app_commands/commands.py:656: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
- discord/app_commands/commands.py:672: error: Bad number of arguments for type alias, expected: 0, given: 3  [type-arg]
- discord/app_commands/commands.py:672: error: Invalid location for ParamSpec "P"  [valid-type]
- discord/app_commands/commands.py:672: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
- discord/app_commands/commands.py:716: error: Bad number of arguments for type alias, expected: 0, given: 3  [type-arg]
- discord/app_commands/commands.py:716: error: Invalid location for ParamSpec "P"  [valid-type]
- discord/app_commands/commands.py:716: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
- discord/app_commands/commands.py:761: error: No overload variant of "TranslationContext" matches argument types "TranslationContextLocation", "Command[GroupT, P, T]"  [call-overload]
- discord/app_commands/commands.py:761: note: Possible overload variants:
- discord/app_commands/commands.py:761: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.command_name], data: Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu]) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:761: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.command_description], data: Command[Any, [VarArg(Any), KwArg(Any)], Any]) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:761: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.group_name, TranslationContextLocation.group_description], data: Group) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:761: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.parameter_name, TranslationContextLocation.parameter_description], data: Parameter) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:761: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.choice_name], data: Choice[Any]) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:761: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.other], data: Any) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:762: error: No overload variant of "TranslationContext" matches argument types "TranslationContextLocation", "Command[GroupT, P, T]"  [call-overload]
- discord/app_commands/commands.py:762: note: Possible overload variants:
- discord/app_commands/commands.py:762: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.command_name], data: Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu]) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:762: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.command_description], data: Command[Any, [VarArg(Any), KwArg(Any)], Any]) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:762: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.group_name, TranslationContextLocation.group_description], data: Group) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:762: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.parameter_name, TranslationContextLocation.parameter_description], data: Parameter) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:762: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.choice_name], data: Choice[Any]) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:762: note:     def [_L <: TranslationContextLocation, _D] __init__(self, location: Literal[TranslationContextLocation.other], data: Any) -> TranslationContext[_L, _D]
- discord/app_commands/commands.py:778: error: Argument 2 to "Parameter" has incompatible type "Command[GroupT, P, T]"; expected "Command[Any, [VarArg(Any), KwArg(Any)], Any]"  [arg-type]
- discord/app_commands/commands.py:850: error: Argument 1 to "CommandSignatureMismatch" has incompatible type "Command[GroupT, P, T]"; expected "Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu, Group]"  [arg-type]
- discord/app_commands/commands.py:875: error: Argument 1 to "CommandSignatureMismatch" has incompatible type "Command[GroupT, P, T]"; expected "Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu, Group]"  [arg-type]
- discord/app_commands/commands.py:876: error: Argument 1 to "CommandInvokeError" has incompatible type "Command[GroupT, P, T]"; expected "Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu]"  [arg-type]
- discord/app_commands/commands.py:880: error: Argument 1 to "CommandInvokeError" has incompatible type "Command[GroupT, P, T]"; expected "Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu]"  [arg-type]
- discord/app_commands/commands.py:902: error: Argument 1 to "CommandSignatureMismatch" has incompatible type "Command[GroupT, P, T]"; expected "Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu, Group]"  [arg-type]
- discord/app_commands/commands.py:905: error: Argument 1 to "CommandSignatureMismatch" has incompatible type "Command[GroupT, P, T]"; expected "Union[Command[Any, [VarArg(Any), KwArg(Any)], Any], ContextMenu, Group]"  [arg-type]
- discord/app_commands/commands.py:947: error: Argument 2 to "Parameter" has incompatible type "Command[GroupT, P, T]"; expected "Command[Any, [VarArg(Any), KwArg(Any)], Any]"  [arg-type]
- discord/app_commands/commands.py:968: error: Argument 2 to "Parameter" has incompatible type "Command[GroupT, P, T]"; expected "Command[Any, [VarArg(Any), KwArg(Any)], Any]"  [arg-type]
- discord/app_commands/commands.py:1007: error: Argument 2 to "maybe_coroutine" has incompatible type "Interaction"; expected <nothing>  [arg-type]
- discord/app_commands/commands.py:1013: error: Need type annotation for "ret"  [var-annotated]

... (truncated 127 lines) ...

Expression (https://github.com/cognitedata/Expression)
+ expression/core/fn.py:36: error: List or tuple expected as variadic arguments  [misc]
+ expression/core/fn.py:36: error: Argument after ** must be a mapping, not "Union[Any, _P.kwargs]"  [arg-type]
+ expression/core/fn.py:55: error: List or tuple expected as variadic arguments  [misc]
+ expression/core/fn.py:55: error: Argument after ** must be a mapping, not "Union[Any, _P.kwargs]"  [arg-type]
- expression/core/fn.py:22: error: ParamSpec "_P" is unbound  [valid-type]
- expression/core/fn.py:25: error: Bad number of arguments for type alias, expected: 1, given: 2  [type-arg]
- expression/core/fn.py:25: error: Invalid location for ParamSpec "_P"  [valid-type]
- expression/core/fn.py:25: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[_P, int]'
- expression/core/fn.py:33: error: Bad number of arguments for type alias, expected: 1, given: 2  [type-arg]
- expression/core/fn.py:33: error: Invalid location for ParamSpec "_P"  [valid-type]
- expression/core/fn.py:33: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[_P, int]'
- expression/core/fn.py:48: error: Bad number of arguments for type alias, expected: 1, given: 2  [type-arg]
- expression/core/fn.py:48: error: Invalid location for ParamSpec "_P"  [valid-type]
- expression/core/fn.py:48: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[_P, int]'
- expression/core/fn.py:52: error: Bad number of arguments for type alias, expected: 1, given: 2  [type-arg]
- expression/core/fn.py:52: error: Invalid location for ParamSpec "_P"  [valid-type]
- expression/core/fn.py:52: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[_P, int]'

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants