Fine-grained: Don't infer partial types from multiple targets (#4553)

Previously if partial types were inferred from assignments in multiple
scopes, like in the example below, fine-grained incremental mode could
display bogus messages, since only one of the scopes could be
reprocessed in one incremental update:

```
x = None

def f() -> None:
    global x
    x = ''
```

This PR fixes the issue by always inferring partial types within a single
fine-grained incremental target. This is always enabled in fine-grained
incremental mode and can also be turned on in other modes through
a new (but hidden) `--local-partial-types` option.

This was complicated by various edge cases that are covered by the 
new test cases and/or documented in comments.

If the new option is not enabled, the old behavior should (mostly) be
preserved, module some fixes that may affect also the default mode.

Work towards #4492.

Author: JukkaL

mypy/checker.py

@@ -107,6 +107,12 @@
         ('is_upper_bound', bool),  # False => precise type
     ])
 
+# Keeps track of partial types in a single scope. In fine-grained incremental
+# mode partial types initially defined at the top level cannot be completed in
+# a function, and we use the 'is_function' attribute to enforce this.
+PartialTypeScope = NamedTuple('PartialTypeScope', [('map', Dict[Var, Context]),
+                                                   ('is_function', bool)])
+
 
 class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
     """Mypy type checker.
@@ -136,7 +142,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
     # Flags; true for dynamically typed functions
     dynamic_funcs = None  # type: List[bool]
     # Stack of collections of variables with partial types
-    partial_types = None  # type: List[Dict[Var, Context]]
+    partial_types = None  # type: List[PartialTypeScope]
     # Vars for which partial type errors are already reported
     # (to avoid logically duplicate errors with different error context).
     partial_reported = None  # type: Set[Var]
@@ -632,7 +638,7 @@ def check_func_item(self, defn: FuncItem,
         self.dynamic_funcs.append(defn.is_dynamic() and not type_override)
 
         with self.errors.enter_function(fdef.name()) if fdef else nothing():
-            with self.enter_partial_types():
+            with self.enter_partial_types(is_function=True):
                 typ = self.function_type(defn)
                 if type_override:
                     typ = type_override.copy_modified(line=typ.line, column=typ.column)
@@ -1244,7 +1250,7 @@ def visit_class_def(self, defn: ClassDef) -> None:
         typ = defn.info
         if typ.is_protocol and typ.defn.type_vars:
             self.check_protocol_variance(defn)
-        with self.errors.enter_type(defn.name), self.enter_partial_types():
+        with self.errors.enter_type(defn.name), self.enter_partial_types(is_class=True):
             old_binder = self.binder
             self.binder = ConditionalTypeBinder()
             with self.binder.top_frame_context():
@@ -1487,7 +1493,7 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type
                          lvalue_type.item.type.is_protocol)):
                     self.msg.concrete_only_assign(lvalue_type, rvalue)
                     return
-                if rvalue_type and infer_lvalue_type:
+                if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType):
                     self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False)
             elif index_lvalue:
                 self.check_indexed_assignment(index_lvalue, rvalue, lvalue)
@@ -1996,7 +2002,7 @@ def infer_partial_type(self, name: Var, lvalue: Lvalue, init_type: Type) -> bool
         else:
             return False
         self.set_inferred_type(name, lvalue, partial_type)
-        self.partial_types[-1][name] = lvalue
+        self.partial_types[-1].map[name] = lvalue
         return True
 
     def set_inferred_type(self, var: Var, lvalue: Lvalue, type: Type) -> None:
@@ -3111,33 +3117,99 @@ def lookup_qualified(self, name: str) -> SymbolTableNode:
                 raise KeyError(msg.format(last, name))
 
     @contextmanager
-    def enter_partial_types(self) -> Iterator[None]:
+    def enter_partial_types(self, *, is_function: bool = False,
+                            is_class: bool = False) -> Iterator[None]:
         """Enter a new scope for collecting partial types.
 
