Make overload impl checks correctly handle TypeVars and untyped impls (#5236)

* 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.

* Remove unify_generics parameter

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,35 @@ 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):
                     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

@@ -967,13 +967,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:
@@ -901,7 +903,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 +918,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 +1040,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]