Fix for #1698 (#1989)

This is an implementation of my proposed fix for #1698 ( #1698 (comment) )

Relevant differences from the description there:

After the suggestion from @ddfisher I used class attributes in Type instead of wrapping Type objects. When I want to restrict a type I make a copy and override the can_be_true or can_be_false with instance attributes
I didn't implement the rules for not after realising that I couldn't find a relevant example where it helped. It's easy to add later if we discover otherwise but I didn't want to bloat an already large PR
I noticed that the deductive power was good enough to start detecting more dead code in unrelated test cases (see modifications in check-statements, check-modules and check-generics).
There are some other improvements that I saw that could be made but I decided to stop here; if you think any of these are necessary I can add them:

Covering the "not" operator, as mentioned above
Renaming "find_isinstance_check" to something like "types_specialized_by_condition" which is more descriptive of its current purpose.
I used copy.copy() followed by an update to create restricted version of types, perhaps there's a better/safer way to copy those (and even cache the restricted versions to reduce memory usage)

Author: dmoisset

mypy/checker.py

@@ -33,7 +33,8 @@
 from mypy.types import (
     Type, AnyType, CallableType, Void, FunctionLike, Overloaded, TupleType,
     Instance, NoneTyp, ErrorType, strip_type,
-    UnionType, TypeVarId, TypeVarType, PartialType, DeletedType, UninhabitedType
+    UnionType, TypeVarId, TypeVarType, PartialType, DeletedType, UninhabitedType,
+    true_only, false_only
 )
 from mypy.sametypes import is_same_type
 from mypy.messages import MessageBuilder
