Make overload impl checks correctly handle TypeVars and untyped impls

This pull request fixes #4619 as
well as an unrelated bug where if the overload implementation was untyped,
we only check to see if there are unsafe overlaps between the *first*
overload alternative and the rest.

Author: Michael0x2a

mypy/checker.py

@@ -42,6 +42,7 @@
     restrict_subtype_away, is_subtype_ignoring_tvars, is_callable_compatible,
     unify_generic_callable, find_member
 )
+from mypy.constraints import SUPERTYPE_OF
 from mypy.maptype import map_instance_to_supertype
 from mypy.typevars import fill_typevars, has_no_typevars
 from mypy.semanal import set_callable_name, refers_to_fullname, calculate_mro
@@ -414,6 +415,23 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
     def check_overlapping_overloads(self, defn: OverloadedFuncDef) -> None:
         # At this point we should have set the impl already, and all remaining
         # items are decorators
+
+        # Compute some info about the implementation (if it exists) for use below
+        impl_type = None  # type: Optional[CallableType]
+        if defn.impl:
+            if isinstance(defn.impl, FuncDef):
+                inner_type = defn.impl.type
+            elif isinstance(defn.impl, Decorator):
+                inner_type = defn.impl.var.type
+            else:
+                assert False, "Impl isn't the right type"
+
+            # This can happen if we've got an overload with a different
+            # decorator or if the implementation is untyped -- we gave up on the types.
+            if inner_type is not None and not isinstance(inner_type, AnyType):
+                assert isinstance(inner_type, CallableType)
+                impl_type = inner_type
+
         is_descriptor_get = defn.info is not None and defn.name() == "__get__"
         for i, item in enumerate(defn.items):
             # TODO overloads involving decorators
@@ -451,43 +469,36 @@ def check_overlapping_overloads(self, defn: OverloadedFuncDef) -> None:
                             self.msg.overloaded_signatures_overlap(
                                 i + 1, i + j + 2, item.func)
 
-            if defn.impl:
-                if isinstance(defn.impl, FuncDef):
-                    impl_type = defn.impl.type
-                elif isinstance(defn.impl, Decorator):
-                    impl_type = defn.impl.var.type
-                else:
-                    assert False, "Impl isn't the right type"
+            if impl_type is not None:
+                assert defn.impl is not None
 
-                # This can happen if we've got an overload with a different
-                # decorator too -- we gave up on the types.
-                if impl_type is None or isinstance(impl_type, AnyType):
-                    return
-                assert isinstance(impl_type, CallableType)
+                # We perform a unification step that's very similar to what
+                # 'is_callable_compatible' would have done if we had set
+                # 'unify_generics' to True -- the only difference is that
+                # we check and see if the impl_type's return value is a
+                # *supertype* of the overload alternative, not a *subtype*.
+                #
+                # This is to match the direction the implementation's return
+                # needs to be compatible in.
+                if impl_type.variables:
+                    impl = unify_generic_callable(impl_type, sig1,
+                                                  ignore_return=False,
+                                                  return_constraint_direction=SUPERTYPE_OF)
+                    if impl is None:
+                        self.msg.overloaded_signatures_typevar_specific(i + 1, defn.impl)
+                        continue
+                else:
+                    impl = impl_type
 
                 # Is the overload alternative's arguments subtypes of the implementation's?
-                if not is_callable_compatible(impl_type, sig1,
+                if not is_callable_compatible(impl, sig1,
                                               is_compat=is_subtype,
-                                              ignore_return=True):
+                                              ignore_return=True,
+                                              unify_generics=False):
                     self.msg.overloaded_signatures_arg_specific(i + 1, defn.impl)
 
-                #  Repeat the same unification process 'is_callable_compatible'
-                #  internally performs so we can examine the return type separately.
-                if impl_type.variables:
-                    # Note: we set 'ignore_return=True' because 'unify_generic_callable'
-                    # normally checks the arguments and return types with differing variance.
-                    #
-                    # This is normally what we want, but for checking the validity of overload
-                    # implementations, we actually want to use the same variance for both.
-                    #
-                    # TODO: Patch 'is_callable_compatible' and  'unify_generic_callable'?
-                    #       somehow so we can customize the variance in all different sorts
-                    #       of ways? This would let us infer more constraints, letting us
-                    #       infer more precise types.
-                    impl_type = unify_generic_callable(impl_type, sig1, ignore_return=True)
-
                 # Is the overload alternative's return type a subtype of the implementation's?
-                if impl_type is not None and not is_subtype(sig1.ret_type, impl_type.ret_type):
+                if not is_subtype(sig1.ret_type, impl.ret_type):
                     self.msg.overloaded_signatures_ret_specific(i + 1, defn.impl)
 
     # Here's the scoop about generators and coroutines.

