From 42b6160e3f9562288887d3b0015130d4e343af97 Mon Sep 17 00:00:00 2001 From: Jared Hance Date: Fri, 9 Oct 2015 02:23:01 -0400 Subject: [PATCH] Support multiple isinstance in a single if. This also squashes a minor weirdness where in order to infer isinstance for resolution further in an and-expr, we were parsing for isinstance in the entire expression! Overall this is pretty self-explanatory. We get rid of the 'kind' stuff and just return a tuple of optional dicts. Fixes #900. --- mypy/checker.py | 105 +++++++++++++-------------- mypy/checkexpr.py | 54 ++++++++------ mypy/test/data/check-isinstance.test | 17 +++++ 3 files changed, 100 insertions(+), 76 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index c8df7d489550..f1eed500be35 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2,7 +2,9 @@ import itertools -from typing import Any, Dict, Set, List, cast, Tuple, Callable, TypeVar, Union +from typing import ( + Any, Dict, Set, List, cast, Tuple, Callable, TypeVar, Union, Optional +) from mypy.errors import Errors from mypy.nodes import ( @@ -45,11 +47,6 @@ from mypy.meet import meet_simple, meet_simple_away, nearest_builtin_ancestor, is_overlapping_types -# Kinds of isinstance checks. -ISINSTANCE_OVERLAPPING = 0 -ISINSTANCE_ALWAYS_TRUE = 1 -ISINSTANCE_ALWAYS_FALSE = 2 - T = TypeVar('T') @@ -1433,16 +1430,21 @@ def visit_if_stmt(self, s: IfStmt) -> Type: for e, b in zip(s.expr, s.body): t = self.accept(e) self.check_not_void(t, e) - var, type, elsetype, kind = find_isinstance_check(e, self.type_map, - self.typing_mode_weak()) - if kind == ISINSTANCE_ALWAYS_FALSE: + if_map, else_map = find_isinstance_check( + e, self.type_map, + self.typing_mode_weak() + ) + if if_map is None: + # The condition is always false # XXX should issue a warning? pass else: # Only type check body if the if condition can be true. self.binder.push_frame() - if var: - self.binder.push(var, type) + if if_map: + for var, type in if_map.items(): + self.binder.push(var, type) + self.accept(b) _, frame = self.binder.pop_frame() if not self.breaking_out: @@ -1451,9 +1453,10 @@ def visit_if_stmt(self, s: IfStmt) -> Type: self.breaking_out = False - if var: - self.binder.push(var, elsetype) - if kind == ISINSTANCE_ALWAYS_TRUE: + if else_map: + for var, type in else_map.items(): + self.binder.push(var, type) + if else_map is None: # The condition is always true => remaining elif/else blocks # can never be reached. @@ -2046,24 +2049,17 @@ def map_type_from_supertype(typ: Type, sub_info: TypeInfo, def find_isinstance_check(node: Node, type_map: Dict[Node, Type], - weak: bool=False) -> Tuple[Node, Type, Type, int]: - """Check if node is an isinstance(variable, type) check. - - If successful, return tuple (variable, target-type, else-type, - kind); otherwise, return (None, AnyType, AnyType, -1). + weak: bool=False) \ + -> Tuple[Optional[Dict[Node, Type]], Optional[Dict[Node, Type]]]: + """Find any isinstance checks (within a chain of ands)". - When successful, the kind takes one of these values: + Return value is a map of variables to their types if the condition + is true and a map of variables to their types if the condition is false. - ISINSTANCE_OVERLAPPING: The type of variable and the target type are - partially overlapping => the test result can be True or False. - ISINSTANCE_ALWAYS_TRUE: The target type at least as general as the - variable type => the test is always True. - ISINSTANCE_ALWAYS_FALSE: The target type and the variable type are not - overlapping => the test is always False. + If either of the values in the tuple is None, then that particular + branch can never occur. - If it is an isinstance check, but we don't understand the argument - type, then in weak mode it is treated as Any and in non-weak mode - it is not treated as an isinstance. + Guaranteed to not return None, None. (But may return {}, {}) """ if isinstance(node, CallExpr): if refers_to_fullname(node.callee, 'builtins.isinstance'): @@ -2072,46 +2068,45 @@ def find_isinstance_check(node: Node, vartype = type_map[expr] type = get_isinstance_type(node.args[1], type_map) if type: - kind = ISINSTANCE_OVERLAPPING elsetype = vartype if vartype: if is_proper_subtype(vartype, type): - kind = ISINSTANCE_ALWAYS_TRUE elsetype = None + return {expr: type}, None elif not is_overlapping_types(vartype, type): - kind = ISINSTANCE_ALWAYS_FALSE + return None, {expr: elsetype} else: elsetype = restrict_subtype_away(vartype, type) - return expr, type, elsetype, kind + return {expr: type}, {expr: elsetype} else: # An isinstance check, but we don't understand the type if weak: - return expr, AnyType(), vartype, ISINSTANCE_OVERLAPPING + return {expr: AnyType()}, {expr: vartype} elif isinstance(node, OpExpr) and node.op == 'and': - # XXX We should extend this to support two isinstance checks in the same - # expression - (var, type, elsetype, kind) = find_isinstance_check(node.left, type_map, weak) - if var is None: - (var, type, elsetype, kind) = find_isinstance_check(node.left, type_map, weak) - if var: - if kind == ISINSTANCE_ALWAYS_TRUE: - kind = ISINSTANCE_OVERLAPPING - return (var, type, AnyType(), kind) + left_if_vars, right_else_vars = find_isinstance_check( + node.left, + type_map, + weak, + ) + + right_if_vars, right_else_vars = find_isinstance_check( + node.right, + type_map, + weak, + ) + if left_if_vars: + left_if_vars.update(right_if_vars) + else: + left_if_vars = right_if_vars + + # Make no claim about the types in else + return left_if_vars, {} elif isinstance(node, UnaryExpr) and node.op == 'not': - (var, type, elsetype, kind) = find_isinstance_check(node.expr, type_map, weak) - return (var, elsetype, type, invert_isinstance_kind(kind)) + left, right = find_isinstance_check(node.expr, type_map, weak) + return right, left # Not a supported isinstance check - return None, AnyType(), AnyType(), -1 - - -def invert_isinstance_kind(kind: int) -> int: - if kind == ISINSTANCE_ALWAYS_TRUE: - return ISINSTANCE_ALWAYS_FALSE - elif kind == ISINSTANCE_ALWAYS_FALSE: - return ISINSTANCE_ALWAYS_TRUE - else: - return kind + return {}, {} def get_isinstance_type(node: Node, type_map: Dict[Node, Type]) -> Type: diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 231a05c8df66..fee3e1bf58f8 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -911,17 +911,21 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: left_type = self.accept(e.left, ctx) if e.op == 'and': - var, type, elsetype, kind = \ - mypy.checker.find_isinstance_check(e, self.chk.type_map, + # else_map unused + if_map, else_map = \ + mypy.checker.find_isinstance_check(e.left, self.chk.type_map, self.chk.typing_mode_weak()) else: - var = None - if var: - self.chk.binder.push_frame() - self.chk.binder.push(var, type) + if_map = None + + self.chk.binder.push_frame() + if if_map: + for var, type in if_map.items(): + self.chk.binder.push(var, type) + right_type = self.accept(e.right, left_type) - if var: - self.chk.binder.pop_frame() + + self.chk.binder.pop_frame() self.check_not_void(left_type, context) self.check_not_void(right_type, context) @@ -1278,22 +1282,30 @@ def visit_conditional_expr(self, e: ConditionalExpr) -> Type: # Gain type information from isinstance if it is there # but only for the current expression - variable, inst_if_type, inst_else_type, kind = mypy.checker.find_isinstance_check( + if_map, else_map = mypy.checker.find_isinstance_check( e.cond, self.chk.type_map, self.chk.typing_mode_weak()) - if variable: - self.chk.binder.push_frame() - self.chk.binder.push(variable, inst_if_type) - if_type = self.accept(e.if_expr) - self.chk.binder.pop_frame() - self.chk.binder.push_frame() - self.chk.binder.push(variable, inst_else_type) - else_type = self.accept(e.else_expr, context=if_type) - self.chk.binder.pop_frame() - else: - if_type = self.accept(e.if_expr) - else_type = self.accept(e.else_expr, context=if_type) + + self.chk.binder.push_frame() + + if if_map: + for var, type in if_map.items(): + self.chk.binder.push(var, type) + + if_type = self.accept(e.if_expr) + + self.chk.binder.pop_frame() + self.chk.binder.push_frame() + + if else_map: + for var, type in else_map.items(): + self.chk.binder.push(var, type) + + else_type = self.accept(e.else_expr, context=if_type) + + self.chk.binder.pop_frame() + return join.join_types(if_type, else_type) # diff --git a/mypy/test/data/check-isinstance.test b/mypy/test/data/check-isinstance.test index fed0cf55d84b..97f95cb0bdf3 100644 --- a/mypy/test/data/check-isinstance.test +++ b/mypy/test/data/check-isinstance.test @@ -721,3 +721,20 @@ x.flag if isinstance(x, B) else 0 0 if not isinstance(x, B) else x.flag 0 if isinstance(x, B) else x.flag # E: "A" has no attribute "flag" [builtins fixtures/isinstancelist.py] +[case testIsinstanceMultiAnd] +class A: + pass + +class B(A): + flag = 1 + +class C(A): + glaf = 1 + +x = B() # type: A +y = C() # type: A + +if isinstance(x, B) and isinstance(y, C): + x.flag += 1 + y.glaf += 1 +[builtins fixtures/isinstancelist.py]