-        Also report errors for variables which still have partial
+        Also report errors for (some) variables which still have partial
         types, i.e. we couldn't infer a complete type.
         """
-        self.partial_types.append({})
+        self.partial_types.append(PartialTypeScope({}, is_function))
         yield
 
-        partial_types = self.partial_types.pop()
+        partial_types, _ = self.partial_types.pop()
         if not self.current_node_deferred:
             for var, context in partial_types.items():
-                if isinstance(var.type, PartialType) and var.type.type is None:
-                    # None partial type: assume variable is intended to have type None
+                # If we require local partial types, there are a few exceptions where
+                # we fall back to inferring just "None" as the type from a None initaliazer:
+                #
+                # 1. If all happens within a single function this is acceptable, since only
+                #    the topmost function is a separate target in fine-grained incremental mode.
+                #    We primarily want to avoid "splitting" partial types across targets.
+                #
+                # 2. A None initializer in the class body if the attribute is defined in a base
+                #    class is fine, since the attribute is already defined and it's currently okay
+                #    to vary the type of an attribute covariantly. The None type will still be
+                #    checked for compatibility with base classes elsewhere. Without this exception
+                #    mypy could require an annotation for an attribute that already has been
+                #    declared in a base class, which would be bad.
+                allow_none = (not self.options.local_partial_types
+                              or is_function
+                              or (is_class and self.is_defined_in_base_class(var)))
+                if (allow_none
+                        and isinstance(var.type, PartialType)
+                        and var.type.type is None):
                     var.type = NoneTyp()
                 else:
                     if var not in self.partial_reported:
                         self.msg.need_annotation_for_var(var, context)
                         self.partial_reported.add(var)
+                    # Give the variable an 'Any' type to avoid generating multiple errors
+                    # from a single missing annotation.
                     var.type = AnyType(TypeOfAny.from_error)
 
+    def is_defined_in_base_class(self, var: Var) -> bool:
+        if var.info is not None:
+            for base in var.info.mro[1:]:
+                if base.get(var.name()) is not None:
+                    return True
+            if var.info.fallback_to_any:
+                return True
+        return False
+
     def find_partial_types(self, var: Var) -> Optional[Dict[Var, Context]]:
-        for partial_types in reversed(self.partial_types):
-            if var in partial_types:
-                return partial_types
+        """Look for an active partial type scope containing variable.
+
+        A scope is active if assignments in the current context can refine a partial
+        type originally defined in the scope. This is affected by the local_partial_types
+        configuration option.
+        """
+        in_scope, partial_types = self.find_partial_types_in_all_scopes(var)
+        if in_scope:
+            return partial_types
         return None
 
+    def find_partial_types_in_all_scopes(self, var: Var) -> Tuple[bool,
+                                                                  Optional[Dict[Var, Context]]]:
+        """Look for partial type scope containing variable.
+
+        Return tuple (is the scope active, scope).
+        """
+        active = self.partial_types
+        inactive = []  # type: List[PartialTypeScope]
+        if self.options.local_partial_types:
+            # All scopes within the outermost function are active. Scopes out of
+            # the outermost function are inactive to allow local reasoning (important
+            # for fine-grained incremental mode).
+            for i, t in enumerate(self.partial_types):
+                if t.is_function:
+                    active = self.partial_types[i:]
+                    inactive = self.partial_types[:i]
+                    break
+            else:
+                # Not within a function -- only the innermost scope is in scope.
+                active = self.partial_types[-1:]
+                inactive = self.partial_types[:-1]
+        # First look within in-scope partial types.
+        for scope in reversed(active):
+            if var in scope.map:
+                return True, scope.map
+        # Then for out-of-scope partial types.
+        for scope in reversed(inactive):
+            if var in scope.map:
+                return False, scope.map
+        return False, None
+
     def temp_node(self, t: Type, context: Optional[Context] = None) -> TempNode:
         """Create a temporary node with the given, fixed type."""
         temp = TempNode(t)

mypy/checkexpr.py

@@ -136,16 +136,20 @@ def analyze_ref_expr(self, e: RefExpr, lvalue: bool = False) -> Type:
             # Variable reference.
             result = self.analyze_var_ref(node, e)
             if isinstance(result, PartialType):
-                if result.type is None:
+                in_scope, partial_types = self.chk.find_partial_types_in_all_scopes(node)
+                if result.type is None and in_scope:
                     # 'None' partial type. It has a well-defined type. In an lvalue context
                     # we want to preserve the knowledge of it being a partial type.
                     if not lvalue:
                         result = NoneTyp()
                 else:
-                    partial_types = self.chk.find_partial_types(node)
                     if partial_types is not None and not self.chk.current_node_deferred:
-                        context = partial_types[node]
-                        self.msg.need_annotation_for_var(node, context)
+                        if in_scope:
+                            context = partial_types[node]
+                            self.msg.need_annotation_for_var(node, context)
+                        else:
+                            # Defer the node -- we might get a better type in the outer scope
+                            self.chk.handle_cannot_determine_type(node.name(), e)
                     result = AnyType(TypeOfAny.special_form)
         elif isinstance(node, FuncDef):
             # Reference to a global function.

mypy/dmypy_server.py

@@ -111,6 +111,9 @@ def __init__(self, flags: List[str]) -> None:
                 options.cache_fine_grained = True  # set this so that cache options match
             else:
                 options.cache_dir = os.devnull
+            # Fine-grained incremental doesn't support general partial types
+            # (details in https://github.com/python/mypy/issues/4492)
+            options.local_partial_types = True
 
     def serve(self) -> None:
         """Serve requests, synchronously (no thread or fork)."""
@@ -180,7 +183,7 @@ def cmd_stop(self) -> Dict[str, object]:
         """Stop daemon."""
         return {}
 
-    last_sources = None
+    last_sources = None  # type: List[mypy.build.BuildSource]
 
     def cmd_check(self, files: Sequence[str]) -> Dict[str, object]:
         """Check a list of files."""

mypy/main.py

@@ -370,6 +370,9 @@ def add_invertible_flag(flag: str,
     parser.add_argument('--dump-graph', action='store_true', help=argparse.SUPPRESS)
     # --semantic-analysis-only does exactly that.
     parser.add_argument('--semantic-analysis-only', action='store_true', help=argparse.SUPPRESS)
+    # --local-partial-types disallows partial types spanning module top level and a function
+    # (implicitly defined in fine-grained incremental mode)
+    parser.add_argument('--local-partial-types', action='store_true', help=argparse.SUPPRESS)
     # deprecated options
     parser.add_argument('--disallow-any', dest='special-opts:disallow_any',
                         help=argparse.SUPPRESS)

mypy/options.py

@@ -174,6 +174,8 @@ def __init__(self) -> None:
         self.show_column_numbers = False  # type: bool
         self.dump_graph = False
         self.dump_deps = False
+        # If True, partial types can't span a module top level and a function
+        self.local_partial_types = False
 
     def __eq__(self, other: object) -> bool:
         return self.__class__ == other.__class__ and self.__dict__ == other.__dict__

mypy/test/testdmypy.py

@@ -121,6 +121,7 @@ def run_case_once(self, testcase: DataDrivenTestCase, incremental_step: int) ->
             if 'fine-grained' in testcase.file:
                 server_options.append('--experimental')
                 options.fine_grained_incremental = True
+                options.local_partial_types = True
             self.server = dmypy_server.Server(server_options)  # TODO: Fix ugly API
             self.server.options = options
 

mypy/test/testfinegrained.py

@@ -142,6 +142,7 @@ def build(self,
         options.fine_grained_incremental = not build_cache
         options.use_fine_grained_cache = enable_cache and not build_cache
         options.cache_fine_grained = enable_cache
+        options.local_partial_types = True
 
         main_path = os.path.join(test_temp_dir, 'main')
         with open(main_path, 'w') as f:

test-data/unit/check-dmypy-fine-grained.test

@@ -181,3 +181,21 @@ tmp/a.py:1: error: "int" not callable
 [delete nonexistent_stub.pyi.2]
 [out1]
 [out2]
+
+[case testPartialNoneTypeFineGrainedIncremental]
+# cmd: mypy -m a b
+[file a.py]
+import b
+b.y
+x = None
+def f() -> None:
+    global x
+    x = ''
+[file b.py]
+y = 0
+[file b.py.2]
+y = ''
+[out1]
+tmp/a.py:3: error: Need type annotation for 'x'
+[out2]
+tmp/a.py:3: error: Need type annotation for 'x'