mypy/messages.py

@@ -961,13 +961,17 @@ def overloaded_signature_will_never_match(self, index1: int, index2: int,
                 index2=index2),
             context)
 
-    def overloaded_signatures_arg_specific(self, index1: int, context: Context) -> None:
+    def overloaded_signatures_typevar_specific(self, index: int, context: Context) -> None:
+        self.fail('Overloaded function implementation cannot satisfy signature {} '.format(index) +
+                  'due to inconsistencies in how they use type variables', context)
+
+    def overloaded_signatures_arg_specific(self, index: int, context: Context) -> None:
         self.fail('Overloaded function implementation does not accept all possible arguments '
-                  'of signature {}'.format(index1), context)
+                  'of signature {}'.format(index), context)
 
-    def overloaded_signatures_ret_specific(self, index1: int, context: Context) -> None:
+    def overloaded_signatures_ret_specific(self, index: int, context: Context) -> None:
         self.fail('Overloaded function implementation cannot produce return type '
-                  'of signature {}'.format(index1), context)
+                  'of signature {}'.format(index), context)
 
     def operator_method_signatures_overlap(
             self, reverse_class: TypeInfo, reverse_method: str, forward_class: Type,

mypy/subtypes.py

@@ -198,6 +198,8 @@ def visit_type_var(self, left: TypeVarType) -> bool:
         right = self.right
         if isinstance(right, TypeVarType) and left.id == right.id:
             return True
+        if left.values and is_subtype(UnionType.make_simplified_union(left.values), right):
+            return True
         return is_subtype(left.upper_bound, self.right)
 
     def visit_callable_type(self, left: CallableType) -> bool:
@@ -578,7 +580,8 @@ def is_callable_compatible(left: CallableType, right: CallableType,
                            ignore_return: bool = False,
                            ignore_pos_arg_names: bool = False,
                            check_args_covariantly: bool = False,
-                           allow_partial_overlap: bool = False) -> bool:
+                           allow_partial_overlap: bool = False,
+                           unify_generics: bool = True) -> bool:
     """Is the left compatible with the right, using the provided compatibility check?
 
     is_compat:
@@ -666,6 +669,11 @@ def g(x: int) -> int: ...
 
         If the 'some_check' function is also symmetric, the two calls would be equivalent
         whether or not we check the args covariantly.
+
+    unify_generics:
+        If this parameter is set to False, we do not attempt to unify away TypeVars before
+        checking the callable. This option should be set to False only if you want to externally
+        handle generics in some custom way.
     """
     if is_compat_return is None:
         is_compat_return = is_compat
@@ -678,34 +686,35 @@ def g(x: int) -> int: ...
     if right.is_type_obj() and not left.is_type_obj():
         return False
 
-    # A callable L is a subtype of a generic callable R if L is a
-    # subtype of every type obtained from R by substituting types for
-    # the variables of R. We can check this by simply leaving the
-    # generic variables of R as type variables, effectively varying
-    # over all possible values.
-
-    # It's okay even if these variables share ids with generic
-    # type variables of L, because generating and solving
-    # constraints for the variables of L to make L a subtype of R
-    # (below) treats type variables on the two sides as independent.
-    if left.variables:
-        # Apply generic type variables away in left via type inference.
-        unified = unify_generic_callable(left, right, ignore_return=ignore_return)
-        if unified is None:
-            return False
-        else:
-            left = unified
+    if unify_generics:
+        # A callable L is a subtype of a generic callable R if L is a
+        # subtype of every type obtained from R by substituting types for
+        # the variables of R. We can check this by simply leaving the
+        # generic variables of R as type variables, effectively varying
+        # over all possible values.
+
+        # It's okay even if these variables share ids with generic
+        # type variables of L, because generating and solving
+        # constraints for the variables of L to make L a subtype of R
+        # (below) treats type variables on the two sides as independent.
+        if left.variables:
+            # Apply generic type variables away in left via type inference.
+            unified = unify_generic_callable(left, right, ignore_return=ignore_return)
+            if unified is None:
+                return False
+            else:
+                left = unified
 
-    # If we allow partial overlaps, we don't need to leave R generic:
-    # if we can find even just a single typevar assignment which
-    # would make these callables compatible, we should return True.
+        # If we allow partial overlaps, we don't need to leave R generic:
+        # if we can find even just a single typevar assignment which
+        # would make these callables compatible, we should return True.
 
