Skip to content

Commit

Permalink
Clarify variance convention for Parameters (#16302)
Browse files Browse the repository at this point in the history
Fixes #16296

In my big refactoring I flipped the variance convention for the
`Parameters` type, but I did it inconsistently in one place. After
working some more with ParamSpecs, it now seems to me the original
convention is easier to remember. I also now explicitly put it in the
type docstring.
  • Loading branch information
ilevkivskyi authored Oct 23, 2023
1 parent 341929b commit cda163d
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 17 deletions.
9 changes: 3 additions & 6 deletions mypy/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,11 +692,8 @@ def visit_parameters(self, template: Parameters) -> list[Constraint]:
return self.infer_against_any(template.arg_types, self.actual)
if type_state.infer_polymorphic and isinstance(self.actual, Parameters):
# For polymorphic inference we need to be able to infer secondary constraints
# in situations like [x: T] <: P <: [x: int]. Note we invert direction, since
# this function expects direction between callables.
return infer_callable_arguments_constraints(
template, self.actual, neg_op(self.direction)
)
# in situations like [x: T] <: P <: [x: int].
return infer_callable_arguments_constraints(template, self.actual, self.direction)
raise RuntimeError("Parameters cannot be constrained to")

# Non-leaf types
Expand Down Expand Up @@ -1128,7 +1125,7 @@ def visit_callable_type(self, template: CallableType) -> list[Constraint]:
)
)
if param_spec_target is not None:
res.append(Constraint(param_spec, neg_op(self.direction), param_spec_target))
res.append(Constraint(param_spec, self.direction, param_spec_target))
if extra_tvars:
for c in res:
c.extra_tvars += cactual.variables
Expand Down
13 changes: 9 additions & 4 deletions mypy/join.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,13 @@ def visit_parameters(self, t: Parameters) -> ProperType:
if isinstance(self.s, Parameters):
if len(t.arg_types) != len(self.s.arg_types):
return self.default(self.s)
from mypy.meet import meet_types

return t.copy_modified(
# Note that since during constraint inference we already treat whole ParamSpec as
# contravariant, we should join individual items, not meet them like for Callables
arg_types=[join_types(s_a, t_a) for s_a, t_a in zip(self.s.arg_types, t.arg_types)]
arg_types=[
meet_types(s_a, t_a) for s_a, t_a in zip(self.s.arg_types, t.arg_types)
],
arg_names=combine_arg_names(self.s, t),
)
else:
return self.default(self.s)
Expand Down Expand Up @@ -754,7 +757,9 @@ def combine_similar_callables(t: CallableType, s: CallableType) -> CallableType:
)


def combine_arg_names(t: CallableType, s: CallableType) -> list[str | None]:
def combine_arg_names(
t: CallableType | Parameters, s: CallableType | Parameters
) -> list[str | None]:
"""Produces a list of argument names compatible with both callables.
For example, suppose 't' and 's' have the following signatures:
Expand Down
6 changes: 3 additions & 3 deletions mypy/meet.py
Original file line number Diff line number Diff line change
Expand Up @@ -708,10 +708,10 @@ def visit_parameters(self, t: Parameters) -> ProperType:
if isinstance(self.s, Parameters):
if len(t.arg_types) != len(self.s.arg_types):
return self.default(self.s)
from mypy.join import join_types

return t.copy_modified(
# Note that since during constraint inference we already treat whole ParamSpec as
# contravariant, we should meet individual items, not join them like for Callables
arg_types=[meet_types(s_a, t_a) for s_a, t_a in zip(self.s.arg_types, t.arg_types)]
arg_types=[join_types(s_a, t_a) for s_a, t_a in zip(self.s.arg_types, t.arg_types)]
)
else:
return self.default(self.s)
Expand Down
2 changes: 0 additions & 2 deletions mypy/subtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,6 @@ def visit_unpack_type(self, left: UnpackType) -> bool:

def visit_parameters(self, left: Parameters) -> bool:
if isinstance(self.right, Parameters):
# TODO: direction here should be opposite, this function expects
# order of callables, while parameters are contravariant.
return are_parameters_compatible(
left,
self.right,
Expand Down
5 changes: 4 additions & 1 deletion mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,10 @@ class FormalArgument(NamedTuple):
class Parameters(ProperType):
"""Type that represents the parameters to a function.
Used for ParamSpec analysis."""
Used for ParamSpec analysis. Note that by convention we handle this
type as a Callable without return type, not as a "tuple with names",
so that it behaves contravariantly, in particular [x: int] <: [int].
"""

__slots__ = (
"arg_types",
Expand Down
29 changes: 28 additions & 1 deletion test-data/unit/check-parameter-specification.test
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,7 @@ def wrong_name_constructor(b: bool) -> SomeClass:
func(SomeClass, constructor)
reveal_type(func(SomeClass, wrong_constructor)) # N: Revealed type is "def (a: Never) -> __main__.SomeClass"
reveal_type(func_regular(SomeClass, wrong_constructor)) # N: Revealed type is "def (Never) -> __main__.SomeClass"
func(SomeClass, wrong_name_constructor) # E: Argument 1 to "func" has incompatible type "Type[SomeClass]"; expected "Callable[[Never], SomeClass]"
reveal_type(func(SomeClass, wrong_name_constructor)) # N: Revealed type is "def (Never) -> __main__.SomeClass"
[builtins fixtures/paramspec.pyi]

[case testParamSpecInTypeAliasBasic]
Expand Down Expand Up @@ -2059,3 +2059,30 @@ def test2(x: int, y: int) -> str: ...
reveal_type(call(test1, 1)) # N: Revealed type is "builtins.str"
reveal_type(call(test2, 1, 2)) # N: Revealed type is "builtins.str"
[builtins fixtures/paramspec.pyi]

[case testParamSpecCorrectParameterNameInference]
from typing import Callable, Protocol
from typing_extensions import ParamSpec, Concatenate

def a(i: int) -> None: ...
def b(__i: int) -> None: ...

class WithName(Protocol):
def __call__(self, i: int) -> None: ...
NoName = Callable[[int], None]

def f1(__fn: WithName, i: int) -> None: ...
def f2(__fn: NoName, i: int) -> None: ...

P = ParamSpec("P")
def d(f: Callable[P, None], fn: Callable[Concatenate[Callable[P, None], P], None]) -> Callable[P, None]:
def inner(*args: P.args, **kwargs: P.kwargs) -> None:
fn(f, *args, **kwargs)
return inner

reveal_type(d(a, f1)) # N: Revealed type is "def (i: builtins.int)"
reveal_type(d(a, f2)) # N: Revealed type is "def (i: builtins.int)"
reveal_type(d(b, f1)) # E: Cannot infer type argument 1 of "d" \
# N: Revealed type is "def (*Any, **Any)"
reveal_type(d(b, f2)) # N: Revealed type is "def (builtins.int)"
[builtins fixtures/paramspec.pyi]

0 comments on commit cda163d

Please sign in to comment.