From 0cf8a6682a3f30b23be68f20b51441a9be6e4574 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Sat, 20 May 2023 07:13:06 +0900 Subject: [PATCH 1/4] Reconsider constraints involving parameter specifications --- mypy/constraints.py | 70 ++++++++++++++----- .../unit/check-parameter-specification.test | 30 ++++++++ 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/mypy/constraints.py b/mypy/constraints.py index 9a662f1004f7..4127884850f5 100644 --- a/mypy/constraints.py +++ b/mypy/constraints.py @@ -689,25 +689,33 @@ def visit_instance(self, template: Instance) -> list[Constraint]: ) elif isinstance(tvar, ParamSpecType) and isinstance(mapped_arg, ParamSpecType): suffix = get_proper_type(instance_arg) + prefix = mapped_arg.prefix + length = len(prefix.arg_types) if isinstance(suffix, CallableType): - prefix = mapped_arg.prefix from_concat = bool(prefix.arg_types) or suffix.from_concatenate suffix = suffix.copy_modified(from_concatenate=from_concat) if isinstance(suffix, (Parameters, CallableType)): # no such thing as variance for ParamSpecs # TODO: is there a case I am missing? - # TODO: constraints between prefixes - prefix = mapped_arg.prefix - suffix = suffix.copy_modified( - suffix.arg_types[len(prefix.arg_types) :], - suffix.arg_kinds[len(prefix.arg_kinds) :], - suffix.arg_names[len(prefix.arg_names) :], + + constrained_to = suffix.copy_modified( + suffix.arg_types[length:], + suffix.arg_kinds[length:], + suffix.arg_names[length:], ) - res.append(Constraint(mapped_arg, SUPERTYPE_OF, suffix)) + res.append(Constraint(mapped_arg, SUPERTYPE_OF, constrained_to)) elif isinstance(suffix, ParamSpecType): - res.append(Constraint(mapped_arg, SUPERTYPE_OF, suffix)) + suffix_prefix = suffix.prefix + constrained = suffix.copy_modified( + prefix=suffix_prefix.copy_modified( + suffix_prefix.arg_types[length:], + suffix_prefix.arg_kinds[length:], + suffix_prefix.arg_names[length:], + ) + ) + res.append(Constraint(mapped_arg, SUPERTYPE_OF, constrained)) else: # This case should have been handled above. assert not isinstance(tvar, TypeVarTupleType) @@ -759,6 +767,8 @@ def visit_instance(self, template: Instance) -> list[Constraint]: template_arg, ParamSpecType ): suffix = get_proper_type(mapped_arg) + prefix = template_arg.prefix + length = len(prefix.arg_types) if isinstance(suffix, CallableType): prefix = template_arg.prefix @@ -768,16 +778,22 @@ def visit_instance(self, template: Instance) -> list[Constraint]: if isinstance(suffix, (Parameters, CallableType)): # no such thing as variance for ParamSpecs # TODO: is there a case I am missing? - # TODO: constraints between prefixes - prefix = template_arg.prefix suffix = suffix.copy_modified( - suffix.arg_types[len(prefix.arg_types) :], - suffix.arg_kinds[len(prefix.arg_kinds) :], - suffix.arg_names[len(prefix.arg_names) :], + suffix.arg_types[length:], + suffix.arg_kinds[length:], + suffix.arg_names[length:], ) res.append(Constraint(template_arg, SUPERTYPE_OF, suffix)) elif isinstance(suffix, ParamSpecType): + suffix_prefix = suffix.prefix + constrained = suffix.copy_modified( + prefix=suffix_prefix.copy_modified( + suffix_prefix.arg_types[length:], + suffix_prefix.arg_kinds[length:], + suffix_prefix.arg_names[length:], + ) + ) res.append(Constraint(template_arg, SUPERTYPE_OF, suffix)) else: # This case should have been handled above. @@ -923,9 +939,19 @@ def visit_callable_type(self, template: CallableType) -> list[Constraint]: prefix_len = len(prefix.arg_types) cactual_ps = cactual.param_spec() + cactual_prefix: Parameters | CallableType + if cactual_ps: + cactual_prefix = cactual_ps.prefix + else: + cactual_prefix = cactual + + max_prefix_len = len( + [k for k in cactual_prefix.arg_kinds if k in (ARG_POS, ARG_OPT)] + ) + prefix_len = min(prefix_len, max_prefix_len) + + # we could check the prefixes match here, but that should be caught elsewhere. if not cactual_ps: - max_prefix_len = len([k for k in cactual.arg_kinds if k in (ARG_POS, ARG_OPT)]) - prefix_len = min(prefix_len, max_prefix_len) res.append( Constraint( param_spec, @@ -939,7 +965,17 @@ def visit_callable_type(self, template: CallableType) -> list[Constraint]: ) ) else: - res.append(Constraint(param_spec, SUBTYPE_OF, cactual_ps)) + # earlier, cactual_prefix = cactual_ps.prefix. thus, this is guaranteed + assert isinstance(cactual_prefix, Parameters) + + constrained_by = cactual_ps.copy_modified( + prefix=cactual_prefix.copy_modified( + cactual_prefix.arg_types[prefix_len:], + cactual_prefix.arg_kinds[prefix_len:], + cactual_prefix.arg_names[prefix_len:], + ) + ) + res.append(Constraint(param_spec, SUBTYPE_OF, constrained_by)) # compare prefixes cactual_prefix = cactual.copy_modified( diff --git a/test-data/unit/check-parameter-specification.test b/test-data/unit/check-parameter-specification.test index fe66b18fbfea..e12ce649f79d 100644 --- a/test-data/unit/check-parameter-specification.test +++ b/test-data/unit/check-parameter-specification.test @@ -1520,3 +1520,33 @@ 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 testComplicatedParamSpecReturnType] +# regression test for https://github.com/python/mypy/issues/15073 +from typing import TypeVar, Callable +from typing_extensions import ParamSpec, Concatenate + +R = TypeVar("R") +P = ParamSpec("P") + +def f( +) -> Callable[[Callable[Concatenate[Callable[P, R], P], R]], Callable[P, R]]: + def r(fn: Callable[Concatenate[Callable[P, R], P], R]) -> Callable[P, R]: ... + return r +[builtins fixtures/paramspec.pyi] + +[case testParamSpecToParamSpecAssignment] +# minimized from https://github.com/python/mypy/issues/15037 +# ~ the same as https://github.com/python/mypy/issues/15065 +from typing import Callable +from typing_extensions import Concatenate, ParamSpec + +P = ParamSpec("P") + +def f(f: Callable[Concatenate[int, P], None]) -> Callable[P, None]: ... + +x: Callable[ + [Callable[Concatenate[int, P], None]], + Callable[P, None], +] = f +[builtins fixtures/paramspec.pyi] From a979083e4891612b46a367d239c52e81133d6e4c Mon Sep 17 00:00:00 2001 From: A5rocks Date: Fri, 7 Jul 2023 13:17:31 +0900 Subject: [PATCH 2/4] Quick fix, I think --- mypy/constraints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/constraints.py b/mypy/constraints.py index cf7cc5a4dc77..6eb415c5285b 100644 --- a/mypy/constraints.py +++ b/mypy/constraints.py @@ -790,7 +790,7 @@ def visit_instance(self, template: Instance) -> list[Constraint]: suffix_prefix.arg_names[length:], ) ) - res.append(Constraint(template_arg, SUPERTYPE_OF, suffix)) + res.append(Constraint(template_arg, SUPERTYPE_OF, constrained)) else: # This case should have been handled above. assert not isinstance(tvar, TypeVarTupleType) From 2480ac61e8954368074016fab72e9d7187071b1c Mon Sep 17 00:00:00 2001 From: A5rocks Date: Fri, 7 Jul 2023 14:52:10 +0900 Subject: [PATCH 3/4] Add some tests (even if one has the wrong result) --- mypy/test/testconstraints.py | 49 +++++++++++++++++++ mypy/test/typefixture.py | 42 ++++++++++++++++ .../unit/check-parameter-specification.test | 2 +- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/mypy/test/testconstraints.py b/mypy/test/testconstraints.py index b46f31327150..8a53287b4240 100644 --- a/mypy/test/testconstraints.py +++ b/mypy/test/testconstraints.py @@ -159,3 +159,52 @@ def test_var_length_tuple_with_fixed_length_tuple(self) -> None: Instance(fx.std_tuplei, [fx.a]), SUPERTYPE_OF, ) + + def test_paramspec_constrained_with_concatenate(self) -> None: + # for legibility (and my own understanding), `Tester.normal()` is `Tester[P]` + # and `Tester.concatenate()` is `Tester[Concatenate[A, P]]` + # ... and 2nd arg to infer_constraints ends up on LHS of equality + fx = self.fx + + # equiv to: x: Tester[Q] = Tester.normal() + assert set( + infer_constraints(Instance(fx.gpsi, [fx.p]), Instance(fx.gpsi, [fx.q]), SUBTYPE_OF) + ) == {Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q)} + + # equiv to: x: Tester[Q] = Tester.concatenate() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p_concatenate]), Instance(fx.gpsi, [fx.q]), SUBTYPE_OF + ) + ) == { + # TODO: this is obviously wrong, I think? + Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q) + } + + # equiv to: x: Tester[Concatenate[B, Q]] = Tester.normal() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p]), Instance(fx.gpsi, [fx.q_concatenate]), SUBTYPE_OF + ) + ) == {Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q_concatenate)} + + # equiv to: x: Tester[Concatenate[B, Q]] = Tester.concatenate() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p_concatenate]), + Instance(fx.gpsi, [fx.q_concatenate]), + SUBTYPE_OF, + ) + ) == { + # this is correct as we assume other parts of mypy will warn that [B] != [A] + Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q) + } + + # equiv to: x: Tester[Concatenate[A, Q]] = Tester.concatenate() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p_concatenate]), + Instance(fx.gpsi, [fx.q_concatenate]), + SUBTYPE_OF, + ) + ) == {Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q)} diff --git a/mypy/test/typefixture.py b/mypy/test/typefixture.py index bf1500a3cdec..df78eeb62956 100644 --- a/mypy/test/typefixture.py +++ b/mypy/test/typefixture.py @@ -5,6 +5,8 @@ from __future__ import annotations +from typing import Sequence + from mypy.nodes import ( ARG_OPT, ARG_POS, @@ -26,6 +28,9 @@ Instance, LiteralType, NoneType, + Parameters, + ParamSpecFlavor, + ParamSpecType, Type, TypeAliasType, TypeOfAny, @@ -238,6 +243,31 @@ def make_type_var_tuple(name: str, id: int, upper_bound: Type) -> TypeVarTupleTy "GV2", mro=[self.oi], typevars=["T", "Ts", "S"], typevar_tuple_index=1 ) + def make_parameter_specification( + name: str, id: int, concatenate: Sequence[Type] + ) -> ParamSpecType: + return ParamSpecType( + name, + name, + id, + ParamSpecFlavor.BARE, + self.o, + AnyType(TypeOfAny.from_omitted_generics), + prefix=Parameters( + concatenate, [ARG_POS for _ in concatenate], [None for _ in concatenate] + ), + ) + + self.p = make_parameter_specification("P", 1, []) + self.p_concatenate = make_parameter_specification("P", 1, [self.a]) + self.q = make_parameter_specification("Q", 2, []) + self.q_concatenate = make_parameter_specification("Q", 2, [self.b]) + self.q_concatenate_a = make_parameter_specification("Q", 2, [self.a]) + + self.gpsi = self.make_type_info( + "GPS", mro=[self.oi], typevars=["P"], paramspec_indexes={0} + ) + def _add_bool_dunder(self, type_info: TypeInfo) -> None: signature = CallableType([], [], [], Instance(self.bool_type_info, []), self.function) bool_func = FuncDef("__bool__", [], Block([])) @@ -299,6 +329,7 @@ def make_type_info( bases: list[Instance] | None = None, typevars: list[str] | None = None, typevar_tuple_index: int | None = None, + paramspec_indexes: set[int] | None = None, variances: list[int] | None = None, ) -> TypeInfo: """Make a TypeInfo suitable for use in unit tests.""" @@ -326,6 +357,17 @@ def make_type_info( AnyType(TypeOfAny.from_omitted_generics), ) ) + elif paramspec_indexes is not None and id - 1 in paramspec_indexes: + v.append( + ParamSpecType( + n, + n, + id, + ParamSpecFlavor.BARE, + self.o, + AnyType(TypeOfAny.from_omitted_generics), + ) + ) else: if variances: variance = variances[id - 1] diff --git a/test-data/unit/check-parameter-specification.test b/test-data/unit/check-parameter-specification.test index b71270085494..52c1422b9a47 100644 --- a/test-data/unit/check-parameter-specification.test +++ b/test-data/unit/check-parameter-specification.test @@ -783,7 +783,7 @@ _P = ParamSpec("_P") class Job(Generic[_P]): def __init__(self, target: Callable[_P, None]) -> None: - self.target = target + ... def func( action: Union[Job[int], Callable[[int], None]], From 9d41d9ccf05ed1ad010085e24f60ba6a07951f9b Mon Sep 17 00:00:00 2001 From: A5rocks Date: Sat, 8 Jul 2023 10:54:57 +0900 Subject: [PATCH 4/4] Test and behavior fixes --- mypy/constraints.py | 63 ++++++++++++++++++++--- mypy/test/testconstraints.py | 97 ++++++++++++++++++++---------------- 2 files changed, 110 insertions(+), 50 deletions(-) diff --git a/mypy/constraints.py b/mypy/constraints.py index 6eb415c5285b..1c47b110fc13 100644 --- a/mypy/constraints.py +++ b/mypy/constraints.py @@ -79,15 +79,19 @@ def __repr__(self) -> str: op_str = "<:" if self.op == SUPERTYPE_OF: op_str = ":>" - return f"{self.type_var} {op_str} {self.target}" + return f"{self.origin_type_var} {op_str} {self.target}" def __hash__(self) -> int: - return hash((self.type_var, self.op, self.target)) + return hash((self.origin_type_var, self.op, self.target)) def __eq__(self, other: object) -> bool: if not isinstance(other, Constraint): return False - return (self.type_var, self.op, self.target) == (other.type_var, other.op, other.target) + return (self.origin_type_var, self.op, self.target) == ( + other.origin_type_var, + other.op, + other.target, + ) def infer_constraints_for_callable( @@ -695,15 +699,27 @@ def visit_instance(self, template: Instance) -> list[Constraint]: if isinstance(suffix, (Parameters, CallableType)): # no such thing as variance for ParamSpecs # TODO: is there a case I am missing? + length = min(length, len(suffix.arg_types)) constrained_to = suffix.copy_modified( suffix.arg_types[length:], suffix.arg_kinds[length:], suffix.arg_names[length:], ) - res.append(Constraint(mapped_arg, SUPERTYPE_OF, constrained_to)) + constrained_from = mapped_arg.copy_modified( + prefix=prefix.copy_modified( + prefix.arg_types[length:], + prefix.arg_kinds[length:], + prefix.arg_names[length:], + ) + ) + + res.append(Constraint(constrained_from, SUPERTYPE_OF, constrained_to)) + res.append(Constraint(constrained_from, SUBTYPE_OF, constrained_to)) elif isinstance(suffix, ParamSpecType): suffix_prefix = suffix.prefix + length = min(length, len(suffix_prefix.arg_types)) + constrained = suffix.copy_modified( prefix=suffix_prefix.copy_modified( suffix_prefix.arg_types[length:], @@ -711,7 +727,16 @@ def visit_instance(self, template: Instance) -> list[Constraint]: suffix_prefix.arg_names[length:], ) ) - res.append(Constraint(mapped_arg, SUPERTYPE_OF, constrained)) + constrained_from = mapped_arg.copy_modified( + prefix=prefix.copy_modified( + prefix.arg_types[length:], + prefix.arg_kinds[length:], + prefix.arg_names[length:], + ) + ) + + res.append(Constraint(constrained_from, SUPERTYPE_OF, constrained)) + res.append(Constraint(constrained_from, SUBTYPE_OF, constrained)) else: # This case should have been handled above. assert not isinstance(tvar, TypeVarTupleType) @@ -771,18 +796,31 @@ def visit_instance(self, template: Instance) -> list[Constraint]: from_concat = bool(prefix.arg_types) or suffix.from_concatenate suffix = suffix.copy_modified(from_concatenate=from_concat) + # TODO: this is almost a copy-paste of code above: make this into a function if isinstance(suffix, (Parameters, CallableType)): # no such thing as variance for ParamSpecs # TODO: is there a case I am missing? + length = min(length, len(suffix.arg_types)) - suffix = suffix.copy_modified( + constrained_to = suffix.copy_modified( suffix.arg_types[length:], suffix.arg_kinds[length:], suffix.arg_names[length:], ) - res.append(Constraint(template_arg, SUPERTYPE_OF, suffix)) + constrained_from = template_arg.copy_modified( + prefix=prefix.copy_modified( + prefix.arg_types[length:], + prefix.arg_kinds[length:], + prefix.arg_names[length:], + ) + ) + + res.append(Constraint(constrained_from, SUPERTYPE_OF, constrained_to)) + res.append(Constraint(constrained_from, SUBTYPE_OF, constrained_to)) elif isinstance(suffix, ParamSpecType): suffix_prefix = suffix.prefix + length = min(length, len(suffix_prefix.arg_types)) + constrained = suffix.copy_modified( prefix=suffix_prefix.copy_modified( suffix_prefix.arg_types[length:], @@ -790,7 +828,16 @@ def visit_instance(self, template: Instance) -> list[Constraint]: suffix_prefix.arg_names[length:], ) ) - res.append(Constraint(template_arg, SUPERTYPE_OF, constrained)) + constrained_from = template_arg.copy_modified( + prefix=prefix.copy_modified( + prefix.arg_types[length:], + prefix.arg_kinds[length:], + prefix.arg_names[length:], + ) + ) + + res.append(Constraint(constrained_from, SUPERTYPE_OF, constrained)) + res.append(Constraint(constrained_from, SUBTYPE_OF, constrained)) else: # This case should have been handled above. assert not isinstance(tvar, TypeVarTupleType) diff --git a/mypy/test/testconstraints.py b/mypy/test/testconstraints.py index 8a53287b4240..b177bb320ff3 100644 --- a/mypy/test/testconstraints.py +++ b/mypy/test/testconstraints.py @@ -166,45 +166,58 @@ def test_paramspec_constrained_with_concatenate(self) -> None: # ... and 2nd arg to infer_constraints ends up on LHS of equality fx = self.fx - # equiv to: x: Tester[Q] = Tester.normal() - assert set( - infer_constraints(Instance(fx.gpsi, [fx.p]), Instance(fx.gpsi, [fx.q]), SUBTYPE_OF) - ) == {Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q)} - - # equiv to: x: Tester[Q] = Tester.concatenate() - assert set( - infer_constraints( - Instance(fx.gpsi, [fx.p_concatenate]), Instance(fx.gpsi, [fx.q]), SUBTYPE_OF - ) - ) == { - # TODO: this is obviously wrong, I think? - Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q) - } - - # equiv to: x: Tester[Concatenate[B, Q]] = Tester.normal() - assert set( - infer_constraints( - Instance(fx.gpsi, [fx.p]), Instance(fx.gpsi, [fx.q_concatenate]), SUBTYPE_OF - ) - ) == {Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q_concatenate)} - - # equiv to: x: Tester[Concatenate[B, Q]] = Tester.concatenate() - assert set( - infer_constraints( - Instance(fx.gpsi, [fx.p_concatenate]), - Instance(fx.gpsi, [fx.q_concatenate]), - SUBTYPE_OF, - ) - ) == { - # this is correct as we assume other parts of mypy will warn that [B] != [A] - Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q) - } - - # equiv to: x: Tester[Concatenate[A, Q]] = Tester.concatenate() - assert set( - infer_constraints( - Instance(fx.gpsi, [fx.p_concatenate]), - Instance(fx.gpsi, [fx.q_concatenate]), - SUBTYPE_OF, - ) - ) == {Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q)} + # I don't think we can parametrize... + for direction in (SUPERTYPE_OF, SUBTYPE_OF): + print(f"direction is {direction}") + # equiv to: x: Tester[Q] = Tester.normal() + assert set( + infer_constraints(Instance(fx.gpsi, [fx.p]), Instance(fx.gpsi, [fx.q]), direction) + ) == { + Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q), + Constraint(type_var=fx.p, op=SUBTYPE_OF, target=fx.q), + } + + # equiv to: x: Tester[Q] = Tester.concatenate() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p_concatenate]), Instance(fx.gpsi, [fx.q]), direction + ) + ) == { + Constraint(type_var=fx.p_concatenate, op=SUPERTYPE_OF, target=fx.q), + Constraint(type_var=fx.p_concatenate, op=SUBTYPE_OF, target=fx.q), + } + + # equiv to: x: Tester[Concatenate[B, Q]] = Tester.normal() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p]), Instance(fx.gpsi, [fx.q_concatenate]), direction + ) + ) == { + Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q_concatenate), + Constraint(type_var=fx.p, op=SUBTYPE_OF, target=fx.q_concatenate), + } + + # equiv to: x: Tester[Concatenate[B, Q]] = Tester.concatenate() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p_concatenate]), + Instance(fx.gpsi, [fx.q_concatenate]), + direction, + ) + ) == { + # this is correct as we assume other parts of mypy will warn that [B] != [A] + Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q), + Constraint(type_var=fx.p, op=SUBTYPE_OF, target=fx.q), + } + + # equiv to: x: Tester[Concatenate[A, Q]] = Tester.concatenate() + assert set( + infer_constraints( + Instance(fx.gpsi, [fx.p_concatenate]), + Instance(fx.gpsi, [fx.q_concatenate]), + direction, + ) + ) == { + Constraint(type_var=fx.p, op=SUPERTYPE_OF, target=fx.q), + Constraint(type_var=fx.p, op=SUBTYPE_OF, target=fx.q), + }