Avoid false "Incompatible return value type" errors when dealing with isinstance, constrained type variables and multiple inheritance (Fixes #13800)

mypy/checker.py

@@ -313,6 +313,9 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
     scope: CheckerScope
     # Stack of function return types
     return_types: list[Type]
+    # Registry of the return types already evaluated when checking a function
+    # definition that includes constrained type variables
+    previously_expanded_return_types: list[Type]
     # Flags; true for dynamically typed functions
     dynamic_funcs: list[bool]
     # Stack of collections of variables with partial types
@@ -383,6 +386,7 @@ def __init__(
         self.binder = ConditionalTypeBinder()
         self.globals = tree.names
         self.return_types = []
+        self.previously_expanded_return_types = []
         self.dynamic_funcs = []
         self.partial_types = []
         self.partial_reported = set()
@@ -1302,9 +1306,10 @@ def check_func_def(
                     ):
                         self.note(message_registry.EMPTY_BODY_ABSTRACT, defn)
 
-            self.return_types.pop()
+            self.previously_expanded_return_types.append(self.return_types.pop())
 
             self.binder = old_binder
+        self.previously_expanded_return_types.clear()
 
     def check_unbound_return_typevar(self, typ: CallableType) -> None:
         """Fails when the return typevar is not defined in arguments."""
@@ -4225,7 +4230,28 @@ def visit_if_stmt(self, s: IfStmt) -> None:
                 # XXX Issue a warning if condition is always False?
                 with self.binder.frame_context(can_skip=True, fall_through=2):
                     self.push_type_map(if_map)
-                    self.accept(b)
+                    # Eventually, broaden the expanded return type to avoid false
+                    # "incompatible return value type" error messages (see issue 13800).
+                    if self.return_types and (if_map is not None):
+                        original_return_type = self.return_types[-1]
+                        for typ in if_map.values():
+                            typ = get_proper_type(typ)
+                            if isinstance(typ, UnionType):
+                                typs = flatten_nested_unions(typ.items)
+                            else:
+                                typs = [typ]
+                            for typ in typs:
+                                typ = get_proper_type(typ)
+                                if isinstance(typ, Instance) and (typ.type.is_intersection):
+                                    for base in typ.type.bases:
+                                        if base in self.previously_expanded_return_types:
+                                            self.return_types[-1] = UnionType.make_union(
+                                                (self.return_types[-1], base)
+                                            )
+                        self.accept(b)
+                        self.return_types[-1] = original_return_type
+                    else:
+                        self.accept(b)
 
                 # XXX Issue a warning if condition is always True?
                 self.push_type_map(else_map)

test-data/unit/check-isinstance.test

@@ -2486,24 +2486,97 @@ class B:
     attr: int
 class C:
     attr: str
+class D:
+    attr: int
+
+# basic test cases:
 
 T1 = TypeVar('T1', A, B)
 def f1(x: T1) -> T1:
     if isinstance(x, A):
-        # The error message is confusing, but we indeed do run into problems if
-        # 'x' is a subclass of A and B
-        return A()   # E: Incompatible return value type (got "A", expected "B")
+        # No error, because "A" takes priority over "B" when passing a subclass
+        # of "A" and "B" to "f1".
+        return A()
     else:
         return B()
 
 T2 = TypeVar('T2', B, C)
 def f2(x: T2) -> T2:
     if isinstance(x, B):
-        # In contrast, it's impossible for a subclass of "B" and "C" to
-        # exist, so this is fine
+        # No error, because a subclass of "B" and "C" cannot exist.
         return B()
     else:
         return C()
+
+T3 = TypeVar('T3', B, A)
+def f3(x: T3) -> T3:
+    if isinstance(x, A):
+        # The error message is confusing, but we indeed do run into problems if
+        # 'x' is a subclass of "A" and "B" and "B" takes priority over "A".
+        return A()  # E: Incompatible return value type (got "A", expected "B")
+    else:
+        return B()
+
+# more complex test cases:
+
+T4 = TypeVar('T4', A, B, D)
+def f4(x: T4) -> T4:
+    if isinstance(x, A):
+        return A()
+    elif isinstance(x, B):
+        return B()
+    else:
+        return D()
+
+T5 = TypeVar('T5', A, B, D)
+def f5(x: T5) -> T5:
+    if isinstance(x, A):
+        return A()
+    if isinstance(x, B):
+        return B()
+    return D()
+
+T6 = TypeVar('T6', A, B, D)
+def f6(x: T6) -> T6:
+    if isinstance(x, A):
+        if isinstance(x, B):
+            return B()  # E: Incompatible return value type (got "B", expected "A")
+        return A()
+    if isinstance(x, B):
+        return B()
+    return D()
+
+T7 = TypeVar('T7', A, B, D)
+def f7(x: T7) -> T7:
+    if isinstance(x, (A, B)):
+        return A()  # E: Incompatible return value type (got "A", expected "B")
+    return D()
+
+T8 = TypeVar('T8', A, B, D)
+def f8(x: T8) -> T8:
+    if isinstance(x, (A, B)):
+        if isinstance(x, A):
+            return A()
+        return B()
+    return D()
+
+T9 = TypeVar('T9', A, D, B)
+def f9(x: T9) -> T9:
+    if isinstance(x, A):
+        return A()
+    elif isinstance(x, B):
+        return B()  # E: Incompatible return value type (got "B", expected "D")
+    else:
+        return D()
+
+T10 = TypeVar('T10', A, D, B)
+def f10(x: T10) -> T10:
+    if isinstance(x, (A, B)):
+        if isinstance(x, A):
+            return A()
+        return B() # E: Incompatible return value type (got "B", expected "Union[D, A]")
+    return D()
+
 [builtins fixtures/isinstance.pyi]
 
 [case testIsInstanceAdHocIntersectionUsage]