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

Don't duplicate ParamSpec prefixes and properly substitute Paramspecs #14677

Merged
merged 5 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion mypy/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ def visit_callable_type(self, template: CallableType) -> list[Constraint]:
)

# TODO: see above "FIX" comments for param_spec is None case
# TODO: this assume positional arguments
# TODO: this assumes positional arguments
for t, a in zip(prefix.arg_types, cactual_prefix.arg_types):
res.extend(infer_constraints(t, a, neg_op(self.direction)))

Expand Down
24 changes: 22 additions & 2 deletions mypy/expandtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,10 @@ def visit_type_var(self, t: TypeVarType) -> Type:
return repl

def visit_param_spec(self, t: ParamSpecType) -> Type:
repl = get_proper_type(self.variables.get(t.id, t))
# set prefix to something empty so we don't duplicate it
repl = get_proper_type(
self.variables.get(t.id, t.copy_modified(prefix=Parameters([], [], [])))
)
if isinstance(repl, Instance):
# TODO: what does prefix mean in this case?
# TODO: why does this case even happen? Instances aren't plural.
Expand Down Expand Up @@ -357,7 +360,7 @@ def visit_callable_type(self, t: CallableType) -> Type:
# must expand both of them with all the argument types,
# kinds and names in the replacement. The return type in
# the replacement is ignored.
if isinstance(repl, CallableType) or isinstance(repl, Parameters):
if isinstance(repl, (CallableType, Parameters)):
# Substitute *args: P.args, **kwargs: P.kwargs
prefix = param_spec.prefix
# we need to expand the types in the prefix, so might as well
Expand All @@ -370,6 +373,23 @@ def visit_callable_type(self, t: CallableType) -> Type:
ret_type=t.ret_type.accept(self),
type_guard=(t.type_guard.accept(self) if t.type_guard is not None else None),
)
# TODO: it seems this only has to be done *sometimes*. Conceptually this should only
# be done once; we should update that "once" location rather than here.
# (see testAlreadyExpandedCallableWithParamSpecReplacement)
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
elif isinstance(repl, ParamSpecType) and len(t.arg_types) == 2:
# We're substituting one paramspec for another; this can mean that the prefix
# changes. (e.g. sub Concatenate[int, P] for Q)
prefix = repl.prefix
old_prefix = param_spec.prefix

# Check assumptions. I'm not sure what order to switch these:
assert not old_prefix.arg_types or not prefix.arg_types
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks a bit suspicious to me. Why can't both the arg types truthy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't found a case where they were both truthy and yet not equal to each other yet -- I don't know what order to mix their arguments together. My gut says what I wrote next is correct but I would rather have an explicit test case (or be shown that this never happens)


t = t.copy_modified(
arg_types=prefix.arg_types + old_prefix.arg_types + t.arg_types,
arg_kinds=prefix.arg_kinds + old_prefix.arg_kinds + t.arg_kinds,
arg_names=prefix.arg_names + old_prefix.arg_names + t.arg_names,
)

var_arg = t.var_arg()
if var_arg is not None and isinstance(var_arg.typ, UnpackType):
Expand Down
3 changes: 1 addition & 2 deletions mypy/subtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,7 @@ def _is_subtype(self, left: Type, right: Type) -> bool:
return is_proper_subtype(left, right, subtype_context=self.subtype_context)
return is_subtype(left, right, subtype_context=self.subtype_context)

# visit_x(left) means: is left (which is an instance of X) a subtype of
# right?
# visit_x(left) means: is left (which is an instance of X) a subtype of right?

def visit_unbound_type(self, left: UnboundType) -> bool:
# This can be called if there is a bad type annotation. The result probably
Expand Down
17 changes: 7 additions & 10 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class TypeOfAny:
# Does this Any come from an error?
from_error: Final = 5
# Is this a type that can't be represented in mypy's type system? For instance, type of
# call to NewType...). Even though these types aren't real Anys, we treat them as such.
# call to NewType(...). Even though these types aren't real Anys, we treat them as such.
# Also used for variables named '_'.
special_form: Final = 6
# Does this Any come from interaction with another Any?
Expand Down Expand Up @@ -1970,22 +1970,19 @@ def param_spec(self) -> ParamSpecType | None:
if self.arg_kinds[-2] != ARG_STAR or self.arg_kinds[-1] != ARG_STAR2:
return None
arg_type = self.arg_types[-2]
if not isinstance(arg_type, ParamSpecType):
if not isinstance(arg_type, ParamSpecType) or not isinstance(
self.arg_types[-1], ParamSpecType
):
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
return None

# sometimes paramspectypes are analyzed in from mysterious places,
# e.g. def f(prefix..., *args: P.args, **kwargs: P.kwargs) -> ...: ...
prefix = arg_type.prefix
if not prefix.arg_types:
# TODO: confirm that all arg kinds are positional
prefix = Parameters(self.arg_types[:-2], self.arg_kinds[:-2], self.arg_names[:-2])
return ParamSpecType(
arg_type.name,
arg_type.fullname,
arg_type.id,
ParamSpecFlavor.BARE,
arg_type.upper_bound,
prefix=prefix,
)

return arg_type.copy_modified(flavor=ParamSpecFlavor.BARE, prefix=prefix)

def expand_param_spec(
self, c: CallableType | Parameters, no_prefix: bool = False
Expand Down
37 changes: 37 additions & 0 deletions test-data/unit/check-parameter-specification.test
Original file line number Diff line number Diff line change
Expand Up @@ -1471,3 +1471,40 @@ def test(f: Concat[T, ...]) -> None: ...

class Defer: ...
[builtins fixtures/paramspec.pyi]

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

P = ParamSpec("P")
Q = ParamSpec("Q")

def foo(f: Callable[P, int]) -> Callable[P, int]:
return f

def bar(f: Callable[Concatenate[str, Q], int]) -> Callable[Concatenate[str, Q], int]:
return foo(f)
[builtins fixtures/paramspec.pyi]

[case testAlreadyExpandedCallableWithParamSpecReplacement]
from typing import Callable, Any, overload
from typing_extensions import Concatenate, ParamSpec

P = ParamSpec("P")

@overload
def command() -> Callable[[Callable[Concatenate[object, object, P], object]], None]: # E: Overloaded function signatures 1 and 2 overlap with incompatible return types
...

@overload
def command(
cls: int = ...,
) -> Callable[[Callable[Concatenate[object, P], object]], None]:
...

def command(
cls: int = 42,
) -> Any:
...
[builtins fixtures/paramspec.pyi]