Fixes to union simplification and isinstance

The main change is that unions containing Any are no longer simplified
to just Any.

This required changes in various other places to keep the existing
semantics, and resulted in some fixes to existing test cases.

Author: JukkaL

mypy/checkexpr.py

@@ -32,7 +32,7 @@
 from mypy import messages
 from mypy.infer import infer_type_arguments, infer_function_type_arguments
 from mypy import join
-from mypy.meet import meet_simple
+from mypy.meet import narrow_declared_type
 from mypy.maptype import map_instance_to_supertype
 from mypy.subtypes import is_subtype, is_equivalent
 from mypy import applytype
@@ -2213,8 +2213,7 @@ def narrow_type_from_binder(self, expr: Expression, known_type: Type) -> Type:
         if expr.literal >= LITERAL_TYPE:
             restriction = self.chk.binder.get(expr)
             if restriction:
-                ans = meet_simple(known_type, restriction)
-                return ans
+                return narrow_declared_type(known_type, restriction)
         return known_type
 
 

mypy/meet.py

@@ -25,21 +25,25 @@ def meet_types(s: Type, t: Type) -> Type:
     return t.accept(TypeMeetVisitor(s))
 
 
-def meet_simple(s: Type, t: Type, default_right: bool = True) -> Type:
-    if s == t:
-        return s
-    if isinstance(s, UnionType):
-        return UnionType.make_simplified_union([meet_types(x, t) for x in s.items])
-    elif not is_overlapping_types(s, t, use_promotions=True):
+def narrow_declared_type(declared: Type, narrowed: Type) -> Type:
+    """Return the declared type narrowed down to another type."""
+    # TODO: What are the reasons for not just using meet_types()?
+    if declared == narrowed:
+        return declared
+    if isinstance(declared, UnionType):
+        return UnionType.make_simplified_union([narrow_declared_type(x, narrowed)
+                                                for x in declared.items])
+    elif not is_overlapping_types(declared, narrowed, use_promotions=True):
         if experiments.STRICT_OPTIONAL:
             return UninhabitedType()
         else:
             return NoneTyp()
-    else:
-        if default_right:
-            return t
-        else:
-            return s
+    elif isinstance(narrowed, UnionType):
+        return UnionType.make_simplified_union([narrow_declared_type(declared, x)
+                                                for x in narrowed.items])
+    elif isinstance(narrowed, AnyType):
+        return narrowed
+    return meet_types(declared, narrowed)
 
 
 def is_overlapping_types(t: Type, s: Type, use_promotions: bool = False) -> bool:

mypy/subtypes.py

@@ -508,12 +508,17 @@ def restrict_subtype_away(t: Type, s: Type) -> Type:
         return t
 
 
-def is_proper_subtype(t: Type, s: Type) -> bool:
+def is_proper_subtype(left: Type, right: Type) -> bool:
     """Check if t is a proper subtype of s?
 
     For proper subtypes, there's no need to rely on compatibility due to
     Any types. Any instance type t is also a proper subtype of t.
     """
+    if isinstance(right, UnionType) and not isinstance(left, UnionType):
+        return any([is_proper_subtype(left, item)
+                    for item in right.items])
+    return left.accept(ProperSubtypeVisitor(right))
+
     # FIX tuple types
     if isinstance(s, UnionType):
         if isinstance(t, UnionType):
@@ -544,6 +549,122 @@ def check_argument(left: Type, right: Type, variance: int) -> bool:
         return sametypes.is_same_type(t, s)
 
 
