From 4b1fe1c6f9d75a852c666b4110abec6aa0c4552c Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Wed, 29 Mar 2023 16:04:15 +0200 Subject: [PATCH 01/10] Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). When finding no compatible callable right away, try to let method `ConstraintBuilderVisitor.find_matching_overload_item` return the first overloaded function with the suitable self type. Checked by test case `TestOverloadedMethodWithExplictSelfTypes`. --- mypy/constraints.py | 11 +++++--- test-data/unit/check-protocols.test | 39 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/mypy/constraints.py b/mypy/constraints.py index c8c3c7933b6e..a341a2f31149 100644 --- a/mypy/constraints.py +++ b/mypy/constraints.py @@ -10,7 +10,7 @@ from mypy.argmap import ArgTypeExpander from mypy.erasetype import erase_typevars from mypy.maptype import map_instance_to_supertype -from mypy.nodes import ARG_OPT, ARG_POS, CONTRAVARIANT, COVARIANT, ArgKind +from mypy.nodes import ARG_OPT, ARG_POS, CONTRAVARIANT, COVARIANT, ArgKind, FuncDef from mypy.types import ( TUPLE_LIKE_INSTANCE_NAMES, AnyType, @@ -1138,8 +1138,13 @@ def find_matching_overload_item(overloaded: Overloaded, template: CallableType) item, template, is_compat=mypy.subtypes.is_subtype, ignore_return=True ): return item - # Fall back to the first item if we can't find a match. This is totally arbitrary -- - # maybe we should just bail out at this point. + # Try to return the first item with the correct self type (fixes issue 14943). + for item in items: + if isinstance(item.definition, FuncDef) and isinstance(item.definition.type, CallableType): + if item.bound_args and item.definition.type.arg_types: + if item.bound_args[0] == item.definition.type.arg_types[0]: + return item + # Give up and just return the first of all items. return items[0] diff --git a/test-data/unit/check-protocols.test b/test-data/unit/check-protocols.test index 182745b99e40..7c7d352d9b22 100644 --- a/test-data/unit/check-protocols.test +++ b/test-data/unit/check-protocols.test @@ -4020,3 +4020,42 @@ class P(Protocol): [file lib.py] class C: ... + +[case TestOverloadedMethodWithExplictSelfTypes] +from typing import Generic, overload, Protocol, TypeVar, Union + +AnyStr = TypeVar("AnyStr", str, bytes) +T_co = TypeVar("T_co", covariant=True) +T_contra = TypeVar("T_contra", contravariant=True) + +class SupportsRead(Protocol[T_co]): + def read(self) -> T_co: ... + +class SupportsWrite(Protocol[T_contra]): + def write(self, s: T_contra) -> int: ... + +class Input(Generic[AnyStr]): + def read(self) -> AnyStr: ... + +class Output(Generic[AnyStr]): + @overload + def write(self: Output[str], s: str) -> int: ... + @overload + def write(self: Output[bytes], s: bytes) -> int: ... + def write(self, s: Union[str, bytes]) -> int: ... + +def f(src: SupportsRead[AnyStr], dst: SupportsWrite[AnyStr]) -> None: ... + +def g1(a: Input[bytes], b: Output[bytes]) -> None: + f(a, b) + +def g2(a: Input[bytes], b: Output[bytes]) -> None: + f(a, b) + +def g3(a: Input[str], b: Output[bytes]) -> None: + f(a, b) # E: Cannot infer type argument 1 of "f" + +def g4(a: Input[bytes], b: Output[str]) -> None: + f(a, b) # E: Cannot infer type argument 1 of "f" + +[builtins fixtures/tuple.pyi] From f3541539f07baa0a597e2a0baf960a54ee2c2ac3 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Tue, 11 Apr 2023 00:06:02 +0200 Subject: [PATCH 02/10] revert first attempt --- mypy/constraints.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/mypy/constraints.py b/mypy/constraints.py index a341a2f31149..c8c3c7933b6e 100644 --- a/mypy/constraints.py +++ b/mypy/constraints.py @@ -10,7 +10,7 @@ from mypy.argmap import ArgTypeExpander from mypy.erasetype import erase_typevars from mypy.maptype import map_instance_to_supertype -from mypy.nodes import ARG_OPT, ARG_POS, CONTRAVARIANT, COVARIANT, ArgKind, FuncDef +from mypy.nodes import ARG_OPT, ARG_POS, CONTRAVARIANT, COVARIANT, ArgKind from mypy.types import ( TUPLE_LIKE_INSTANCE_NAMES, AnyType, @@ -1138,13 +1138,8 @@ def find_matching_overload_item(overloaded: Overloaded, template: CallableType) item, template, is_compat=mypy.subtypes.is_subtype, ignore_return=True ): return item - # Try to return the first item with the correct self type (fixes issue 14943). - for item in items: - if isinstance(item.definition, FuncDef) and isinstance(item.definition.type, CallableType): - if item.bound_args and item.definition.type.arg_types: - if item.bound_args[0] == item.definition.type.arg_types[0]: - return item - # Give up and just return the first of all items. + # Fall back to the first item if we can't find a match. This is totally arbitrary -- + # maybe we should just bail out at this point. return items[0] From 589e0c66798fdee009473cb781758b7fe57a8676 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Tue, 11 Apr 2023 00:53:26 +0200 Subject: [PATCH 03/10] try filtering in `bind_self` instead --- mypy/subtypes.py | 7 +++++++ mypy/typeops.py | 20 +++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/mypy/subtypes.py b/mypy/subtypes.py index 9c6518b9e487..f3f4cf4bd2b9 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -86,6 +86,7 @@ def __init__( # Supported for both proper and non-proper ignore_promotions: bool = False, ignore_uninhabited: bool = False, + ignore_type_vars: bool = False, # Proper subtype flags erase_instances: bool = False, keep_erased_types: bool = False, @@ -96,6 +97,7 @@ def __init__( self.ignore_declared_variance = ignore_declared_variance self.ignore_promotions = ignore_promotions self.ignore_uninhabited = ignore_uninhabited + self.ignore_type_vars = ignore_type_vars self.erase_instances = erase_instances self.keep_erased_types = keep_erased_types self.options = options @@ -119,6 +121,7 @@ def is_subtype( ignore_declared_variance: bool = False, ignore_promotions: bool = False, ignore_uninhabited: bool = False, + ignore_type_vars: bool = False, options: Options | None = None, ) -> bool: """Is 'left' subtype of 'right'? @@ -139,6 +142,7 @@ def is_subtype( ignore_declared_variance=ignore_declared_variance, ignore_promotions=ignore_promotions, ignore_uninhabited=ignore_uninhabited, + ignore_type_vars=ignore_type_vars, options=options, ) else: @@ -287,6 +291,9 @@ def _is_subtype( # ErasedType as we do for non-proper subtyping. return True + if subtype_context.ignore_type_vars and (isinstance(left, TypeVarType) or isinstance(right, TypeVarType)): + return True + if isinstance(right, UnionType) and not isinstance(left, UnionType): # Normally, when 'left' is not itself a union, the only way # 'left' can be a subtype of the union 'right' is if it is a diff --git a/mypy/typeops.py b/mypy/typeops.py index 8c01fb118076..1a5c3fc0a35a 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -274,10 +274,28 @@ class B(A): pass b = B().copy() # type: B """ + + from mypy.subtypes import is_subtype + if isinstance(method, Overloaded): + # Try to remove overload items with non-matching self types first (fixes #14943) + origtype = get_proper_type(original_type) + if isinstance(origtype, Instance): + methoditems = [] + for mi in method.items: + selftype = get_proper_type(mi.arg_types[0]) + if not isinstance(selftype, Instance) or is_subtype( + origtype, selftype, ignore_type_vars=True + ): + methoditems.append(mi) + if len(methoditems) == 0: + methoditems = method.items + else: + methoditems = method.items return cast( - F, Overloaded([bind_self(c, original_type, is_classmethod) for c in method.items]) + F, Overloaded([bind_self(mi, original_type, is_classmethod) for mi in methoditems]) ) + assert isinstance(method, CallableType) func = method if not func.arg_types: From 9c107b30fc6aeae4ef34cc213896518a2edf49ea Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Tue, 11 Apr 2023 01:12:28 +0200 Subject: [PATCH 04/10] black --- mypy/subtypes.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mypy/subtypes.py b/mypy/subtypes.py index f3f4cf4bd2b9..261f11503d7b 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -291,7 +291,10 @@ def _is_subtype( # ErasedType as we do for non-proper subtyping. return True - if subtype_context.ignore_type_vars and (isinstance(left, TypeVarType) or isinstance(right, TypeVarType)): + + if subtype_context.ignore_type_vars and ( + isinstance(left, TypeVarType) or isinstance(right, TypeVarType) + ): return True if isinstance(right, UnionType) and not isinstance(left, UnionType): From b143796b3d7d555152875353d236a34908e6a36a Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Tue, 11 Apr 2023 01:25:35 +0200 Subject: [PATCH 05/10] flake --- mypy/subtypes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mypy/subtypes.py b/mypy/subtypes.py index 261f11503d7b..cbd5e731e3e7 100644 --- a/mypy/subtypes.py +++ b/mypy/subtypes.py @@ -291,7 +291,6 @@ def _is_subtype( # ErasedType as we do for non-proper subtyping. return True - if subtype_context.ignore_type_vars and ( isinstance(left, TypeVarType) or isinstance(right, TypeVarType) ): From 6b1573fe2043c85f87f06900dd6b078ad9a5d8b1 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Fri, 14 Apr 2023 21:47:29 +0200 Subject: [PATCH 06/10] Simplify `checker.TypeChecker.bind_and_map_method` and remove "xfail" from `testSelfTypeOverrideCompatibilityTypeVar` --- mypy/checker.py | 22 ++-------------------- test-data/unit/check-selftype.test | 2 +- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index cdf2ab648545..171c45822791 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1940,6 +1940,7 @@ def bind_and_map_method( sub_info: class where the method is used super_info: class where the method was defined """ + mapped_typ = cast(FunctionLike, map_type_from_supertype(typ, sub_info, super_info)) if isinstance(sym.node, (FuncDef, OverloadedFuncDef, Decorator)) and not is_static( sym.node ): @@ -1947,28 +1948,9 @@ def bind_and_map_method( is_class_method = sym.node.func.is_class else: is_class_method = sym.node.is_class - - mapped_typ = cast(FunctionLike, map_type_from_supertype(typ, sub_info, super_info)) active_self_type = self.scope.active_self_type() - if isinstance(mapped_typ, Overloaded) and active_self_type: - # If we have an overload, filter to overloads that match the self type. - # This avoids false positives for concrete subclasses of generic classes, - # see testSelfTypeOverrideCompatibility for an example. - filtered_items = [ - item - for item in mapped_typ.items - if not item.arg_types or is_subtype(active_self_type, item.arg_types[0]) - ] - # If we don't have any filtered_items, maybe it's always a valid override - # of the superclass? However if you get to that point you're in murky type - # territory anyway, so we just preserve the type and have the behaviour match - # that of older versions of mypy. - if filtered_items: - mapped_typ = Overloaded(filtered_items) - return bind_self(mapped_typ, active_self_type, is_class_method) - else: - return cast(FunctionLike, map_type_from_supertype(typ, sub_info, super_info)) + return mapped_typ def get_op_other_domain(self, tp: FunctionLike) -> Type | None: if isinstance(tp, CallableType): diff --git a/test-data/unit/check-selftype.test b/test-data/unit/check-selftype.test index 8083aaf7cf38..6d58997869f8 100644 --- a/test-data/unit/check-selftype.test +++ b/test-data/unit/check-selftype.test @@ -232,7 +232,7 @@ class C(A[None]): # N: def f(self, s: int) -> int [builtins fixtures/tuple.pyi] -[case testSelfTypeOverrideCompatibilityTypeVar-xfail] +[case testSelfTypeOverrideCompatibilityTypeVar] from typing import overload, TypeVar, Union AT = TypeVar("AT", bound="A") From f1414f268b38a1c16ee182c844723f108c155d8f Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Sat, 15 Apr 2023 01:33:07 +0200 Subject: [PATCH 07/10] Add `testSelfTypeOverrideCompatibilitySelfTypeVar` (not my work: taken from #15045 for comparison) --- test-data/unit/check-selftype.test | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test-data/unit/check-selftype.test b/test-data/unit/check-selftype.test index 6d58997869f8..53c24584cb73 100644 --- a/test-data/unit/check-selftype.test +++ b/test-data/unit/check-selftype.test @@ -266,6 +266,26 @@ class B(A): def f(*a, **kw): ... [builtins fixtures/dict.pyi] +[case testSelfTypeOverrideCompatibilitySelfTypeVar] +from typing import Any, Generic, Self, TypeVar, overload + +T_co = TypeVar('T_co', covariant=True) + +class Config(Generic[T_co]): + @overload + def get(self, instance: None) -> Self: ... + @overload + def get(self, instance: Any) -> T_co: ... + def get(self, *a, **kw): ... + +class MultiConfig(Config[T_co]): + @overload + def get(self, instance: None) -> Self: ... + @overload + def get(self, instance: Any) -> T_co: ... + def get(self, *a, **kw): ... +[builtins fixtures/dict.pyi] + [case testSelfTypeSuper] from typing import TypeVar, cast From 6e5476a9a4eb71c75361164c2c15d177b34f5b1a Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Sat, 15 Apr 2023 02:35:36 +0200 Subject: [PATCH 08/10] ignore_type_vars=False for testing --- mypy/typeops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index 1a5c3fc0a35a..498ec0567be1 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -285,7 +285,7 @@ class B(A): pass for mi in method.items: selftype = get_proper_type(mi.arg_types[0]) if not isinstance(selftype, Instance) or is_subtype( - origtype, selftype, ignore_type_vars=True + origtype, selftype, ignore_type_vars=False ): methoditems.append(mi) if len(methoditems) == 0: From 3097ac258e059da79c9e9836983aada2d764f054 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Sun, 16 Apr 2023 01:15:41 +0200 Subject: [PATCH 09/10] Also filter externally defined self types (preliminary approach) --- mypy/typeops.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index 498ec0567be1..ca4918e5760c 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -143,7 +143,9 @@ def type_object_type_from_function( # ... # # We need to map B's __init__ to the type (List[T]) -> None. - signature = bind_self(signature, original_type=default_self, is_classmethod=is_new) + signature = bind_self( + signature, original_type=default_self, is_classmethod=is_new, selftypes=orig_self_types + ) signature = cast(FunctionLike, map_type_from_supertype(signature, info, def_info)) special_sig: str | None = None @@ -251,7 +253,12 @@ def supported_self_type(typ: ProperType) -> bool: F = TypeVar("F", bound=FunctionLike) -def bind_self(method: F, original_type: Type | None = None, is_classmethod: bool = False) -> F: +def bind_self( + method: F, + original_type: Type | None = None, + is_classmethod: bool = False, + selftypes: list[Type | None] | None = None, +) -> F: """Return a copy of `method`, with the type of its first parameter (usually self or cls) bound to original_type. @@ -282,14 +289,22 @@ class B(A): pass origtype = get_proper_type(original_type) if isinstance(origtype, Instance): methoditems = [] - for mi in method.items: - selftype = get_proper_type(mi.arg_types[0]) - if not isinstance(selftype, Instance) or is_subtype( - origtype, selftype, ignore_type_vars=False + if selftypes is not None: + selftypes_copy = selftypes.copy() + selftypes.clear() + for idx, methoditem in enumerate(method.items): + selftype = get_self_type(methoditem, origtype) + selftype_proper = get_proper_type(selftype) + if not isinstance(selftype_proper, Instance) or is_subtype( + origtype, selftype_proper, ignore_type_vars=False ): - methoditems.append(mi) + methoditems.append(methoditem) + if selftypes is not None: + selftypes.append(selftypes_copy[idx]) if len(methoditems) == 0: methoditems = method.items + if selftypes is not None: + selftypes.extend(selftypes_copy) else: methoditems = method.items return cast( From 33a1f96c4c3ba32657835ed5b03a6ddc21fc255c Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Sun, 16 Apr 2023 01:16:14 +0200 Subject: [PATCH 10/10] reverse ignore_type_vars=False --- mypy/typeops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index ca4918e5760c..f0d40a61b29a 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -296,7 +296,7 @@ class B(A): pass selftype = get_self_type(methoditem, origtype) selftype_proper = get_proper_type(selftype) if not isinstance(selftype_proper, Instance) or is_subtype( - origtype, selftype_proper, ignore_type_vars=False + origtype, selftype_proper, ignore_type_vars=True ): methoditems.append(methoditem) if selftypes is not None: