Skip to content

Make sure to keep type variables even if unused. #15248

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

Closed
wants to merge 1 commit into from
Closed
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
26 changes: 25 additions & 1 deletion mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,7 @@ def check_callable_call(
need_refresh = any(
isinstance(v, (ParamSpecType, TypeVarTupleType)) for v in callee.variables
)
old_callee = callee
callee = freshen_function_type_vars(callee)
callee = self.infer_function_type_arguments_using_context(callee, context)
if need_refresh:
Expand All @@ -1440,7 +1441,7 @@ def check_callable_call(
lambda i: self.accept(args[i]),
)
callee = self.infer_function_type_arguments(
callee, args, arg_kinds, formal_to_actual, context
callee, args, arg_kinds, formal_to_actual, context, old_callee
)
if need_refresh:
formal_to_actual = map_actuals_to_formals(
Expand Down Expand Up @@ -1730,6 +1731,7 @@ def infer_function_type_arguments(
arg_kinds: list[ArgKind],
formal_to_actual: list[list[int]],
context: Context,
unfreshened_callee_type: CallableType,
) -> CallableType:
"""Infer the type arguments for a generic callee type.

Expand Down Expand Up @@ -1773,6 +1775,28 @@ def infer_function_type_arguments(
callee_type, args, arg_kinds, formal_to_actual, inferred_args, context
)

return_type = get_proper_type(callee_type.ret_type)
if isinstance(return_type, CallableType):
# fixup:
# def [T] () -> def (T) -> T
# into
# def () -> def [T] (T) -> T
for i, argument in enumerate(inferred_args):
if isinstance(get_proper_type(argument), UninhabitedType):
# un-"freshen" the type variable :^)
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this. A type def [T] () -> <anything with T> doesn't make sense, and we should figure out why it was inferred/constructed in the first place. This is really just sweeping dust under the carpet.

Copy link
Collaborator Author

@A5rocks A5rocks May 19, 2023

Choose a reason for hiding this comment

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

I can try to explain why this happens:

Have a function that takes a Callable[Concatenate[T, P], ...] and then returns a Callable[P, Callable[[T], ...]]. This could be some utility function that curries (in reverse), for example.

Now, the problem is that if you use this on def [T] (T, int) -> ... then this will turn into... def [T] (int) -> (T) -> .... At least, as currently implemented. Thus, I see two solutions:

  • Fix the new function at call time (what I do here) when we have all this information
  • Fix the function at construction time, which I'm not sure how to do: I'm not quite sure how to scan through the parameters to find if type variables are still used. For example, this should still work: def [T] (T, T) -> ... into def [T] (T) -> T -> ....

Sorry if this is a confusing explanation!

Copy link
Collaborator Author

@A5rocks A5rocks May 19, 2023

Choose a reason for hiding this comment

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

Ah just to be clear, the problem with def [T] (int) -> (T) -> ... is we'll infer an UninhabitedType for T, meaning that we can't actually trust the return type anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. I am actually working on a (tangentially related) PR that will likely fix this (or at least will allow a more principled solution). I will post a link here when it is ready.

variable = unfreshened_callee_type.variables[i]
inferred_args[i] = variable

# handle multiple type variables
return_type = return_type.copy_modified(
variables=[*return_type.variables, variable]
)

callee_type = callee_type.copy_modified(
# Question: am I allowed to assign the get_proper_type'd thing?
ret_type=return_type
)

if (
callee_type.special_sig == "dict"
and len(inferred_args) == 2
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-inference.test
Original file line number Diff line number Diff line change
Expand Up @@ -3105,7 +3105,8 @@ T = TypeVar('T')

def f(x: Optional[T] = None) -> Callable[..., T]: ...

x = f() # E: Need type annotation for "x"
# Question: Is this alright?
Copy link
Member

Choose a reason for hiding this comment

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

No, this test specifically tests the error will be shown.

x: Callable[..., T] = f()
y = x

[case testDontNeedAnnotationForCallable]
Expand Down
25 changes: 25 additions & 0 deletions test-data/unit/check-parameter-specification.test
Original file line number Diff line number Diff line change
Expand Up @@ -1520,3 +1520,28 @@ def identity(func: Callable[P, None]) -> Callable[P, None]: ...
@identity
def f(f: Callable[P, None], *args: P.args, **kwargs: P.kwargs) -> None: ...
[builtins fixtures/paramspec.pyi]

[case testParamSpecificationSurvivesCall]
# https://github.com/python/mypy/issues/12595
from typing import TypeVar, Callable
from typing_extensions import Concatenate, ParamSpec

T = TypeVar('T')
U = TypeVar('U')
P = ParamSpec('P')

def t(f: Callable[P, T]) -> Callable[P, T]: ...

def pop_off(
transform: Callable[Concatenate[T, P], U]
) -> Callable[P, Callable[[T], U]]: ...

u = pop_off(t)
Copy link
Member

Choose a reason for hiding this comment

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

TBH I don't think this situation is what is going on in the issue. IIUC you tried to simplify the example, but it seems to me you may have simplified too much.

reveal_type(u) # N: Revealed type is "def [P, T] () -> def (def (*P.args, **P.kwargs) -> T`-2) -> def (*P.args, **P.kwargs) -> T`-2"

@u()
def f(x: int) -> None: ...

reveal_type(f) # N: Revealed type is "def (x: builtins.int)"
f(0)
[builtins fixtures/paramspec.pyi]