+class ProperSubtypeVisitor(TypeVisitor[bool]):
+    def __init__(self, right: Type) -> None:
+        self.right = right
+
+    def visit_unbound_type(self, left: UnboundType) -> bool:
+        return True
+
+    def visit_error_type(self, left: ErrorType) -> bool:
+        return False
+
+    def visit_type_list(self, left: TypeList) -> bool:
+        assert False, 'Should not happen'
+
+    def visit_any(self, left: AnyType) -> bool:
+        return isinstance(self.right, AnyType)
+
+    def visit_void(self, left: Void) -> bool:
+        return True
+
+    def visit_none_type(self, left: NoneTyp) -> bool:
+        if experiments.STRICT_OPTIONAL:
+            return (isinstance(self.right, NoneTyp) or
+                    is_named_instance(self.right, 'builtins.object'))
+        else:
+            return not isinstance(self.right, Void)
+
+    def visit_uninhabited_type(self, left: UninhabitedType) -> bool:
+        return not isinstance(self.right, Void)
+
+    def visit_erased_type(self, left: ErasedType) -> bool:
+        return True
+
+    def visit_deleted_type(self, left: DeletedType) -> bool:
+        return True
+
+    def visit_instance(self, left: Instance) -> bool:
+        if isinstance(self.right, Instance):
+            if not left.type.has_base(self.right.type.fullname()):
+                return False
+
+            def check_argument(leftarg: Type, rightarg: Type, variance: int) -> bool:
+                if variance == COVARIANT:
+                    return is_proper_subtype(leftarg, rightarg)
+                elif variance == CONTRAVARIANT:
+                    return is_proper_subtype(rightarg, leftarg)
+                else:
+                    return sametypes.is_same_type(leftarg, rightarg)
+
+            # Map left type to corresponding right instances.
+            left = map_instance_to_supertype(left, self.right.type)
+
+            return all(check_argument(ta, ra, tvar.variance) for ta, ra, tvar in
+                       zip(left.args, self.right.args, self.right.type.defn.type_vars))
+        return False
+
+    def visit_type_var(self, left: TypeVarType) -> bool:
+        if isinstance(self.right, TypeVarType) and left.id == self.right.id:
+            return True
+        return is_proper_subtype(left.upper_bound, self.right)
+
+    def visit_callable_type(self, left: CallableType) -> bool:
+        # TODO: Implement this properly
+        return is_subtype(left, self.right)
+
+    def visit_tuple_type(self, left: TupleType) -> bool:
+        right = self.right
+        if isinstance(right, Instance):
+            if (is_named_instance(right, 'builtins.tuple') or
+                    is_named_instance(right, 'typing.Iterable') or
+                    is_named_instance(right, 'typing.Container') or
+                    is_named_instance(right, 'typing.Sequence') or
+                    is_named_instance(right, 'typing.Reversible')):
+                if not right.args:
+                    return False
+                iter_type = right.args[0]
+                return all(is_proper_subtype(li, iter_type) for li in left.items)
+            return is_proper_subtype(left.fallback, right)
+        elif isinstance(right, TupleType):
+            if len(left.items) != len(right.items):
+                return False
+            for l, r in zip(left.items, right.items):
+                if not is_proper_subtype(l, r):
+                    return False
+            return is_proper_subtype(left.fallback, right.fallback)
+        return False
+
+    def visit_typeddict_type(self, left: TypedDictType) -> bool:
+        # TODO: Does it make sense to support TypedDict here?
+        return False
+
+    def visit_overloaded(self, left: Overloaded) -> bool:
+        # TODO: What's the right thing to do here?
+        return False
+
+    def visit_union_type(self, left: UnionType) -> bool:
+        return all([is_proper_subtype(item, self.right) for item in left.items])
+
+    def visit_partial_type(self, left: PartialType) -> bool:
+        # TODO: What's the right thing to do here?
+        return False
+
+    def visit_type_type(self, left: TypeType) -> bool:
+        right = self.right
+        if isinstance(right, TypeType):
+            return is_proper_subtype(left.item, right.item)
+        if isinstance(right, CallableType):
+            # This is unsound, we don't check the __init__ signature.
+            return right.is_type_obj() and is_proper_subtype(left.item, right.ret_type)
+        if isinstance(right, Instance):
+            if right.type.fullname() in ('builtins.type', 'builtins.object'):
+                return True
+            item = left.item
+            return isinstance(item, Instance) and is_proper_subtype(item, right.type.metaclass_type)
+        return False
+
+
 def is_more_precise(t: Type, s: Type) -> bool:
     """Check if t is a more precise type than s.
 

mypy/types.py

@@ -995,11 +995,7 @@ def make_simplified_union(items: List[Type], line: int = -1, column: int = -1) -
                     all_items.append(typ)
             items = all_items
 
-        if any(isinstance(typ, AnyType) for typ in items):
-            return AnyType()
-
-        from mypy.subtypes import is_subtype
-        from mypy.sametypes import is_same_type
+        from mypy.subtypes import is_proper_subtype
 
         removed = set()  # type: Set[int]
         for i, ti in enumerate(items):
@@ -1008,9 +1004,7 @@ def make_simplified_union(items: List[Type], line: int = -1, column: int = -1) -
             cbt = cbf = False
             for j, tj in enumerate(items):
                 if (i != j
-                    and is_subtype(tj, ti)
-                    and (not (isinstance(tj, Instance) and tj.type.fallback_to_any)
-                         or is_same_type(ti, tj))):
+                    and is_proper_subtype(tj, ti)):
                     removed.add(j)
                     cbt = cbt or tj.can_be_true
                     cbf = cbf or tj.can_be_false

test-data/unit/check-classes.test

@@ -2388,9 +2388,9 @@ class A: pass
 class B(A): pass
 
 @overload
-def f(a: Type[A]) -> int: pass  # E: Overloaded function signatures 1 and 2 overlap with incompatible return types
+def f(a: Type[B]) -> int: pass  # E: Overloaded function signatures 1 and 2 overlap with incompatible return types
 @overload
-def f(a: Type[B]) -> str: pass
+def f(a: Type[A]) -> str: pass
 [builtins fixtures/classmethod.pyi]
 [out]
 

test-data/unit/check-dynamic-typing.test

@@ -79,8 +79,8 @@ n = 0
 d in a  # E: Unsupported right operand type for in ("A")
 d and a
 d or a
-c = d and b # Unintuitive type inference?
-c = d or b  # Unintuitive type inference?
+c = d and b # E: Incompatible types in assignment (expression has type "Union[Any, bool]", variable has type "C")
+c = d or b  # E: Incompatible types in assignment (expression has type "Union[Any, bool]", variable has type "C")
 
 c = d + a
 c = d - a
@@ -123,8 +123,8 @@ n = 0
 a and d
 a or d
 c = a in d
-c = b and d # Unintuitive type inference?
-c = b or d  # Unintuitive type inference?
+c = b and d # E: Incompatible types in assignment (expression has type "Union[bool, Any]", variable has type "C")
+c = b or d  # E: Incompatible types in assignment (expression has type "Union[bool, Any]", variable has type "C")
 b = a + d
 b = a / d
 

test-data/unit/check-generics.test

@@ -779,7 +779,7 @@ if not isinstance(s, str):
 
 z = None # type: TNode # Same as TNode[Any]
 z.x
-z.foo() # Any simplifies Union to Any now. This test should be updated after #2197
+z.foo() # E: Some element of union has no attribute "foo"
 
 [builtins fixtures/isinstance.pyi]
 

test-data/unit/check-isinstance.test

@@ -399,6 +399,7 @@ def f(x: Union[List[int], List[str], int]) -> None:
             a + 'x' # E: Unsupported operand types for + ("int" and "str")
 
         # type of a?
+        reveal_type(x) # E: Revealed type is 'Union[builtins.list[builtins.int], builtins.list[builtins.str]]'
         x + 1 # E: Unsupported operand types for + (likely involving Union)
     else:
         x[0] # E: Value of type "int" is not indexable

test-data/unit/check-optional.test

@@ -547,9 +547,31 @@ a = None # type: Any
 reveal_type(u(C(), None))  # E: Revealed type is 'Union[builtins.None, __main__.C*]'
 reveal_type(u(None, C()))  # E: Revealed type is 'Union[__main__.C*, builtins.None]'
 
-# This will be fixed later
-reveal_type(u(a, None))  # E: Revealed type is 'Any'
-reveal_type(u(None, a))  # E: Revealed type is 'Any'
+reveal_type(u(a, None))  # E: Revealed type is 'Union[builtins.None, Any]'
+reveal_type(u(None, a))  # E: Revealed type is 'Union[Any, builtins.None]'
 
 reveal_type(u(1, None))  # E: Revealed type is 'Union[builtins.None, builtins.int*]'
 reveal_type(u(None, 1))  # E: Revealed type is 'Union[builtins.int*, builtins.None]'
+
+[case testOptionalAndAnyBaseClass]
+from typing import Any, Optional
+class C(Any):
+    pass
+x = None  # type: Optional[C]
+x.foo()  # E: Some element of union has no attribute "foo"
+
+[case testUnionSimplificationWithStrictOptional]
+from typing import Any, TypeVar, Union
+class C(Any): pass
+T = TypeVar('T')
+S = TypeVar('S')
+def u(x: T, y: S) -> Union[S, T]: pass
+a = None # type: Any
+
+# Test both orders
+reveal_type(u(C(), None))  # E: Revealed type is 'Union[builtins.None, __main__.C*]'
+reveal_type(u(None, C()))  # E: Revealed type is 'Union[__main__.C*, builtins.None]'
+reveal_type(u(a, None))  # E: Revealed type is 'Union[builtins.None, Any]'
+reveal_type(u(None, a))  # E: Revealed type is 'Union[Any, builtins.None]'
+reveal_type(u(1, None))  # E: Revealed type is 'Union[builtins.None, builtins.int*]'
+reveal_type(u(None, 1))  # E: Revealed type is 'Union[builtins.int*, builtins.None]'

test-data/unit/check-statements.test

@@ -723,11 +723,11 @@ try:
 except BaseException as e1:
     reveal_type(e1)  # E: Revealed type is 'builtins.BaseException'
 except (E1, BaseException) as e2:
-    reveal_type(e2)  # E: Revealed type is 'Any'
+    reveal_type(e2)  # E: Revealed type is 'Union[Any, builtins.BaseException]'
 except (E1, E2) as e3:
-    reveal_type(e3)  # E: Revealed type is 'Any'
+    reveal_type(e3)  # E: Revealed type is 'Union[Any, __main__.E2]'
 except (E1, E2, BaseException) as e4:
-    reveal_type(e4)  # E: Revealed type is 'Any'
+    reveal_type(e4)  # E: Revealed type is 'Union[Any, builtins.BaseException]'
 
 try: pass
 except E1 as e1: