[Reverted] Fixes to union simplification, isinstance and more

mypy/checker.py

@@ -28,7 +28,7 @@
     Type, AnyType, CallableType, FunctionLike, Overloaded, TupleType, TypedDictType,
     Instance, NoneTyp, ErrorType, strip_type, TypeType,
     UnionType, TypeVarId, TypeVarType, PartialType, DeletedType, UninhabitedType, TypeVarDef,
-    true_only, false_only, function_type, is_named_instance
+    true_only, false_only, function_type, is_named_instance, union_items
 )
 from mypy.sametypes import is_same_type, is_same_types
 from mypy.messages import MessageBuilder
@@ -768,44 +768,45 @@ def check_overlapping_op_methods(self,
         # of x in __radd__ would not be A, the methods could be
         # non-overlapping.
 
-        if isinstance(forward_type, CallableType):
-            # TODO check argument kinds
-            if len(forward_type.arg_types) < 1:
-                # Not a valid operator method -- can't succeed anyway.
-                return
+        for forward_item in union_items(forward_type):
+            if isinstance(forward_item, CallableType):
+                # TODO check argument kinds
+                if len(forward_item.arg_types) < 1:
+                    # Not a valid operator method -- can't succeed anyway.
+                    return
 
-            # Construct normalized function signatures corresponding to the
-            # operator methods. The first argument is the left operand and the
-            # second operand is the right argument -- we switch the order of
-            # the arguments of the reverse method.
-            forward_tweaked = CallableType(
-                [forward_base, forward_type.arg_types[0]],
-                [nodes.ARG_POS] * 2,
-                [None] * 2,
-                forward_type.ret_type,
-                forward_type.fallback,
-                name=forward_type.name)
-            reverse_args = reverse_type.arg_types
-            reverse_tweaked = CallableType(
-                [reverse_args[1], reverse_args[0]],
-                [nodes.ARG_POS] * 2,
-                [None] * 2,
-                reverse_type.ret_type,
-                fallback=self.named_type('builtins.function'),
-                name=reverse_type.name)
-
-            if is_unsafe_overlapping_signatures(forward_tweaked,
-                                                reverse_tweaked):
-                self.msg.operator_method_signatures_overlap(
-                    reverse_class.name(), reverse_name,
-                    forward_base.type.name(), forward_name, context)
-        elif isinstance(forward_type, Overloaded):
-            for item in forward_type.items():
-                self.check_overlapping_op_methods(
-                    reverse_type, reverse_name, reverse_class,
-                    item, forward_name, forward_base, context)
-        elif not isinstance(forward_type, AnyType):
-            self.msg.forward_operator_not_callable(forward_name, context)
+                # Construct normalized function signatures corresponding to the
+                # operator methods. The first argument is the left operand and the
+                # second operand is the right argument -- we switch the order of
+                # the arguments of the reverse method.
+                forward_tweaked = CallableType(
+                    [forward_base, forward_item.arg_types[0]],
+                    [nodes.ARG_POS] * 2,
+                    [None] * 2,
+                    forward_item.ret_type,
+                    forward_item.fallback,
+                    name=forward_item.name)
+                reverse_args = reverse_type.arg_types
+                reverse_tweaked = CallableType(
+                    [reverse_args[1], reverse_args[0]],
+                    [nodes.ARG_POS] * 2,
+                    [None] * 2,
+                    reverse_type.ret_type,
+                    fallback=self.named_type('builtins.function'),
+                    name=reverse_type.name)
+
+                if is_unsafe_overlapping_signatures(forward_tweaked,
+                                                    reverse_tweaked):
+                    self.msg.operator_method_signatures_overlap(
+                        reverse_class.name(), reverse_name,
+                        forward_base.type.name(), forward_name, context)
+            elif isinstance(forward_item, Overloaded):
+                for item in forward_item.items():
+                    self.check_overlapping_op_methods(
+                        reverse_type, reverse_name, reverse_class,
+                        item, forward_name, forward_base, context)
+            elif not isinstance(forward_item, AnyType):
+                self.msg.forward_operator_not_callable(forward_name, context)
 
     def check_inplace_operator_method(self, defn: FuncBase) -> None:
         """Check an inplace operator method such as __iadd__.

mypy/subtypes.py

@@ -16,6 +16,7 @@
     ARG_POS, ARG_OPT, ARG_NAMED, ARG_NAMED_OPT, ARG_STAR, ARG_STAR2,
 )
 from mypy.maptype import map_instance_to_supertype
+from mypy.sametypes import is_same_type
 
 from mypy import experiments
 
@@ -281,9 +282,15 @@ def visit_type_type(self, left: TypeType) -> bool:
 
 def is_callable_subtype(left: CallableType, right: CallableType,
                         ignore_return: bool = False,
-                        ignore_pos_arg_names: bool = False) -> bool:
+                        ignore_pos_arg_names: bool = False,
+                        use_proper_subtype: bool = False) -> bool:
     """Is left a subtype of right?"""
 
+    if use_proper_subtype:
+        is_compat = is_proper_subtype
+    else:
+        is_compat = is_subtype
+
     # If either function is implicitly typed, ignore positional arg names too
     if left.implicit or right.implicit:
         ignore_pos_arg_names = True
@@ -310,7 +317,7 @@ def is_callable_subtype(left: CallableType, right: CallableType,
             return False
 
     # Check return types.
-    if not ignore_return and not is_subtype(left.ret_type, right.ret_type):
+    if not ignore_return and not is_compat(left.ret_type, right.ret_type):
         return False
 
     if right.is_ellipsis_args:
@@ -367,7 +374,7 @@ def is_callable_subtype(left: CallableType, right: CallableType,
                 right_by_position = right.argument_by_position(j)
                 assert right_by_position is not None
                 if not are_args_compatible(left_by_position, right_by_position,
-                                           ignore_pos_arg_names):
+                                           ignore_pos_arg_names, use_proper_subtype):
                     return False
                 j += 1
             continue
@@ -390,7 +397,7 @@ def is_callable_subtype(left: CallableType, right: CallableType,
                 right_by_name = right.argument_by_name(name)
                 assert right_by_name is not None
                 if not are_args_compatible(left_by_name, right_by_name,
-                                           ignore_pos_arg_names):
+                                           ignore_pos_arg_names, use_proper_subtype):
                     return False
             continue
 
@@ -399,7 +406,7 @@ def is_callable_subtype(left: CallableType, right: CallableType,
         if left_arg is None:
             return False
 
-        if not are_args_compatible(left_arg, right_arg, ignore_pos_arg_names):
+        if not are_args_compatible(left_arg, right_arg, ignore_pos_arg_names, use_proper_subtype):
             return False
 
     done_with_positional = False
@@ -415,11 +422,11 @@ def is_callable_subtype(left: CallableType, right: CallableType,
 
         # Check that *args and **kwargs types match in this loop
         if left_kind == ARG_STAR:
-            if right_star_type is not None and not is_subtype(right_star_type, left_arg.typ):
+            if right_star_type is not None and not is_compat(right_star_type, left_arg.typ):
                 return False
             continue
         elif left_kind == ARG_STAR2:
-            if right_star2_type is not None and not is_subtype(right_star2_type, left_arg.typ):
+            if right_star2_type is not None and not is_compat(right_star2_type, left_arg.typ):
                 return False
             continue
 
@@ -450,7 +457,8 @@ def is_callable_subtype(left: CallableType, right: CallableType,
 def are_args_compatible(
         left: FormalArgument,
         right: FormalArgument,
-        ignore_pos_arg_names: bool) -> bool:
+        ignore_pos_arg_names: bool,
+        use_proper_subtype: bool) -> bool:
     # If right has a specific name it wants this argument to be, left must
     # have the same.
     if right.name is not None and left.name != right.name:
@@ -461,8 +469,12 @@ def are_args_compatible(
     if right.pos is not None and left.pos != right.pos:
         return False
     # Left must have a more general type
-    if not is_subtype(right.typ, left.typ):
-        return False
+    if use_proper_subtype:
+        if not is_proper_subtype(right.typ, left.typ):
+            return False
+    else:
+        if not is_subtype(right.typ, left.typ):
+            return False
     # If right's argument is optional, left's must also be.
     if not right.required and left.required:
         return False
@@ -519,10 +531,10 @@ def restrict_subtype_away(t: Type, s: Type) -> Type:
 
 
 def is_proper_subtype(left: Type, right: Type) -> bool:
-    """Check if t is a proper subtype of s?
+    """Is left a proper subtype of right?
 
     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.
+    Any types. Every usable type is a proper subtype of itself.
     """
     if isinstance(right, UnionType) and not isinstance(left, UnionType):
         return any([is_proper_subtype(left, item)
@@ -535,9 +547,13 @@ def __init__(self, right: Type) -> None:
         self.right = right
 
     def visit_unbound_type(self, left: UnboundType) -> bool:
+        # This can be called if there is a bad type annotation. The result probably
+        # doesn't matter much but by returning True we simplify these bad types away
+        # from unions, which could filter out some bogus messages.
         return True
 
     def visit_error_type(self, left: ErrorType) -> bool:
+        # This isn't a real type.
         return False
 
     def visit_type_list(self, left: TypeList) -> bool:
@@ -556,18 +572,21 @@ def visit_uninhabited_type(self, left: UninhabitedType) -> bool:
         return True
 
     def visit_erased_type(self, left: ErasedType) -> bool:
+        # This may be encountered during type inference. The result probably doesn't
+        # matter much.
         return True
 
     def visit_deleted_type(self, left: DeletedType) -> bool:
         return True
 
     def visit_instance(self, left: Instance) -> bool:
-        if isinstance(self.right, Instance):
+        right = self.right
+        if isinstance(right, Instance):
             for base in left.type.mro:
-                if base._promote and is_proper_subtype(base._promote, self.right):
+                if base._promote and is_proper_subtype(base._promote, right):
                     return True
 
-            if not left.type.has_base(self.right.type.fullname()):
+            if not left.type.has_base(right.type.fullname()):
                 return False
 
             def check_argument(leftarg: Type, rightarg: Type, variance: int) -> bool:
@@ -579,20 +598,34 @@ def check_argument(leftarg: Type, rightarg: Type, variance: int) -> bool:
                     return sametypes.is_same_type(leftarg, rightarg)
 
             # Map left type to corresponding right instances.
-            left = map_instance_to_supertype(left, self.right.type)
+            left = map_instance_to_supertype(left, 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))
+                       zip(left.args, right.args, 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
+        # TODO: Value restrictions
         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)
+        right = self.right
+        if isinstance(right, CallableType):
+            return is_callable_subtype(
+                left, right,
+                ignore_pos_arg_names=False,
+                use_proper_subtype=True)
+        elif isinstance(right, Overloaded):
+            return all(is_proper_subtype(left, item)
+                       for item in right.items())
+        elif isinstance(right, Instance):
+            return is_proper_subtype(left.fallback, right)
+        elif isinstance(right, TypeType):
+            # This is unsound, we don't check the __init__ signature.
+            return left.is_type_obj() and is_proper_subtype(left.ret_type, right.item)
+        return False
 
     def visit_tuple_type(self, left: TupleType) -> bool:
         right = self.right
@@ -606,7 +639,8 @@ def visit_tuple_type(self, left: TupleType) -> bool:
                     return False
                 iter_type = right.args[0]
                 if is_named_instance(right, 'builtins.tuple') and isinstance(iter_type, AnyType):
-                    # Special case plain 'tuple' which is needed for isinstance(x, tuple).
+                    # TODO: We shouldn't need this special case. This is currently needed
+                    #       for isinstance(x, tuple), though it's unclear why.
                     return True
                 return all(is_proper_subtype(li, iter_type) for li in left.items)
             return is_proper_subtype(left.fallback, right)
@@ -620,8 +654,16 @@ def visit_tuple_type(self, left: TupleType) -> bool:
         return False
 
     def visit_typeddict_type(self, left: TypedDictType) -> bool:
-        # TODO: Does it make sense to support TypedDict here?
-        return False
+        right = self.right
+        if isinstance(right, TypedDictType):
+            for name, typ in left.items.items():
+                if name in right.items and not is_same_type(typ, right.items[name]):
+                    return False
+            for name, typ in right.items.items():
+                if name not in left.items:
+                    return False
+            return True
+        return is_proper_subtype(left.fallback, right)
 
     def visit_overloaded(self, left: Overloaded) -> bool:
         # TODO: What's the right thing to do here?
@@ -635,18 +677,23 @@ def visit_partial_type(self, left: PartialType) -> bool:
         return False
 
     def visit_type_type(self, left: TypeType) -> bool:
+        # TODO: Handle metaclasses?
         right = self.right
         if isinstance(right, TypeType):
+            # This is unsound, we don't check the __init__ signature.
             return is_proper_subtype(left.item, right.item)
         if isinstance(right, CallableType):
-            # This is unsound, we don't check the __init__ signature.
+            # This is also unsound because of __init__.
             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'):
+            if right.type.fullname() == 'builtins.type':
+                # TODO: Strictly speaking, the type builtins.type is considered equivalent to
+                #       Type[Any]. However, this would break the is_proper_subtype check in
+                #       conditional_type_map for cases like isinstance(x, type) when the type
+                #       of x is Type[int]. It's unclear what's the right way to address this.
+                return True
+            if right.type.fullname() == 'builtins.object':
                 return True
-            item = left.item
-            return isinstance(item, Instance) and is_proper_subtype(item,
-                                                                    right.type.metaclass_type)
         return False
 
 

mypy/typeanal.py

@@ -7,7 +7,7 @@
     Type, UnboundType, TypeVarType, TupleType, TypedDictType, UnionType, Instance,
     AnyType, CallableType, NoneTyp, DeletedType, TypeList, TypeVarDef, TypeVisitor,
     StarType, PartialType, EllipsisType, UninhabitedType, TypeType, get_typ_args, set_typ_args,
-    get_type_vars,
+    get_type_vars, union_items
 )
 from mypy.nodes import (
     BOUND_TVAR, UNBOUND_TVAR, TYPE_ALIAS, UNBOUND_IMPORTED,
@@ -570,7 +570,8 @@ def make_optional_type(t: Type) -> Type:
         return t
     if isinstance(t, NoneTyp):
         return t
-    if isinstance(t, UnionType) and any(isinstance(item, NoneTyp)
-                                        for item in t.items):
-        return t
+    if isinstance(t, UnionType):
+        items = [item for item in union_items(t)
+                 if not isinstance(item, NoneTyp)]
+        return UnionType(items + [NoneTyp()], t.line, t.column)
     return UnionType([t, NoneTyp()], t.line, t.column)

mypy/types.py

@@ -1001,6 +1001,8 @@ def make_simplified_union(items: List[Type], line: int = -1, column: int = -1) -
         Note: This must NOT be used during semantic analysis, since TypeInfos may not
               be fully initialized.
         """
+        # TODO: Make this a function living somewhere outside mypy.types. Most other non-trivial
+        #       type operations are not static methods, so this is inconsistent.
         while any(isinstance(typ, UnionType) for typ in items):
             all_items = []  # type: List[Type]
             for typ in items:
@@ -1751,6 +1753,20 @@ def get_type_vars(typ: Type) -> List[TypeVarType]:
     return tvars
 
 
+def union_items(typ: Type) -> List[Type]:
+    """Return the flattened items of a union type.
+
+    For non-union types, return a list containing just the argument.
+    """
+    if isinstance(typ, UnionType):
+        items = []
+        for item in typ.items:
+            items.extend(union_items(item))
+        return items
+    else:
+        return [typ]
+
+
 deserialize_map = {
     key: obj.deserialize  # type: ignore
     for key, obj in globals().items()