-    # So, we repeat the above checks in the opposite direction. This also
-    # lets us preserve the 'symmetry' property of allow_partial_overlap.
-    if allow_partial_overlap and right.variables:
-        unified = unify_generic_callable(right, left, ignore_return=ignore_return)
-        if unified is not None:
-            right = unified
+        # So, we repeat the above checks in the opposite direction. This also
+        # lets us preserve the 'symmetry' property of allow_partial_overlap.
+        if allow_partial_overlap and right.variables:
+            unified = unify_generic_callable(right, left, ignore_return=ignore_return)
+            if unified is not None:
+                right = unified
 
     # Check return types.
     if not ignore_return and not is_compat_return(left.ret_type, right.ret_type):
@@ -901,7 +910,9 @@ def new_is_compat(left: Type, right: Type) -> bool:
 
 
 def unify_generic_callable(type: CallableType, target: CallableType,
-                           ignore_return: bool) -> Optional[CallableType]:
+                           ignore_return: bool,
+                           return_constraint_direction: int = mypy.constraints.SUBTYPE_OF,
+                           ) -> Optional[CallableType]:
     """Try to unify a generic callable type with another callable type.
 
     Return unified CallableType if successful; otherwise, return None.
@@ -914,7 +925,7 @@ def unify_generic_callable(type: CallableType, target: CallableType,
         constraints.extend(c)
     if not ignore_return:
         c = mypy.constraints.infer_constraints(
-            type.ret_type, target.ret_type, mypy.constraints.SUBTYPE_OF)
+            type.ret_type, target.ret_type, return_constraint_direction)
         constraints.extend(c)
     type_var_ids = [tvar.id for tvar in type.variables]
     inferred_vars = mypy.solve.solve_constraints(type_var_ids, constraints)
@@ -1036,7 +1047,8 @@ def check_argument(leftarg: Type, rightarg: Type, variance: int) -> bool:
     def visit_type_var(self, left: TypeVarType) -> bool:
         if isinstance(self.right, TypeVarType) and left.id == self.right.id:
             return True
-        # TODO: Value restrictions
+        if left.values and is_subtype(UnionType.make_simplified_union(left.values), self.right):
+            return True
         return is_proper_subtype(left.upper_bound, self.right)
 
     def visit_callable_type(self, left: CallableType) -> bool:

test-data/unit/check-overloading.test

@@ -204,6 +204,17 @@ class A: pass
 class B: pass
 [builtins fixtures/isinstance.pyi]
 
+[case testTypeCheckOverloadWithUntypedImplAndMultipleVariants]
+from typing import overload
+
+@overload
+def f(x: int) -> str: ...
+@overload
+def f(x: str) -> int: ...  # E: Overloaded function signatures 2 and 3 overlap with incompatible return types
+@overload
+def f(x: object) -> str: ...
+def f(x): ...
+
 [case testTypeCheckOverloadWithImplTooSpecificArg]
 from typing import overload, Any
 
@@ -284,14 +295,61 @@ def f(x: 'A') -> 'A': ...
 @overload
 def f(x: 'B') -> 'B': ...
 
-def f(x: Union[T, B]) -> T:  # E: Overloaded function implementation cannot produce return type of signature 2
+def f(x: Union[T, B]) -> T:  # E: Overloaded function implementation cannot satisfy signature 2 due to inconsistencies in how they use type variables
     ...
 
 reveal_type(f(A())) # E: Revealed type is '__main__.A'
 reveal_type(f(B())) # E: Revealed type is '__main__.B'
 
 [builtins fixtures/isinstance.pyi]
 
+[case testTypeCheckOverloadImplementationTypeVarWithValueRestriction]
+from typing import overload, TypeVar, Union
+
+class A: pass
+class B: pass
+class C: pass
+
+T = TypeVar('T', A, B)
+
+@overload
+def foo(x: T) -> T: ...
+@overload
+def foo(x: C) -> int: ...
+def foo(x: Union[A, B, C]) -> Union[A, B, int]:
+    if isinstance(x, C):
+        return 3
+    else:
+        return x
+
+@overload
+def bar(x: T) -> T: ...
+@overload
+def bar(x: C) -> int: ...
+def bar(x: Union[T, C]) -> Union[T, int]:
+    if isinstance(x, C):
+        return 3
+    else:
+        return x
+
+[builtins fixtures/isinstancelist.pyi]
+
+[case testTypeCheckOverloadImplementationTypeVarDifferingUsage]
+from typing import overload, Union, List, TypeVar
+
+T = TypeVar('T')
+
+@overload
+def foo(t: List[T]) -> T: ...
+@overload
+def foo(t: T) -> T: ...
+def foo(t: Union[List[T], T]) -> T:
+    if isinstance(t, list):
+        return t[0]
+    else:
+        return t
+[builtins fixtures/isinstancelist.pyi]
+
 [case testTypeCheckOverloadedFunctionBody]
 from foo import *
 [file foo.pyi]