@@ -2447,11 +2448,16 @@ def find_isinstance_check(node: Node,
             if is_not:
                 if_vars, else_vars = else_vars, if_vars
             return if_vars, else_vars
-    elif isinstance(node, RefExpr) and experiments.STRICT_OPTIONAL:
-        # The type could be falsy, so we can't deduce anything new about the else branch
+    elif isinstance(node, RefExpr):
+        # Restrict the type of the variable to True-ish/False-ish in the if and else branches
+        # respectively
         vartype = type_map[node]
-        _, if_vars = conditional_type_map(node, vartype, NoneTyp(), weak=weak)
-        return if_vars, {}
+        if_type = true_only(vartype)
+        else_type = false_only(vartype)
+        ref = node  # type: Node
+        if_map = {ref: if_type} if not isinstance(if_type, UninhabitedType) else None
+        else_map = {ref: else_type} if not isinstance(else_type, UninhabitedType) else None
+        return if_map, else_map
     elif isinstance(node, OpExpr) and node.op == 'and':
         left_if_vars, left_else_vars = find_isinstance_check(
             node.left,

mypy/checkexpr.py

@@ -5,7 +5,8 @@
 from mypy.types import (
     Type, AnyType, CallableType, Overloaded, NoneTyp, Void, TypeVarDef,
     TupleType, Instance, TypeVarId, TypeVarType, ErasedType, UnionType,
-    PartialType, DeletedType, UnboundType, UninhabitedType, TypeType
+    PartialType, DeletedType, UnboundType, UninhabitedType, TypeType,
+    true_only, false_only
 )
 from mypy.nodes import (
     NameExpr, RefExpr, Var, FuncDef, OverloadedFuncDef, TypeInfo, CallExpr,
@@ -1094,22 +1095,20 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type:
         ctx = self.chk.type_context[-1]
         left_type = self.accept(e.left, ctx)
 
+        assert e.op in ('and', 'or')  # Checked by visit_op_expr
+
         if e.op == 'and':
             right_map, left_map = \
                 mypy.checker.find_isinstance_check(e.left, self.chk.type_map,
                                                    self.chk.typing_mode_weak())
+            restricted_left_type = false_only(left_type)
+            result_is_left = not left_type.can_be_true
         elif e.op == 'or':
             left_map, right_map = \
                 mypy.checker.find_isinstance_check(e.left, self.chk.type_map,
                                                    self.chk.typing_mode_weak())
-        else:
-            left_map = None
-            right_map = None
-
-        if left_map and e.left in left_map:
-            # The type of expressions in left_map is the type they'll have if
-            # the left operand is the result of the operator.
-            left_type = left_map[e.left]
+            restricted_left_type = true_only(left_type)
+            result_is_left = not left_type.can_be_false
 
         with self.chk.binder.frame_context():
             if right_map:
@@ -1121,13 +1120,23 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type:
         self.check_usable_type(left_type, context)
         self.check_usable_type(right_type, context)
 
-        # If either of the type maps is None that means that result cannot happen.
-        # If both of the type maps are None we just have no information.
-        if left_map is not None and right_map is None:
+        if right_map is None:
+            # The boolean expression is statically known to be the left value
+            assert left_map is not None  # find_isinstance_check guarantees this
             return left_type
-        elif left_map is None and right_map is not None:
+        if left_map is None:
+            # The boolean expression is statically known to be the right value
+            assert right_map is not None  # find_isinstance_check guarantees this
             return right_type
-        return UnionType.make_simplified_union([left_type, right_type])
+
+        if isinstance(restricted_left_type, UninhabitedType):
+            # The left operand can never be the result
+            return right_type
+        elif result_is_left:
+            # The left operand is always the result
+            return left_type
+        else:
+            return UnionType.make_simplified_union([restricted_left_type, right_type])
 
     def check_list_multiply(self, e: OpExpr) -> Type:
         """Type check an expression of form '[...] * e'.

mypy/join.py

@@ -6,7 +6,7 @@
     Type, AnyType, NoneTyp, Void, TypeVisitor, Instance, UnboundType,
     ErrorType, TypeVarType, CallableType, TupleType, ErasedType, TypeList,
     UnionType, FunctionLike, Overloaded, PartialType, DeletedType,
-    UninhabitedType, TypeType
+    UninhabitedType, TypeType, true_or_false
 )
 from mypy.maptype import map_instance_to_supertype
 from mypy.subtypes import is_subtype, is_equivalent, is_subtype_ignoring_tvars
@@ -17,6 +17,11 @@
 def join_simple(declaration: Type, s: Type, t: Type) -> Type:
     """Return a simple least upper bound given the declared type."""
 
+    if (s.can_be_true, s.can_be_false) != (t.can_be_true, t.can_be_false):
+        # if types are restricted in different ways, use the more general versions
+        s = true_or_false(s)
+        t = true_or_false(t)
+
     if isinstance(s, AnyType):
         return s
 
@@ -60,6 +65,11 @@ def join_types(s: Type, t: Type) -> Type:
 
     If the join does not exist, return an ErrorType instance.
     """
+    if (s.can_be_true, s.can_be_false) != (t.can_be_true, t.can_be_false):
+        # if types are restricted in different ways, use the more general versions
+        s = true_or_false(s)
+        t = true_or_false(t)
+
     if isinstance(s, AnyType):
         return s
 

mypy/test/testtypes.py

@@ -3,15 +3,16 @@
 from typing import List
 
 from mypy.myunit import (
-    Suite, assert_equal, assert_true, assert_false
+    Suite, assert_equal, assert_true, assert_false, assert_type
 )
 from mypy.erasetype import erase_type
 from mypy.expandtype import expand_type
-from mypy.join import join_types
+from mypy.join import join_types, join_simple
 from mypy.meet import meet_types
 from mypy.types import (
     UnboundType, AnyType, Void, CallableType, TupleType, TypeVarDef, Type,
-    Instance, NoneTyp, ErrorType, Overloaded, TypeType,
+    Instance, NoneTyp, ErrorType, Overloaded, TypeType, UnionType, UninhabitedType,
+    true_only, false_only
 )
 from mypy.nodes import ARG_POS, ARG_OPT, ARG_STAR, CONTRAVARIANT, INVARIANT, COVARIANT
 from mypy.subtypes import is_subtype, is_more_precise, is_proper_subtype
@@ -232,6 +233,95 @@ def test_is_proper_subtype_invariance(self):
         assert_false(is_proper_subtype(fx.gb, fx.ga))
         assert_false(is_proper_subtype(fx.ga, fx.gb))
 
+    # can_be_true / can_be_false
+
+    def test_empty_tuple_always_false(self):
+        tuple_type = self.tuple()
+        assert_true(tuple_type.can_be_false)
+        assert_false(tuple_type.can_be_true)
+
+    def test_nonempty_tuple_always_true(self):
+        tuple_type = self.tuple(AnyType(), AnyType())
+        assert_true(tuple_type.can_be_true)
+        assert_false(tuple_type.can_be_false)
+
+    def test_union_can_be_true_if_any_true(self):
+        union_type = UnionType([self.fx.a, self.tuple()])
+        assert_true(union_type.can_be_true)
+
+    def test_union_can_not_be_true_if_none_true(self):
+        union_type = UnionType([self.tuple(), self.tuple()])
+        assert_false(union_type.can_be_true)
+
+    def test_union_can_be_false_if_any_false(self):
+        union_type = UnionType([self.fx.a, self.tuple()])
+        assert_true(union_type.can_be_false)
+
+    def test_union_can_not_be_false_if_none_false(self):
+        union_type = UnionType([self.tuple(self.fx.a), self.tuple(self.fx.d)])
+        assert_false(union_type.can_be_false)
+
+    # true_only / false_only
+
+    def test_true_only_of_false_type_is_uninhabited(self):
+        to = true_only(NoneTyp())
+        assert_type(UninhabitedType, to)
+
+    def test_true_only_of_true_type_is_idempotent(self):
+        always_true = self.tuple(AnyType())
+        to = true_only(always_true)
+        assert_true(always_true is to)
+
+    def test_true_only_of_instance(self):
+        to = true_only(self.fx.a)
+        assert_equal(str(to), "A")
+        assert_true(to.can_be_true)
+        assert_false(to.can_be_false)
+        assert_type(Instance, to)
+        # The original class still can be false
+        assert_true(self.fx.a.can_be_false)
+
+    def test_true_only_of_union(self):
+        tup_type = self.tuple(AnyType())
+        # Union of something that is unknown, something that is always true, something
+        # that is always false
+        union_type = UnionType([self.fx.a, tup_type, self.tuple()])
+        to = true_only(union_type)
+        assert_equal(len(to.items), 2)
+        assert_true(to.items[0].can_be_true)
+        assert_false(to.items[0].can_be_false)
+        assert_true(to.items[1] is tup_type)
+
+    def test_false_only_of_true_type_is_uninhabited(self):
+        fo = false_only(self.tuple(AnyType()))
+        assert_type(UninhabitedType, fo)
+
+    def test_false_only_of_false_type_is_idempotent(self):
+        always_false = NoneTyp()
+        fo = false_only(always_false)
+        assert_true(always_false is fo)
+
+    def test_false_only_of_instance(self):
+        fo = false_only(self.fx.a)
+        assert_equal(str(fo), "A")
+        assert_false(fo.can_be_true)
+        assert_true(fo.can_be_false)
+        assert_type(Instance, fo)
+        # The original class still can be true
+        assert_true(self.fx.a.can_be_true)
+
+    def test_false_only_of_union(self):
+        tup_type = self.tuple()
+        # Union of something that is unknown, something that is always true, something
+        # that is always false
+        union_type = UnionType([self.fx.a, self.tuple(AnyType()), tup_type])
+        assert_equal(len(union_type.items), 3)
+        fo = false_only(union_type)
+        assert_equal(len(fo.items), 2)
+        assert_false(fo.items[0].can_be_true)
+        assert_true(fo.items[0].can_be_false)
+        assert_true(fo.items[1] is tup_type)
+
     # Helpers
 
     def tuple(self, *a):
@@ -343,6 +433,22 @@ def test_any_type(self):
                   self.callable(self.fx.a, self.fx.b)]:
             self.assert_join(t, self.fx.anyt, self.fx.anyt)
 
+    def test_mixed_truth_restricted_type_simple(self):
+        # join_simple against differently restricted truthiness types drops restrictions.
+        true_a = true_only(self.fx.a)
+        false_o = false_only(self.fx.o)
+        j = join_simple(self.fx.o, true_a, false_o)
+        assert_true(j.can_be_true)
+        assert_true(j.can_be_false)
+
+    def test_mixed_truth_restricted_type(self):
+        # join_types against differently restricted truthiness types drops restrictions.
+        true_any = true_only(AnyType())
+        false_o = false_only(self.fx.o)
+        j = join_types(true_any, false_o)
+        assert_true(j.can_be_true)
+        assert_true(j.can_be_false)
+
     def test_other_mixed_types(self):
         # In general, joining unrelated types produces object.
         for t1 in [self.fx.a, self.fx.t, self.tuple(),