From 59eba9c8b9226e236370a2d89a044a5fb5a9c235 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sat, 20 Aug 2022 19:12:52 +0100 Subject: [PATCH 1/3] Fix overload overlap check for UninhabitedType --- mypy/checker.py | 12 +++-- mypy/meet.py | 14 ++++-- mypy/subtypes.py | 64 ++++++++++++++++++++------- test-data/unit/check-overloading.test | 26 +++++++++++ 4 files changed, 93 insertions(+), 23 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index c275142c6efb..fe34d986f2b6 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -6484,7 +6484,7 @@ def is_unsafe_overlapping_overload_signatures( return is_callable_compatible( signature, other, - is_compat=is_overlapping_types_no_promote, + is_compat=is_overlapping_types_no_promote_no_uninhabited, is_compat_return=lambda l, r: not is_subtype_no_promote(l, r), ignore_return=False, check_args_covariantly=True, @@ -6492,7 +6492,7 @@ def is_unsafe_overlapping_overload_signatures( ) or is_callable_compatible( other, signature, - is_compat=is_overlapping_types_no_promote, + is_compat=is_overlapping_types_no_promote_no_uninhabited, is_compat_return=lambda l, r: not is_subtype_no_promote(r, l), ignore_return=False, check_args_covariantly=False, @@ -6976,8 +6976,12 @@ def is_subtype_no_promote(left: Type, right: Type) -> bool: return is_subtype(left, right, ignore_promotions=True) -def is_overlapping_types_no_promote(left: Type, right: Type) -> bool: - return is_overlapping_types(left, right, ignore_promotions=True) +def is_overlapping_types_no_promote_no_uninhabited(left: Type, right: Type) -> bool: + # For the purpose of unsafe overload checks we consider list[] and list[int] + # non-overlapping. This is consistent with how we treat list[int] and list[str] as + # non-overlapping, despite [] belongs to both. Also this will prevent false positives + # for failed type inference during unification. + return is_overlapping_types(left, right, ignore_promotions=True, ignore_uninhabited=True) def is_private(node_name: str) -> bool: diff --git a/mypy/meet.py b/mypy/meet.py index 21637f57f233..ca7f7ed42f39 100644 --- a/mypy/meet.py +++ b/mypy/meet.py @@ -211,6 +211,7 @@ def is_overlapping_types( right: Type, ignore_promotions: bool = False, prohibit_none_typevar_overlap: bool = False, + ignore_uninhabited: bool = False, ) -> bool: """Can a value of type 'left' also be of type 'right' or vice-versa? @@ -235,6 +236,7 @@ def _is_overlapping_types(left: Type, right: Type) -> bool: right, ignore_promotions=ignore_promotions, prohibit_none_typevar_overlap=prohibit_none_typevar_overlap, + ignore_uninhabited=ignore_uninhabited, ) # We should never encounter this type. @@ -282,8 +284,10 @@ def _is_overlapping_types(left: Type, right: Type) -> bool: ): return True - if is_proper_subtype(left, right, ignore_promotions=ignore_promotions) or is_proper_subtype( - right, left, ignore_promotions=ignore_promotions + if is_proper_subtype( + left, right, ignore_promotions=ignore_promotions, ignore_uninhabited=ignore_uninhabited + ) or is_proper_subtype( + right, left, ignore_promotions=ignore_promotions, ignore_uninhabited=ignore_uninhabited ): return True @@ -425,8 +429,10 @@ def _callable_overlap(left: CallableType, right: CallableType) -> bool: if isinstance(left, Instance) and isinstance(right, Instance): # First we need to handle promotions and structural compatibility for instances # that came as fallbacks, so simply call is_subtype() to avoid code duplication. - if is_subtype(left, right, ignore_promotions=ignore_promotions) or is_subtype( - right, left, ignore_promotions=ignore_promotions + if is_subtype( + left, right, ignore_promotions=ignore_promotions, ignore_uninhabited=ignore_uninhabited + ) or is_subtype( + right, left, ignore_promotions=ignore_promotions, ignore_uninhabited=ignore_uninhabited ): return True diff --git a/mypy/subtypes.py b/mypy/subtypes.py index 9e84e25695dd..0094f9cc3f97 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -67,7 +67,7 @@ IS_CLASSVAR: Final = 2 IS_CLASS_OR_STATIC: Final = 3 -TypeParameterChecker: _TypeAlias = Callable[[Type, Type, int, bool], bool] +TypeParameterChecker: _TypeAlias = Callable[[Type, Type, int, bool, "SubtypeContext"], bool] class SubtypeContext: @@ -80,6 +80,7 @@ def __init__( ignore_declared_variance: bool = False, # Supported for both proper and non-proper ignore_promotions: bool = False, + ignore_uninhabited: bool = False, # Proper subtype flags erase_instances: bool = False, keep_erased_types: bool = False, @@ -89,6 +90,7 @@ def __init__( self.ignore_pos_arg_names = ignore_pos_arg_names self.ignore_declared_variance = ignore_declared_variance self.ignore_promotions = ignore_promotions + self.ignore_uninhabited = ignore_uninhabited self.erase_instances = erase_instances self.keep_erased_types = keep_erased_types self.options = options @@ -115,6 +117,7 @@ def is_subtype( ignore_pos_arg_names: bool = False, ignore_declared_variance: bool = False, ignore_promotions: bool = False, + ignore_uninhabited: bool = False, options: Options | None = None, ) -> bool: """Is 'left' subtype of 'right'? @@ -134,6 +137,7 @@ def is_subtype( ignore_pos_arg_names=ignore_pos_arg_names, ignore_declared_variance=ignore_declared_variance, ignore_promotions=ignore_promotions, + ignore_uninhabited=ignore_uninhabited, options=options, ) else: @@ -143,6 +147,7 @@ def is_subtype( ignore_pos_arg_names, ignore_declared_variance, ignore_promotions, + ignore_uninhabited, options, } ), "Don't pass both context and individual flags" @@ -177,6 +182,7 @@ def is_proper_subtype( *, subtype_context: SubtypeContext | None = None, ignore_promotions: bool = False, + ignore_uninhabited: bool = False, erase_instances: bool = False, keep_erased_types: bool = False, ) -> bool: @@ -192,12 +198,19 @@ def is_proper_subtype( if subtype_context is None: subtype_context = SubtypeContext( ignore_promotions=ignore_promotions, + ignore_uninhabited=ignore_uninhabited, erase_instances=erase_instances, keep_erased_types=keep_erased_types, ) else: assert not any( - {ignore_promotions, erase_instances, keep_erased_types} + { + ignore_promotions, + ignore_uninhabited, + erase_instances, + keep_erased_types, + ignore_uninhabited, + } ), "Don't pass both context and individual flags" if TypeState.is_assumed_proper_subtype(left, right): return True @@ -215,6 +228,7 @@ def is_equivalent( ignore_type_params: bool = False, ignore_pos_arg_names: bool = False, options: Options | None = None, + subtype_context: SubtypeContext | None = None, ) -> bool: return is_subtype( a, @@ -222,16 +236,20 @@ def is_equivalent( ignore_type_params=ignore_type_params, ignore_pos_arg_names=ignore_pos_arg_names, options=options, + subtype_context=subtype_context, ) and is_subtype( b, a, ignore_type_params=ignore_type_params, ignore_pos_arg_names=ignore_pos_arg_names, options=options, + subtype_context=subtype_context, ) -def is_same_type(a: Type, b: Type, ignore_promotions: bool = True) -> bool: +def is_same_type( + a: Type, b: Type, ignore_promotions: bool = True, subtype_context: SubtypeContext | None = None +) -> bool: """Are these types proper subtypes of each other? This means types may have different representation (e.g. an alias, or @@ -241,8 +259,10 @@ def is_same_type(a: Type, b: Type, ignore_promotions: bool = True) -> bool: # considered not the same type (which is the case at runtime). # Also Union[bool, int] (if it wasn't simplified before) will be different # from plain int, etc. - return is_proper_subtype(a, b, ignore_promotions=ignore_promotions) and is_proper_subtype( - b, a, ignore_promotions=ignore_promotions + return is_proper_subtype( + a, b, ignore_promotions=ignore_promotions, subtype_context=subtype_context + ) and is_proper_subtype( + b, a, ignore_promotions=ignore_promotions, subtype_context=subtype_context ) @@ -306,11 +326,15 @@ def check_item(left: Type, right: Type, subtype_context: SubtypeContext) -> bool return left.accept(SubtypeVisitor(orig_right, subtype_context, proper_subtype)) -# TODO: should we pass on the original flags here and in couple other places? -# This seems logical but was never done in the past for some reasons. -def check_type_parameter(lefta: Type, righta: Type, variance: int, proper_subtype: bool) -> bool: +def check_type_parameter( + lefta: Type, righta: Type, variance: int, proper_subtype: bool, subtype_context: SubtypeContext +) -> bool: def check(left: Type, right: Type) -> bool: - return is_proper_subtype(left, right) if proper_subtype else is_subtype(left, right) + return ( + is_proper_subtype(left, right, subtype_context=subtype_context) + if proper_subtype + else is_subtype(left, right, subtype_context=subtype_context) + ) if variance == COVARIANT: return check(lefta, righta) @@ -318,11 +342,18 @@ def check(left: Type, right: Type) -> bool: return check(righta, lefta) else: if proper_subtype: - return is_same_type(lefta, righta) - return is_equivalent(lefta, righta) + # We pass ignore_promotions=False because it is a default for subtype checks. + # The actual value will be taken from the subtype_context, and it is whatever + # the original caller passed. + return is_same_type( + lefta, righta, ignore_promotions=False, subtype_context=subtype_context + ) + return is_equivalent(lefta, righta, subtype_context=subtype_context) -def ignore_type_parameter(lefta: Type, righta: Type, variance: int, proper_subtype: bool) -> bool: +def ignore_type_parameter( + lefta: Type, righta: Type, variance: int, proper_subtype: bool, subtype_context: SubtypeContext +) -> bool: return True @@ -385,7 +416,9 @@ def visit_none_type(self, left: NoneType) -> bool: return True def visit_uninhabited_type(self, left: UninhabitedType) -> bool: - return True + # We ignore this for unsafe overload checks, so that and empty list and + # a list of int will be considered non-overlapping. + return not self.subtype_context.ignore_uninhabited def visit_erased_type(self, left: ErasedType) -> bool: # This may be encountered during type inference. The result probably doesn't @@ -521,12 +554,12 @@ def check_mixed( for lefta, righta, tvar in type_params: if isinstance(tvar, TypeVarType): if not self.check_type_parameter( - lefta, righta, tvar.variance, self.proper_subtype + lefta, righta, tvar.variance, self.proper_subtype, self.subtype_context ): nominal = False else: if not self.check_type_parameter( - lefta, righta, COVARIANT, self.proper_subtype + lefta, righta, COVARIANT, self.proper_subtype, self.subtype_context ): nominal = False if nominal: @@ -694,6 +727,7 @@ def visit_typeddict_type(self, left: TypedDictType) -> bool: if not left.names_are_wider_than(right): return False for name, l, r in left.zip(right): + # TODO: should we pass on the full subtype_context here and below? if self.proper_subtype: check = is_same_type(l, r) else: diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index 33ab1a8602be..5032927dfb05 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -6467,3 +6467,29 @@ spam: Callable[..., str] = lambda x, y: 'baz' reveal_type(func(spam)) # N: Revealed type is "def (*Any, **Any) -> builtins.str" [builtins fixtures/paramspec.pyi] + +[case testGenericOverloadOverlapWithType] +import m + +[file m.pyi] +from typing import TypeVar, Type, overload, Callable + +T = TypeVar("T", bound=str) +@overload +def foo(x: Type[T] | int) -> int: ... +@overload +def foo(x: Callable[[int], bool]) -> str: ... + +[case testGenericOverloadOverlapWithCollection] +import m + +[file m.pyi] +from typing import TypeVar, Sequence, overload, List + +T = TypeVar("T", bound=str) + +@overload +def foo(x: List[T]) -> str: ... +@overload +def foo(x: Sequence[int]) -> int: ... +[builtins fixtures/list.pyi] From 20f001ba8006f8dcc6784dc328fbdfae7009f826 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sat, 20 Aug 2022 19:27:22 +0100 Subject: [PATCH 2/3] Fix bug --- mypy/meet.py | 2 +- mypy/subtypes.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/meet.py b/mypy/meet.py index ca7f7ed42f39..2e9818a0a06d 100644 --- a/mypy/meet.py +++ b/mypy/meet.py @@ -473,7 +473,7 @@ def _callable_overlap(left: CallableType, right: CallableType) -> bool: # Note: it's unclear however, whether returning False is the right thing # to do when inferring reachability -- see https://github.com/python/mypy/issues/5529 - assert type(left) != type(right) + assert type(left) != type(right), f"{type(left)} vs {type(right)}" return False diff --git a/mypy/subtypes.py b/mypy/subtypes.py index 0094f9cc3f97..0a4da609233c 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -418,6 +418,8 @@ def visit_none_type(self, left: NoneType) -> bool: def visit_uninhabited_type(self, left: UninhabitedType) -> bool: # We ignore this for unsafe overload checks, so that and empty list and # a list of int will be considered non-overlapping. + if isinstance(self.right, UninhabitedType): + return True return not self.subtype_context.ignore_uninhabited def visit_erased_type(self, left: ErasedType) -> bool: From 8192aa5ae9ea47f18693a4ee6d6a5526cee6a199 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sat, 20 Aug 2022 21:11:18 +0100 Subject: [PATCH 3/3] Switch to is_subtype() for overload implementation checks --- mypy/checker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index fe34d986f2b6..bf6aa9188cbb 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -747,14 +747,14 @@ def check_overlapping_overloads(self, defn: OverloadedFuncDef) -> None: # Is the overload alternative's arguments subtypes of the implementation's? if not is_callable_compatible( - impl, sig1, is_compat=is_subtype_no_promote, ignore_return=True + impl, sig1, is_compat=is_subtype, ignore_return=True ): self.msg.overloaded_signatures_arg_specific(i + 1, defn.impl) # Is the overload alternative's return type a subtype of the implementation's? if not ( - is_subtype_no_promote(sig1.ret_type, impl.ret_type) - or is_subtype_no_promote(impl.ret_type, sig1.ret_type) + is_subtype(sig1.ret_type, impl.ret_type) + or is_subtype(impl.ret_type, sig1.ret_type) ): self.msg.overloaded_signatures_ret_specific(i + 1, defn.impl)