Turn DeletedType into a status in the type binder Deleted

This got a little bit more complicated than expected, and might be
considered optional. I like having all the logic about deleted
variables in one place, though.

Author: rwbarton

mypy/binder.py

@@ -1,4 +1,4 @@
-from typing import (Dict, List, Set, Iterator)
+from typing import (Dict, List, Set, Iterator, Union, cast)
 from contextlib import contextmanager
 
 from mypy.types import Type, AnyType, PartialType
@@ -9,7 +9,22 @@
 from mypy.sametypes import is_same_type
 
 
-class Frame(Dict[Key, Type]):
+class Deleted:
+    """Frame entry representing a deleted variable."""
+
+    source = ''   # May be None; name that generated this value
+
+    def __init__(self, source: str, as_lvalue: bool = False) -> None:
+        self.source = source
+        self.as_lvalue = as_lvalue
+
+
+# What's known about an expression at a specific moment:
+# either its current type, or that it has been deleted.
+Status = Union[Type, Deleted]
+
+
+class Frame(Dict[Key, Status]):
     """A Frame represents a specific point in the execution of a program.
     It carries information about the current types of expressions at
     that point, arising either from assignments to those expressions
@@ -64,7 +79,7 @@ def __init__(self) -> None:
 
         # Maps expr.literal_hash] to get_declaration(expr)
         # for every expr stored in the binder
-        self.declarations = Frame()
+        self.declarations = {}  # type: Dict[Key, Type]
         # Set of other keys to invalidate if a key is changed, e.g. x -> {x.a, x[0]}
         # Whenever a new key (e.g. x.a.b) is added, we update this
         self.dependencies = {}  # type: Dict[Key, Set[Key]]
@@ -92,18 +107,18 @@ def push_frame(self) -> Frame:
         self.options_on_return.append([])
         return f
 
-    def _push(self, key: Key, type: Type, index: int=-1) -> None:
+    def _push(self, key: Key, type: Status, index: int=-1) -> None:
         self.frames[index][key] = type
 
-    def _get(self, key: Key, index: int=-1) -> Type:
+    def _get(self, key: Key, index: int=-1) -> Status:
         if index < 0:
             index += len(self.frames)
         for i in range(index, -1, -1):
             if key in self.frames[i]:
                 return self.frames[i][key]
         return None
 
-    def push(self, expr: Node, typ: Type) -> None:
+    def push(self, expr: Node, typ: Status) -> None:
         if not expr.literal:
             return
         key = expr.literal_hash
@@ -112,12 +127,23 @@ def push(self, expr: Node, typ: Type) -> None:
             self._add_dependencies(key)
         self._push(key, typ)
 
+    def not_deleted(self, expr: Node) -> None:
+        if not expr.literal:
+            return
+        if isinstance(self.get(expr), Deleted):
+            self.push(expr, self.get_declaration(expr))
+
     def unreachable(self) -> None:
         self.frames[-1].unreachable = True
 
-    def get(self, expr: Node) -> Type:
+    def get(self, expr: Node) -> Status:
         return self._get(expr.literal_hash)
 
+    def get_lvalue(self, expr: Node) -> Status:
+        if expr.literal:
+            return self.declarations.get(expr.literal_hash)
+        return None
+
     def is_unreachable(self) -> bool:
         # TODO: Copy the value of unreachable into new frames to avoid
         # this traversal on every statement?
@@ -148,21 +174,28 @@ def update_from_options(self, frames: List[Frame]) -> bool:
         for key in keys:
             current_value = self._get(key)
             resulting_values = [f.get(key, current_value) for f in frames]
+
             if any(x is None for x in resulting_values):
                 # We didn't know anything about key before
                 # (current_value must be None), and we still don't
                 # know anything about key in at least one possible frame.
+                # It might be deleted though.
+                deleted_values = [x for x in resulting_values if isinstance(x, Deleted)]
+                if deleted_values:
+                    self._push(key, deleted_values[0])
+                    changed = True
                 continue
 
-            if isinstance(self.declarations.get(key), AnyType):
+            declaration = self.declarations.get(key)
+            if isinstance(declaration, AnyType):
                 type = resulting_values[0]
-                if not all(is_same_type(type, t) for t in resulting_values[1:]):
+                if not all(is_same_status(type, t) for t in resulting_values[1:]):
                     type = AnyType()
             else:
                 type = resulting_values[0]
                 for other in resulting_values[1:]:
-                    type = join_simple(self.declarations[key], type, other)
-            if not is_same_type(type, current_value):
+                    type = join_status(declaration, type, other)
+            if not is_same_status(type, current_value):
                 self._push(key, type)
                 changed = True
 
@@ -199,7 +232,7 @@ def get_declaration(self, expr: Node) -> Type:
             return None
 
     def assign_type(self, expr: Node,
-                    type: Type,
+                    type: Status,
                     declared_type: Type,
                     restrict_any: bool = False) -> None:
         if not expr.literal:
@@ -210,7 +243,7 @@ def assign_type(self, expr: Node,
             # Not sure why this happens.  It seems to mainly happen in
             # member initialization.
             return
-        if not is_subtype(type, declared_type):
+        if isinstance(type, Type) and not is_subtype(type, declared_type):
             # Pretty sure this is only happens when there's a type error.
 
             # Ideally this function wouldn't be called if the
@@ -247,13 +280,13 @@ def invalidate_dependencies(self, expr: Node) -> None:
         for dep in self.dependencies.get(expr.literal_hash, set()):
             self._cleanse_key(dep)
 
-    def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type:
+    def most_recent_enclosing_type(self, expr: Node, type: Status) -> Status:
         if isinstance(type, AnyType):
             return self.get_declaration(expr)
         key = expr.literal_hash
-        enclosers = ([self.get_declaration(expr)] +
+        enclosers = ([cast(Status, self.get_declaration(expr))] +
                      [f[key] for f in self.frames
-                      if key in f and is_subtype(type, f[key])])
+                      if key in f])
         return enclosers[-1]
 
     def allow_jump(self, index: int) -> None:
@@ -335,3 +368,20 @@ def top_frame_context(self) -> Iterator[Frame]:
         assert len(self.frames) == 1
         yield self.push_frame()
         self.pop_frame(True, 0)
+
+
+def join_status(declaration: Type, s: Status, t: Status) -> Status:
+    if isinstance(s, Deleted):
+        return s
+
+    if isinstance(t, Deleted):
+        return t
+
+    return join_simple(declaration, s, t)
+
+
+def is_same_status(s: Status, t: Status) -> bool:
+    if isinstance(s, Deleted) or isinstance(t, Deleted):
+        return isinstance(s, Deleted) and isinstance(t, Deleted) and s.as_lvalue == t.as_lvalue
+
+    return is_same_type(s, t)

mypy/checker.py

@@ -52,7 +52,7 @@
 from mypy.join import join_types
 from mypy.treetransform import TransformVisitor
 from mypy.meet import meet_simple, nearest_builtin_ancestor, is_overlapping_types
-from mypy.binder import ConditionalTypeBinder
+from mypy.binder import ConditionalTypeBinder, Deleted
 
 from mypy import experiments
 
@@ -972,6 +972,7 @@ def check_assignment(self, lvalue: Node, rvalue: Node, infer_lvalue_type: bool =
                                                       infer_lvalue_type)
         else:
             lvalue_type, index_lvalue, inferred = self.check_lvalue(lvalue)
+            self.binder.not_deleted(lvalue)
             if lvalue_type:
                 if isinstance(lvalue_type, PartialType) and lvalue_type.type is None:
                     # Try to infer a proper type for a variable with a partial None type.
@@ -1211,6 +1212,10 @@ def check_lvalue(self, lvalue: Node) -> Tuple[Type, IndexExpr, Var]:
             self.store_type(lvalue, lvalue_type)
         elif isinstance(lvalue, NameExpr):
             lvalue_type = self.expr_checker.analyze_ref_expr(lvalue, lvalue=True)
+            binder_type = self.binder.get(lvalue)
+            if isinstance(binder_type, Deleted) and binder_type.as_lvalue:
+                self.msg.deleted_as_lvalue(binder_type.source, lvalue)
+                lvalue_type = AnyType()  # Don't produce a subsequent type error also.
             self.store_type(lvalue, lvalue_type)
         elif isinstance(lvalue, TupleExpr) or isinstance(lvalue, ListExpr):
             lv = cast(Union[TupleExpr, ListExpr], lvalue)
@@ -1246,8 +1251,6 @@ def infer_variable_type(self, name: Var, lvalue: Node,
         elif isinstance(init_type, Void):
             self.check_not_void(init_type, context)
             self.set_inference_error_fallback_type(name, lvalue, init_type, context)
-        elif isinstance(init_type, DeletedType):
-            self.msg.deleted_as_rvalue(init_type, context)
         elif not is_valid_inferred_type(init_type):
             # We cannot use the type of the initialization expression for full type
             # inference (it's not specific enough), but we might be able to give
@@ -1312,6 +1315,10 @@ def narrow_type_from_binder(self, expr: Node, known_type: Type) -> Type:
         if expr.literal >= LITERAL_TYPE:
             restriction = self.binder.get(expr)
             if restriction:
+                if isinstance(restriction, Deleted):
+                    self.msg.deleted_as_rvalue(restriction.source, expr)
+                    return AnyType()  # Don't produce a subsequent type error also.
+
                 ans = meet_simple(known_type, restriction)
                 return ans
         return known_type
@@ -1326,16 +1333,11 @@ def check_simple_assignment(self, lvalue_type: Type, rvalue: Node,
             return AnyType()
         else:
             rvalue_type = self.accept(rvalue, lvalue_type)
-            if isinstance(rvalue_type, DeletedType):
-                self.msg.deleted_as_rvalue(rvalue_type, context)
             if self.typing_mode_weak():
                 return rvalue_type
-            if isinstance(lvalue_type, DeletedType):
-                self.msg.deleted_as_lvalue(lvalue_type, context)
-            else:
-                self.check_subtype(rvalue_type, lvalue_type, context, msg,
-                                   '{} has type'.format(rvalue_name),
-                                   '{} has type'.format(lvalue_name))
+            self.check_subtype(rvalue_type, lvalue_type, context, msg,
+                               '{} has type'.format(rvalue_name),
+                               '{} has type'.format(lvalue_name))
             return rvalue_type
 
     def check_indexed_assignment(self, lvalue: IndexExpr,
@@ -1597,6 +1599,7 @@ def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None:
                                 # causing assignment to set the variable's type.
                                 s.vars[i].is_def = True
                                 self.check_assignment(s.vars[i], self.temp_node(t, s.vars[i]))
+                                self.binder.push(s.vars[i], t)
                         self.accept(s.handlers[i])
                         if s.vars[i]:
                             # Exception variables are deleted in python 3 but not python 2.
@@ -1611,9 +1614,10 @@ def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None:
                                 source = ('(exception variable "{}", which we do not '
                                           'accept outside except: blocks even in '
                                           'python 2)'.format(s.vars[i].name))
-                            var = cast(Var, s.vars[i].node)
-                            var.type = DeletedType(source=source)
-                            self.binder.cleanse(s.vars[i])
+                            self.binder.assign_type(s.vars[i],
+                                                    Deleted(source=source, as_lvalue=True),
+                                                    self.binder.get_declaration(s.vars[i]),
+                                                    self.typing_mode_weak())
             if s.else_body:
                 self.accept(s.else_body)
 
@@ -1711,7 +1715,7 @@ def flatten(t: Node) -> List[Node]:
             for elt in flatten(s.expr):
                 if isinstance(elt, NameExpr):
                     self.binder.assign_type(elt,
-                                            DeletedType(source=elt.name),
+                                            Deleted(source=elt.name),
                                             self.binder.get_declaration(elt),
                                             self.typing_mode_weak())
             return None

mypy/checkexpr.py

@@ -121,12 +121,7 @@ def analyze_var_ref(self, var: Var, context: Context) -> Type:
             # Implicit 'Any' type.
             return AnyType()
         else:
-            # Look up local type of variable with type (inferred or explicit).
-            val = self.chk.binder.get(var)
-            if val is None:
-                return var.type
-            else:
-                return val
+            return var.type
 
     def visit_call_expr(self, e: CallExpr) -> Type:
         """Type check a call expression."""
@@ -691,8 +686,6 @@ def check_arg(self, caller_type: Type, original_caller_type: Type,
         """Check the type of a single argument in a call."""
         if isinstance(caller_type, Void):
             messages.does_not_return_value(caller_type, context)
-        elif isinstance(caller_type, DeletedType):
-            messages.deleted_as_rvalue(caller_type, context)
         elif not is_subtype(caller_type, callee_type):
             messages.incompatible_argument(n, m, callee, original_caller_type,
                                            caller_kind, context)

mypy/checkmember.py

@@ -110,9 +110,6 @@ def analyze_member_access(name: str, typ: Type, node: Context, is_lvalue: bool,
         return analyze_member_access(name, typ.upper_bound, node, is_lvalue, is_super,
                                      builtin_type, not_ready_callback, msg,
                                      report_type=report_type)
-    elif isinstance(typ, DeletedType):
-        msg.deleted_as_rvalue(typ, node)
-        return AnyType()
     elif isinstance(typ, TypeType):
         # Similar to FunctionLike + is_type_obj() above.
         item = None

mypy/messages.py

@@ -562,24 +562,24 @@ def does_not_return_value(self, void_type: Type, context: Context) -> None:
             self.fail('{} does not return a value'.format(
                 capitalize((cast(Void, void_type)).source)), context)
 
-    def deleted_as_rvalue(self, typ: DeletedType, context: Context) -> None:
+    def deleted_as_rvalue(self, source: str, context: Context) -> None:
         """Report an error about using an deleted type as an rvalue."""
-        if typ.source is None:
+        if source is None:
             s = ""
         else:
-            s = " '{}'".format(typ.source)
+            s = " '{}'".format(source)
         self.fail('Trying to read deleted variable{}'.format(s), context)
 
-    def deleted_as_lvalue(self, typ: DeletedType, context: Context) -> None:
+    def deleted_as_lvalue(self, source: str, context: Context) -> None:
         """Report an error about using an deleted type as an lvalue.
 
         Currently, this only occurs when trying to assign to an
         exception variable outside the local except: blocks.
         """
-        if typ.source is None:
+        if source is None:
             s = ""
         else:
-            s = " '{}'".format(typ.source)
+            s = " '{}'".format(source)
         self.fail('Assignment to variable{} outside except: block'.format(s), context)
 
     def no_variant_matches_arguments(self, overload: Overloaded, arg_types: List[Type],

test-data/unit/check-statements.test

@@ -656,7 +656,7 @@ e = 1
 try: pass
 except E1 as e: pass
 e = 1 # E: Assignment to variable 'e' outside except: block
-e = E1() # E: Assignment to variable 'e' outside except: block
+e = E1()
 [builtins fixtures/exception.py]
 
 [case testArbitraryExpressionAsExceptionType]