From 0f8c36cfb44b554f4fc2d01b7e293bd828a53941 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Thu, 2 Feb 2023 14:29:32 +0900 Subject: [PATCH 1/5] Don't duplicate ParamSpec prefixes and check Concatenate subtyping better I ran into the duplication issue personally, but this also fixes https://github.com/python/mypy/issues/12734 --- mypy/constraints.py | 2 +- mypy/expandtype.py | 10 +++- mypy/messages.py | 10 ++-- mypy/subtypes.py | 49 ++++++++++++++++++- mypy/types.py | 28 +++++++---- .../unit/check-parameter-specification.test | 15 ++++++ 6 files changed, 96 insertions(+), 18 deletions(-) diff --git a/mypy/constraints.py b/mypy/constraints.py index a8f04094ca63..c8c3c7933b6e 100644 --- a/mypy/constraints.py +++ b/mypy/constraints.py @@ -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))) diff --git a/mypy/expandtype.py b/mypy/expandtype.py index 7933283b24d6..5d92c474f874 100644 --- a/mypy/expandtype.py +++ b/mypy/expandtype.py @@ -234,7 +234,15 @@ 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)) + repl_ = self.variables.get(t.id) + if not repl_: + # in this case, we are trying to expand it into... nothing? + # if we continue, we will get e.g.: + # repl = t = [int, str, **P] + # at which point, expand_param_spec will duplicate the arguments. + return t + + repl = get_proper_type(repl_) if isinstance(repl, Instance): # TODO: what does prefix mean in this case? # TODO: why does this case even happen? Instances aren't plural. diff --git a/mypy/messages.py b/mypy/messages.py index a5fd09493456..41adc63530fb 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -2399,13 +2399,15 @@ def format_literal_value(typ: LiteralType) -> str: return_type = format(func.ret_type) if func.is_ellipsis_args: return f"Callable[..., {return_type}]" + param_spec = func.param_spec() if param_spec is not None: return f"Callable[{format(param_spec)}, {return_type}]" - args = format_callable_args( - func.arg_types, func.arg_kinds, func.arg_names, format, verbosity - ) - return f"Callable[[{args}], {return_type}]" + else: + args = format_callable_args( + func.arg_types, func.arg_kinds, func.arg_names, format, verbosity + ) + return f"Callable[[{args}], {return_type}]" else: # Use a simple representation for function types; proper # function types may result in long and difficult-to-read diff --git a/mypy/subtypes.py b/mypy/subtypes.py index 9b555480e59b..fc919f498cab 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -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 @@ -643,6 +642,9 @@ def visit_param_spec(self, left: ParamSpecType) -> bool: isinstance(right, ParamSpecType) and right.id == left.id and right.flavor == left.flavor + and is_subtype( + left.as_parameters(), right.as_parameters(), subtype_context=self.subtype_context + ) ): return True if isinstance(right, Parameters) and are_trivial_parameters(right): @@ -665,11 +667,22 @@ def visit_parameters(self, left: Parameters) -> bool: right = self.right if isinstance(right, CallableType): right = right.with_unpacked_kwargs() + # TODO: This heuristic sucks. We need it to make sure we normally do the strict thing. + # I wish we could just always do the strict thing.... + # This takes into advantage that these Parameters probably just came from ParamSpecType#to_parameters() + anyt = AnyType(TypeOfAny.implementation_artifact) + concat_heuristic = all( + t.arg_kinds[-2:] == [ARG_STAR, ARG_STAR2] and t.arg_names[-2:] == [None, None] and t.arg_types[-2:] == [anyt, anyt] for t in (left, right) + ) return are_parameters_compatible( left, right, is_compat=self._is_subtype, + subtype_context=self.subtype_context, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, + strict_concatenate_check=self.options.strict_concatenate + if self.options and concat_heuristic + else True, ) else: return False @@ -688,6 +701,7 @@ def visit_callable_type(self, left: CallableType) -> bool: left, right, is_compat=self._is_subtype, + subtype_context=self.subtype_context, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, strict_concatenate=self.options.strict_concatenate if self.options else True, ) @@ -722,6 +736,7 @@ def visit_callable_type(self, left: CallableType) -> bool: left.with_unpacked_kwargs(), right, is_compat=self._is_subtype, + subtype_context=self.subtype_context, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, ) else: @@ -857,6 +872,7 @@ def visit_overloaded(self, left: Overloaded) -> bool: left_item, right_item, is_compat=self._is_subtype, + subtype_context=self.subtype_context, ignore_return=True, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, strict_concatenate=strict_concat, @@ -865,6 +881,7 @@ def visit_overloaded(self, left: Overloaded) -> bool: right_item, left_item, is_compat=self._is_subtype, + subtype_context=self.subtype_context, ignore_return=True, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, strict_concatenate=strict_concat, @@ -1299,6 +1316,8 @@ def is_callable_compatible( right: CallableType, *, is_compat: Callable[[Type, Type], bool], + # TODO: should this be used more (instead of `ignore_pos_arg_names`, for instance)? + subtype_context: SubtypeContext | None = None, is_compat_return: Callable[[Type, Type], bool] | None = None, ignore_return: bool = False, ignore_pos_arg_names: bool = False, @@ -1453,6 +1472,7 @@ def g(x: int) -> int: ... left, right, is_compat=is_compat, + subtype_context=subtype_context, ignore_pos_arg_names=ignore_pos_arg_names, check_args_covariantly=check_args_covariantly, allow_partial_overlap=allow_partial_overlap, @@ -1472,11 +1492,23 @@ def are_trivial_parameters(param: Parameters | NormalizedCallableType) -> bool: ) +def are_seperated_paramspecs(param: Parameters | NormalizedCallableType) -> bool: + # TODO: based on my tests, param.arg_types will always be of length 2. This is a potential + # optimization, however Callable#param_spec() does not assume this. If this is done, update that too. + return ( + len(param.arg_types) >= 2 + and isinstance(param.arg_types[-2], ParamSpecType) + and isinstance(param.arg_types[-1], ParamSpecType) + ) + + def are_parameters_compatible( left: Parameters | NormalizedCallableType, right: Parameters | NormalizedCallableType, *, is_compat: Callable[[Type, Type], bool], + # TODO: should this be used more (instead of `ignore_pos_arg_names`, for instance)? + subtype_context: SubtypeContext | None = None, ignore_pos_arg_names: bool = False, check_args_covariantly: bool = False, allow_partial_overlap: bool = False, @@ -1495,6 +1527,19 @@ def are_parameters_compatible( if are_trivial_parameters(right): return True + # sometimes we need to compare Callable[Concatenate[prefix, P], ...] and + # (prefix, *args: P.args, **kwargs: P.kwargs) -> ... + if (isinstance(left, CallableType) and isinstance(right, CallableType)) and ( + are_seperated_paramspecs(left) or are_seperated_paramspecs(right) + ): + # note: this doesn't lose parameters. I think. Otherwise, it would be unsafe. + left_ps = left.param_spec() + right_ps = right.param_spec() + if not right_ps or not left_ps: + return False + + return is_subtype(left_ps, right_ps, subtype_context=subtype_context) + # Match up corresponding arguments and check them for compatibility. In # every pair (argL, argR) of corresponding arguments from L and R, argL must # be "more general" than argR if L is to be a subtype of R. diff --git a/mypy/types.py b/mypy/types.py index 90d33839c693..7fe31c8280e2 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -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? @@ -715,6 +715,17 @@ def name_with_suffix(self) -> str: return f"{n}.kwargs" return n + def as_parameters(self) -> Parameters: + return self.prefix.copy_modified( + arg_types=self.prefix.arg_types + + [ + AnyType(TypeOfAny.implementation_artifact), + AnyType(TypeOfAny.implementation_artifact), + ], + arg_names=self.prefix.arg_names + [None, None], + arg_kinds=self.prefix.arg_kinds + [ARG_STAR, ARG_STAR2], + ) + def __hash__(self) -> int: return hash((self.id, self.flavor, self.prefix)) @@ -1970,22 +1981,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 + ): 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 diff --git a/test-data/unit/check-parameter-specification.test b/test-data/unit/check-parameter-specification.test index 56fc3b6faa14..3678ab770046 100644 --- a/test-data/unit/check-parameter-specification.test +++ b/test-data/unit/check-parameter-specification.test @@ -1471,3 +1471,18 @@ 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] From 23b3ffbb7528e42b9b58f3b50bf9f75fc4740d98 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Sun, 12 Feb 2023 08:09:57 +0900 Subject: [PATCH 2/5] Try a more minimal approach --- mypy/expandtype.py | 33 +++++++++++++++++++++++---------- mypy/messages.py | 10 ++++------ mypy/subtypes.py | 46 ---------------------------------------------- mypy/types.py | 11 ----------- 4 files changed, 27 insertions(+), 73 deletions(-) diff --git a/mypy/expandtype.py b/mypy/expandtype.py index 5d92c474f874..ca97af6e6ac0 100644 --- a/mypy/expandtype.py +++ b/mypy/expandtype.py @@ -234,15 +234,10 @@ def visit_type_var(self, t: TypeVarType) -> Type: return repl def visit_param_spec(self, t: ParamSpecType) -> Type: - repl_ = self.variables.get(t.id) - if not repl_: - # in this case, we are trying to expand it into... nothing? - # if we continue, we will get e.g.: - # repl = t = [int, str, **P] - # at which point, expand_param_spec will duplicate the arguments. - return t - - repl = get_proper_type(repl_) + # 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. @@ -365,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 @@ -378,6 +373,24 @@ 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: fix testConstraintsBetweenConcatenatePrefixes + # (it fails without `and repl != param_spec`) + elif isinstance(repl, ParamSpecType) and repl != param_spec: + # 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 + # ... and I don't know where to put non-paramspec t.arg_types + assert len(t.arg_types) == 2 or t.arg_types[:-2] == old_prefix.arg_types + + t = t.copy_modified( + arg_types=prefix.arg_types + old_prefix.arg_types + t.arg_types[-2:], + arg_kinds=prefix.arg_kinds + old_prefix.arg_kinds + t.arg_kinds[-2:], + arg_names=prefix.arg_names + old_prefix.arg_names + t.arg_names[-2:], + ) var_arg = t.var_arg() if var_arg is not None and isinstance(var_arg.typ, UnpackType): diff --git a/mypy/messages.py b/mypy/messages.py index 41adc63530fb..a5fd09493456 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -2399,15 +2399,13 @@ def format_literal_value(typ: LiteralType) -> str: return_type = format(func.ret_type) if func.is_ellipsis_args: return f"Callable[..., {return_type}]" - param_spec = func.param_spec() if param_spec is not None: return f"Callable[{format(param_spec)}, {return_type}]" - else: - args = format_callable_args( - func.arg_types, func.arg_kinds, func.arg_names, format, verbosity - ) - return f"Callable[[{args}], {return_type}]" + args = format_callable_args( + func.arg_types, func.arg_kinds, func.arg_names, format, verbosity + ) + return f"Callable[[{args}], {return_type}]" else: # Use a simple representation for function types; proper # function types may result in long and difficult-to-read diff --git a/mypy/subtypes.py b/mypy/subtypes.py index fc919f498cab..41c78965b348 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -642,9 +642,6 @@ def visit_param_spec(self, left: ParamSpecType) -> bool: isinstance(right, ParamSpecType) and right.id == left.id and right.flavor == left.flavor - and is_subtype( - left.as_parameters(), right.as_parameters(), subtype_context=self.subtype_context - ) ): return True if isinstance(right, Parameters) and are_trivial_parameters(right): @@ -667,22 +664,11 @@ def visit_parameters(self, left: Parameters) -> bool: right = self.right if isinstance(right, CallableType): right = right.with_unpacked_kwargs() - # TODO: This heuristic sucks. We need it to make sure we normally do the strict thing. - # I wish we could just always do the strict thing.... - # This takes into advantage that these Parameters probably just came from ParamSpecType#to_parameters() - anyt = AnyType(TypeOfAny.implementation_artifact) - concat_heuristic = all( - t.arg_kinds[-2:] == [ARG_STAR, ARG_STAR2] and t.arg_names[-2:] == [None, None] and t.arg_types[-2:] == [anyt, anyt] for t in (left, right) - ) return are_parameters_compatible( left, right, is_compat=self._is_subtype, - subtype_context=self.subtype_context, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, - strict_concatenate_check=self.options.strict_concatenate - if self.options and concat_heuristic - else True, ) else: return False @@ -701,7 +687,6 @@ def visit_callable_type(self, left: CallableType) -> bool: left, right, is_compat=self._is_subtype, - subtype_context=self.subtype_context, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, strict_concatenate=self.options.strict_concatenate if self.options else True, ) @@ -736,7 +721,6 @@ def visit_callable_type(self, left: CallableType) -> bool: left.with_unpacked_kwargs(), right, is_compat=self._is_subtype, - subtype_context=self.subtype_context, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, ) else: @@ -872,7 +856,6 @@ def visit_overloaded(self, left: Overloaded) -> bool: left_item, right_item, is_compat=self._is_subtype, - subtype_context=self.subtype_context, ignore_return=True, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, strict_concatenate=strict_concat, @@ -881,7 +864,6 @@ def visit_overloaded(self, left: Overloaded) -> bool: right_item, left_item, is_compat=self._is_subtype, - subtype_context=self.subtype_context, ignore_return=True, ignore_pos_arg_names=self.subtype_context.ignore_pos_arg_names, strict_concatenate=strict_concat, @@ -1316,8 +1298,6 @@ def is_callable_compatible( right: CallableType, *, is_compat: Callable[[Type, Type], bool], - # TODO: should this be used more (instead of `ignore_pos_arg_names`, for instance)? - subtype_context: SubtypeContext | None = None, is_compat_return: Callable[[Type, Type], bool] | None = None, ignore_return: bool = False, ignore_pos_arg_names: bool = False, @@ -1472,7 +1452,6 @@ def g(x: int) -> int: ... left, right, is_compat=is_compat, - subtype_context=subtype_context, ignore_pos_arg_names=ignore_pos_arg_names, check_args_covariantly=check_args_covariantly, allow_partial_overlap=allow_partial_overlap, @@ -1492,23 +1471,11 @@ def are_trivial_parameters(param: Parameters | NormalizedCallableType) -> bool: ) -def are_seperated_paramspecs(param: Parameters | NormalizedCallableType) -> bool: - # TODO: based on my tests, param.arg_types will always be of length 2. This is a potential - # optimization, however Callable#param_spec() does not assume this. If this is done, update that too. - return ( - len(param.arg_types) >= 2 - and isinstance(param.arg_types[-2], ParamSpecType) - and isinstance(param.arg_types[-1], ParamSpecType) - ) - - def are_parameters_compatible( left: Parameters | NormalizedCallableType, right: Parameters | NormalizedCallableType, *, is_compat: Callable[[Type, Type], bool], - # TODO: should this be used more (instead of `ignore_pos_arg_names`, for instance)? - subtype_context: SubtypeContext | None = None, ignore_pos_arg_names: bool = False, check_args_covariantly: bool = False, allow_partial_overlap: bool = False, @@ -1527,19 +1494,6 @@ def are_parameters_compatible( if are_trivial_parameters(right): return True - # sometimes we need to compare Callable[Concatenate[prefix, P], ...] and - # (prefix, *args: P.args, **kwargs: P.kwargs) -> ... - if (isinstance(left, CallableType) and isinstance(right, CallableType)) and ( - are_seperated_paramspecs(left) or are_seperated_paramspecs(right) - ): - # note: this doesn't lose parameters. I think. Otherwise, it would be unsafe. - left_ps = left.param_spec() - right_ps = right.param_spec() - if not right_ps or not left_ps: - return False - - return is_subtype(left_ps, right_ps, subtype_context=subtype_context) - # Match up corresponding arguments and check them for compatibility. In # every pair (argL, argR) of corresponding arguments from L and R, argL must # be "more general" than argR if L is to be a subtype of R. diff --git a/mypy/types.py b/mypy/types.py index 7fe31c8280e2..783eb28a2912 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -715,17 +715,6 @@ def name_with_suffix(self) -> str: return f"{n}.kwargs" return n - def as_parameters(self) -> Parameters: - return self.prefix.copy_modified( - arg_types=self.prefix.arg_types - + [ - AnyType(TypeOfAny.implementation_artifact), - AnyType(TypeOfAny.implementation_artifact), - ], - arg_names=self.prefix.arg_names + [None, None], - arg_kinds=self.prefix.arg_kinds + [ARG_STAR, ARG_STAR2], - ) - def __hash__(self) -> int: return hash((self.id, self.flavor, self.prefix)) From 2d141f1bf760ef79672ece06ac522f93dc3df2fb Mon Sep 17 00:00:00 2001 From: A5rocks Date: Sun, 12 Feb 2023 09:01:12 +0900 Subject: [PATCH 3/5] Fix crash in discord.py --- mypy/expandtype.py | 15 ++++++------- .../unit/check-parameter-specification.test | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/mypy/expandtype.py b/mypy/expandtype.py index ca97af6e6ac0..df375ac1d27c 100644 --- a/mypy/expandtype.py +++ b/mypy/expandtype.py @@ -373,9 +373,10 @@ 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: fix testConstraintsBetweenConcatenatePrefixes - # (it fails without `and repl != param_spec`) - elif isinstance(repl, ParamSpecType) and repl != param_spec: + # 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) + 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 @@ -383,13 +384,11 @@ def visit_callable_type(self, t: CallableType) -> Type: # Check assumptions. I'm not sure what order to switch these: assert not old_prefix.arg_types or not prefix.arg_types - # ... and I don't know where to put non-paramspec t.arg_types - assert len(t.arg_types) == 2 or t.arg_types[:-2] == old_prefix.arg_types t = t.copy_modified( - arg_types=prefix.arg_types + old_prefix.arg_types + t.arg_types[-2:], - arg_kinds=prefix.arg_kinds + old_prefix.arg_kinds + t.arg_kinds[-2:], - arg_names=prefix.arg_names + old_prefix.arg_names + t.arg_names[-2:], + 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() diff --git a/test-data/unit/check-parameter-specification.test b/test-data/unit/check-parameter-specification.test index 3678ab770046..2255d315358a 100644 --- a/test-data/unit/check-parameter-specification.test +++ b/test-data/unit/check-parameter-specification.test @@ -1486,3 +1486,25 @@ def foo(f: Callable[P, int]) -> Callable[P, int]: 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]: + ... + +@overload +def command( + cls: int = ..., +) -> Callable[[Callable[Concatenate[object, P], object]], None]: + ... + +def command( + cls: int = 42, +) -> Any: + ... +[builtins fixtures/paramspec.pyi] From d58f194ad1ffb6ba247d7bc74a26d43a2f50c6df Mon Sep 17 00:00:00 2001 From: A5rocks Date: Sun, 12 Feb 2023 09:07:32 +0900 Subject: [PATCH 4/5] Include errors on the regression test --- test-data/unit/check-parameter-specification.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-parameter-specification.test b/test-data/unit/check-parameter-specification.test index 2255d315358a..9c8f1699d05d 100644 --- a/test-data/unit/check-parameter-specification.test +++ b/test-data/unit/check-parameter-specification.test @@ -1494,7 +1494,7 @@ from typing_extensions import Concatenate, ParamSpec P = ParamSpec("P") @overload -def command() -> Callable[[Callable[Concatenate[object, object, P], object]], None]: +def command() -> Callable[[Callable[Concatenate[object, object, P], object]], None]: # E: Overloaded function signatures 1 and 2 overlap with incompatible return types ... @overload From ed520341197d313fa9bab76cea126e1c65776d37 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Mon, 20 Feb 2023 16:11:48 +0900 Subject: [PATCH 5/5] PR feedback --- mypy/expandtype.py | 8 ++++---- mypy/types.py | 4 +--- test-data/unit/check-parameter-specification.test | 13 +++++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/mypy/expandtype.py b/mypy/expandtype.py index df375ac1d27c..d22bc6cd0b51 100644 --- a/mypy/expandtype.py +++ b/mypy/expandtype.py @@ -373,16 +373,16 @@ 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) + # TODO: Conceptually, the "len(t.arg_types) == 2" should not be here. However, this + # errors without it. Either figure out how to eliminate this or place an + # explanation for why this is necessary. 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: + # Check assumptions. I'm not sure what order to place new prefix vs old prefix: assert not old_prefix.arg_types or not prefix.arg_types t = t.copy_modified( diff --git a/mypy/types.py b/mypy/types.py index 783eb28a2912..964da07c56cb 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1970,9 +1970,7 @@ 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) or not isinstance( - self.arg_types[-1], ParamSpecType - ): + if not isinstance(arg_type, ParamSpecType): return None # sometimes paramspectypes are analyzed in from mysterious places, diff --git a/test-data/unit/check-parameter-specification.test b/test-data/unit/check-parameter-specification.test index 9c8f1699d05d..7ef0485f7841 100644 --- a/test-data/unit/check-parameter-specification.test +++ b/test-data/unit/check-parameter-specification.test @@ -1508,3 +1508,16 @@ def command( ) -> Any: ... [builtins fixtures/paramspec.pyi] + +[case testCopiedParamSpecComparison] +# minimized from https://github.com/python/mypy/issues/12909 +from typing import Callable +from typing_extensions import ParamSpec + +P = ParamSpec("P") + +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]