Skip to content

Fix for #1698 #1989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Aug 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 23 additions & 14 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to have an assert False, "Unknown boolean operator" instead of killing the else completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm adding an assertion about e.op at the beginning (which is assured by visit_op_expr)

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:
Expand All @@ -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'.
Expand Down
12 changes: 11 additions & 1 deletion mypy/join.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need this for join_types also, right?

Copy link
Contributor Author

@dmoisset dmoisset Aug 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to test and couldn't make a failing case with join_type; I think
that one always creates new instances and new instances have always
undetermined boolean values. The problem with join_simple is that it was
reusing the existing types

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

join_types also reuses existing values in some cases. I'd expect the same problem to exist, but just be pretty hard to actually repro.

# 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

Expand Down Expand Up @@ -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

Expand Down
112 changes: 109 additions & 3 deletions mypy/test/testtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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(),
Expand Down
Loading