From a34af88a140d3ce670ed4545ce639f8f24970fa5 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Thu, 4 Aug 2016 18:59:07 +0100 Subject: [PATCH 01/14] Add initial definitions for can_be_true/true_only (and duals) --- mypy/types.py | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/mypy/types.py b/mypy/types.py index 59ed4f9b09e8..afbe0166264d 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -20,6 +20,8 @@ class Type(mypy.nodes.Context): """Abstract base class for all types.""" line = 0 + can_be_true = True + can_be_false = True def __init__(self, line: int = -1) -> None: self.line = line @@ -259,6 +261,7 @@ class Void(Type): the result type of calling such callable. """ + can_be_true = False source = '' # May be None; function that generated this value def __init__(self, source: str = None, line: int = -1) -> None: @@ -294,6 +297,9 @@ class UninhabitedType(Type): is_subtype(UninhabitedType, T) = True """ + can_be_true = False + can_be_false = False + def __init__(self, line: int = -1) -> None: super().__init__(line) @@ -327,6 +333,8 @@ class NoneTyp(Type): of a function, where 'None' means Void. """ + can_be_true = False + def __init__(self, is_ret_type: bool = False, line: int = -1) -> None: super().__init__(line) self.is_ret_type = is_ret_type @@ -480,6 +488,8 @@ def deserialize(cls, data: JsonDict) -> 'TypeVarType': class FunctionLike(Type): """Abstract base class for function types.""" + can_be_false = False + @abstractmethod def is_type_obj(self) -> bool: pass @@ -741,6 +751,8 @@ def __init__(self, items: List[Type], fallback: Instance, line: int = -1, self.items = items self.fallback = fallback self.implicit = implicit + self.can_be_true = len(self.items) > 0 + self.can_be_false = len(self.items) == 0 super().__init__(line) def length(self) -> int: @@ -787,6 +799,8 @@ class UnionType(Type): def __init__(self, items: List[Type], line: int = -1) -> None: self.items = items + self.can_be_true = any(item.can_be_true for item in items) + self.can_be_false = any(item.can_be_false for item in items) super().__init__(line) @staticmethod @@ -821,7 +835,7 @@ def make_simplified_union(items: List[Type], line: int = -1) -> Type: if any(is_subtype(items[i], items[j]) for j in range(len(items)) if j not in removed and j != i): removed.add(i) - + # FIXME: Union[TrueOnly(t1), FalseOnly(t1), t2, ...] == Union[t1, t2, ...] simplified_set = [items[i] for i in range(len(items)) if i not in removed] return UnionType.make_union(simplified_set) @@ -1394,3 +1408,42 @@ def is_named_instance(t: Type, fullname: str) -> bool: return (isinstance(t, Instance) and t.type is not None and t.type.fullname() == fullname) + + +def copy_type(t: Type) -> Type: + # FIXME: Find a cleaner way to copy types + return t.deserialize(t.serialize()) + + +def true_only(t: Type) -> Type: + if not t.can_be_true: + # All values of t are False-ish, so there are no true values in it + return UninhabitedType(line=t.line) + elif not t.can_be_false: + # All values of t are already True-ish, so true_only is idempotent in this case + return t + elif isinstance(t, UnionType): + # The true version of a union type is the union of the true versions of its components + new_items = [true_only(item) for item in t.items] + return UnionType.make_simplified_union(new_items, line=t.line) + else: + new_t = copy_type(t) + new_t.can_be_false = False + return new_t + + +def false_only(t: Type) -> Type: + if not t.can_be_false: + # All values of t are True-ish, so there are no false values in it + return UninhabitedType(line=t.line) + elif not t.can_be_true: + # All values of t are already False-ish, so false_only is idempotent in this case + return t + elif isinstance(t, UnionType): + # The false version of a union type is the union of the false versions of its components + new_items = [false_only(item) for item in t.items] + return UnionType.make_simplified_union(new_items, line=t.line) + else: + new_t = copy_type(t) + new_t.can_be_true = False + return new_t From b9291c4a569dffc23b9032674e0b5faf912e8f4c Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Thu, 4 Aug 2016 21:28:40 +0100 Subject: [PATCH 02/14] Test for the changes in the last commit --- mypy/test/testtypes.py | 94 +++++++++++++++++++++++++++++++++++++++++- mypy/types.py | 5 ++- 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/mypy/test/testtypes.py b/mypy/test/testtypes.py index 73154e2cd867..4bd95ec94e63 100644 --- a/mypy/test/testtypes.py +++ b/mypy/test/testtypes.py @@ -3,7 +3,7 @@ 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 @@ -11,7 +11,8 @@ 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)) + # ca_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): diff --git a/mypy/types.py b/mypy/types.py index afbe0166264d..1e32f6f7046a 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1,6 +1,7 @@ """Classes for representing mypy types.""" from abc import abstractmethod +import copy from typing import ( Any, TypeVar, Dict, List, Tuple, cast, Generic, Set, Sequence, Optional, Union ) @@ -1411,8 +1412,8 @@ def is_named_instance(t: Type, fullname: str) -> bool: def copy_type(t: Type) -> Type: - # FIXME: Find a cleaner way to copy types - return t.deserialize(t.serialize()) + # Check that this works, doesn't create aliasing + return copy.copy(t) def true_only(t: Type) -> Type: From 317140be1cb8e9d6e100a48b2343191b36bfec29 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 10:20:42 +0100 Subject: [PATCH 03/14] Make find_isinstance_check on RefExpr use true_only/false_only --- mypy/checker.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index c8db99f596d3..26128000897b 100644 --- a/mypy/checker.py +++ b/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 @@ -2444,10 +2445,14 @@ def find_isinstance_check(node: Node, 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 + # 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) + if_map = {node: if_type} if not isinstance(if_type, UninhabitedType) else None + else_map = {node: 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, From 2c715049de55039aa7efb091fcd8474522cfca06 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 11:14:55 +0100 Subject: [PATCH 04/14] Fix return type of find_isinstance_check, make tests pass again --- mypy/checker.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 26128000897b..20fb1cd667fd 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2450,8 +2450,9 @@ def find_isinstance_check(node: Node, vartype = type_map[node] if_type = true_only(vartype) else_type = false_only(vartype) - if_map = {node: if_type} if not isinstance(if_type, UninhabitedType) else None - else_map = {node: else_type} if not isinstance(else_type, UninhabitedType) else None + 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( From 6b0398382b78a7273787aa308ce3e4753264f6db Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 12:14:56 +0100 Subject: [PATCH 05/14] Update rules for checking boolean binary operators --- mypy/checkexpr.py | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 62835dda6a88..e246a702e54f 100644 --- a/mypy/checkexpr.py +++ b/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, @@ -1096,18 +1097,14 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: right_map, left_map = \ mypy.checker.find_isinstance_check(e.left, self.chk.type_map, self.chk.typing_mode_weak()) + truthiness_restriction = false_only 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] + truthiness_restriction = true_only + # type_restriction tells about the possible truth values of the left operand + # if that's the result of the boolean operation with self.chk.binder.frame_context(): if right_map: @@ -1119,13 +1116,25 @@ 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]) + + restricted_left_type = truthiness_restriction(left_type) + + if isinstance(restricted_left_type, UninhabitedType): + # The left operand can never be the result + return right_type + elif left_type is restricted_left_type: + # 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'. From 74ba9c5e6926ef3bfc05c0cd07a2ed4e9beaaaae Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 16:48:05 +0100 Subject: [PATCH 06/14] Add tests for type restrictions on if --- test-data/unit/check-expressions.test | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index 63529f4e0bac..ce5aa8670262 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -300,6 +300,28 @@ b = a and b # E: Incompatible types in assignment (expression has type "Union[A, b = b or a # E: Incompatible types in assignment (expression has type "Union[bool, A]", variable has type "bool") b = a or b # E: Incompatible types in assignment (expression has type "Union[A, bool]", variable has type "bool") class A: pass + +[builtins fixtures/bool.py] + +[case testRestrictedTypeAnd] + +b = None # type: bool +i = None # type: str +j = not b and i +if j: + s = j # type: str + + +[builtins fixtures/bool.py] + +[case testRestrictedTypeOr] + +b = None # type: bool +i = None # type: str +j = b or i +if not j: + s = j # type: str + [builtins fixtures/bool.py] [case testNonBooleanOr] From 21ab34d4d8fb6b965b4cd39c008a9c48fa839da0 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 16:48:37 +0100 Subject: [PATCH 07/14] Fix typo --- mypy/test/testtypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/test/testtypes.py b/mypy/test/testtypes.py index 4bd95ec94e63..a76174ac304d 100644 --- a/mypy/test/testtypes.py +++ b/mypy/test/testtypes.py @@ -233,7 +233,7 @@ 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)) - # ca_be_true / can_be_false + # can_be_true / can_be_false def test_empty_tuple_always_false(self): tuple_type = self.tuple() From c70b0d26439bce7bdd3ada9c1407ec0e1e857fc5 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 17:36:49 +0100 Subject: [PATCH 08/14] Modified tests with dead code These tests started to fail because the condition analyzer is now smarter and was detecting some dead code on the tests that were supposed to generate some error messages. --- test-data/unit/check-generics.test | 2 +- test-data/unit/check-modules.test | 2 +- test-data/unit/check-statements.test | 19 ++++++++----------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/test-data/unit/check-generics.test b/test-data/unit/check-generics.test index 2467d6210043..452160e9c768 100644 --- a/test-data/unit/check-generics.test +++ b/test-data/unit/check-generics.test @@ -408,7 +408,7 @@ class A(Generic[T]): if s: return p_s_s # E: Incompatible return value type (got p[S, S], expected p[S, T]) p_t_t = None # type: p[T, T] - if s: + if t: return p_t_t # E: Incompatible return value type (got p[T, T], expected p[S, T]) t = t s = s diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index d90b73451274..45c62e108b32 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -61,7 +61,7 @@ class Bad: pass [case testImportWithinBlock] import typing -if None: +if 1: import m m.a = m.b # E: Incompatible types in assignment (expression has type "B", variable has type "A") m.a = m.a diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index fa19fcf3f9b3..e4b025570ea2 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -96,15 +96,17 @@ def f() -> Iterator[int]: [case testIfStatement] a = None # type: A +a2 = None # type: A +a3 = None # type: A b = None # type: bool if a: - a = b # Fail -elif a: - a = b # Fail -elif a: - a = b # Fail + a = b # E: Incompatible types in assignment (expression has type "bool", variable has type "A") +elif a2: + a = b # E: Incompatible types in assignment (expression has type "bool", variable has type "A") +elif a3: + a = b # E: Incompatible types in assignment (expression has type "bool", variable has type "A") else: - a = b # Fail + a = b # E: Incompatible types in assignment (expression has type "bool", variable has type "A") if b: pass elif b: @@ -114,11 +116,6 @@ if b: class A: pass [builtins fixtures/bool.py] -[out] -main:5: error: Incompatible types in assignment (expression has type "bool", variable has type "A") -main:7: error: Incompatible types in assignment (expression has type "bool", variable has type "A") -main:9: error: Incompatible types in assignment (expression has type "bool", variable has type "A") -main:11: error: Incompatible types in assignment (expression has type "bool", variable has type "A") -- Loops From ce802814fe942ffcab027266f14aba9289bf71d8 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 17:50:39 +0100 Subject: [PATCH 09/14] Make unions truthiness aware --- mypy/types.py | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/mypy/types.py b/mypy/types.py index 1e32f6f7046a..a22f9273506d 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -832,11 +832,21 @@ def make_simplified_union(items: List[Type], line: int = -1) -> Type: from mypy.subtypes import is_subtype removed = set() # type: Set[int] - for i in range(len(items)): - if any(is_subtype(items[i], items[j]) for j in range(len(items)) - if j not in removed and j != i): - removed.add(i) - # FIXME: Union[TrueOnly(t1), FalseOnly(t1), t2, ...] == Union[t1, t2, ...] + for i, ti in enumerate(items): + if i in removed: continue + # Keep track of the truishness info for deleted subtypes which can be relevant + cbt = cbf = False + for j, tj in enumerate(items): + if i != j and is_subtype(tj, ti): + removed.add(j) + cbt = cbt or tj.can_be_true + cbf = cbf or tj.can_be_false + # if deleted subtypes had more general truthiness, use that + if not ti.can_be_true and cbt: + items[i] = true_or_false(ti) + elif not ti.can_be_false and cbf: + items[i] = true_or_false(ti) + simplified_set = [items[i] for i in range(len(items)) if i not in removed] return UnionType.make_union(simplified_set) @@ -1412,11 +1422,16 @@ def is_named_instance(t: Type, fullname: str) -> bool: def copy_type(t: Type) -> Type: - # Check that this works, doesn't create aliasing + """ + Build a copy of the type; used to mutate the copy with truthiness information + """ return copy.copy(t) def true_only(t: Type) -> Type: + """ + Restricted version of t with only True-ish values + """ if not t.can_be_true: # All values of t are False-ish, so there are no true values in it return UninhabitedType(line=t.line) @@ -1434,6 +1449,9 @@ def true_only(t: Type) -> Type: def false_only(t: Type) -> Type: + """ + Restricted version of t with only False-ish values + """ if not t.can_be_false: # All values of t are True-ish, so there are no false values in it return UninhabitedType(line=t.line) @@ -1448,3 +1466,18 @@ def false_only(t: Type) -> Type: new_t = copy_type(t) new_t.can_be_true = False return new_t + + +def true_or_false(t: Type) -> Type: + """ + Unrestricted version of t with both True-ish and False-ish values + + This assumes that t is already restricted more than its "natural" state. Otherwise, this may + rise an exception + """ + new_t = copy_type(t) + if not new_t.can_be_true: + del new_t.can_be_true + if not new_t.can_be_false: + del new_t.can_be_false + return new_t \ No newline at end of file From 19d0920d2f3a5561b12e79ad062a442116edbd40 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Fri, 5 Aug 2016 17:50:58 +0100 Subject: [PATCH 10/14] Enable truthiness checks independently of --strict-optional The code in find_instance_check was made specifically for None related checks, but the new rule is more general so it should be indipendent of strict optional checking. --- mypy/checker.py | 2 +- mypy/types.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 20fb1cd667fd..881f02b670ce 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2444,7 +2444,7 @@ 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: + 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] diff --git a/mypy/types.py b/mypy/types.py index a22f9273506d..19575a38b773 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1480,4 +1480,4 @@ def true_or_false(t: Type) -> Type: del new_t.can_be_true if not new_t.can_be_false: del new_t.can_be_false - return new_t \ No newline at end of file + return new_t From 9aa4799d2a1b75967a0791e9dae7b42bb027d4dd Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Tue, 16 Aug 2016 17:33:06 +0100 Subject: [PATCH 11/14] Unrestrict truthiness when joining types with mixed truthiness --- mypy/join.py | 7 ++++++- mypy/test/testtypes.py | 10 +++++++++- mypy/types.py | 7 ++----- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/mypy/join.py b/mypy/join.py index d63388d35450..e695257a6033 100644 --- a/mypy/join.py +++ b/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 diff --git a/mypy/test/testtypes.py b/mypy/test/testtypes.py index a76174ac304d..48ad6617ba4c 100644 --- a/mypy/test/testtypes.py +++ b/mypy/test/testtypes.py @@ -7,7 +7,7 @@ ) 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, @@ -433,6 +433,14 @@ 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(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_other_mixed_types(self): # In general, joining unrelated types produces object. for t1 in [self.fx.a, self.fx.t, self.tuple(), diff --git a/mypy/types.py b/mypy/types.py index 19575a38b773..a86c89440413 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1471,13 +1471,10 @@ def false_only(t: Type) -> Type: def true_or_false(t: Type) -> Type: """ Unrestricted version of t with both True-ish and False-ish values - - This assumes that t is already restricted more than its "natural" state. Otherwise, this may - rise an exception """ new_t = copy_type(t) - if not new_t.can_be_true: + if not new_t.can_be_true and type(new_t).can_be_true: del new_t.can_be_true - if not new_t.can_be_false: + if not new_t.can_be_false and type(new_t).can_be_false: del new_t.can_be_false return new_t From e53c35572fc60388f1954e58f44639fdea65850b Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Tue, 16 Aug 2016 18:03:03 +0100 Subject: [PATCH 12/14] Apply improvements suggested by @ddfisher in his review --- mypy/checkexpr.py | 14 +++++++------- test-data/unit/check-expressions.test | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index e246a702e54f..4fc9ac6e9cca 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -1093,18 +1093,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()) - truthiness_restriction = false_only + 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()) - truthiness_restriction = true_only - # type_restriction tells about the possible truth values of the left operand - # if that's the result of the boolean operation + 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: @@ -1125,12 +1127,10 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: assert right_map is not None # find_isinstance_check guarantees this return right_type - restricted_left_type = truthiness_restriction(left_type) - if isinstance(restricted_left_type, UninhabitedType): # The left operand can never be the result return right_type - elif left_type is restricted_left_type: + elif result_is_left: # The left operand is always the result return left_type else: diff --git a/test-data/unit/check-expressions.test b/test-data/unit/check-expressions.test index ce5aa8670262..9f89ba3dcae9 100644 --- a/test-data/unit/check-expressions.test +++ b/test-data/unit/check-expressions.test @@ -309,7 +309,7 @@ b = None # type: bool i = None # type: str j = not b and i if j: - s = j # type: str + reveal_type(j) # E: Revealed type is 'builtins.str' [builtins fixtures/bool.py] @@ -320,7 +320,7 @@ b = None # type: bool i = None # type: str j = b or i if not j: - s = j # type: str + reveal_type(j) # E: Revealed type is 'builtins.str' [builtins fixtures/bool.py] From 805357942014d330a81e0542d15afdb8bd49e0dd Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Tue, 16 Aug 2016 20:47:44 +0100 Subject: [PATCH 13/14] Also check join_types for mixed truthiness --- mypy/join.py | 5 +++++ mypy/test/testtypes.py | 12 ++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/mypy/join.py b/mypy/join.py index 006cc6d5eb4a..b5de3fd1bafe 100644 --- a/mypy/join.py +++ b/mypy/join.py @@ -65,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 diff --git a/mypy/test/testtypes.py b/mypy/test/testtypes.py index 48ad6617ba4c..b2b6d0ab438a 100644 --- a/mypy/test/testtypes.py +++ b/mypy/test/testtypes.py @@ -433,14 +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(self): - # Join_simple against differently restricted truthiness types drops restrictions. + 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(), From b364cdbcb3c1c2db9765dd55f2b835b26d0aae18 Mon Sep 17 00:00:00 2001 From: Daniel F Moisset Date: Tue, 16 Aug 2016 20:48:10 +0100 Subject: [PATCH 14/14] Simplify true_only as suggested by @ddfisher --- mypy/types.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mypy/types.py b/mypy/types.py index a86c89440413..d582079732b9 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1473,8 +1473,6 @@ def true_or_false(t: Type) -> Type: Unrestricted version of t with both True-ish and False-ish values """ new_t = copy_type(t) - if not new_t.can_be_true and type(new_t).can_be_true: - del new_t.can_be_true - if not new_t.can_be_false and type(new_t).can_be_false: - del new_t.can_be_false + new_t.can_be_true = type(new_t).can_be_true + new_t.can_be_false = type(new_t).can_be_false return new_t