Fine-grained: Compare symbol table snapshots when following dependencies (#4598)

Previously we just compared symbol table node types, which would fail to
propagate certain changes. For example, we could fail to detect a change
from a module reference to a normal variable.

This only affects detecting changes when we are propagating changes using
fine-grained dependencies. Snapshot comparison was already being used
when processing an entire module.

Fixes #4595.

Author: JukkaL

mypy/server/astdiff.py

@@ -244,7 +244,7 @@ def snapshot_symbol_table(name_prefix: str, table: SymbolTable) -> Dict[str, Sna
 
     Only "shallow" state is included in the snapshot -- references to
     things defined in other modules are represented just by the names of
-    the targers.
+    the targets.
     """
     result = {}  # type: Dict[str, SnapshotItem]
     for name, symbol in table.items():

mypy/server/update.py

@@ -817,6 +817,7 @@ def reprocess_nodes(manager: BuildManager,
     file_node = manager.modules[module_id]
     old_symbols = find_symbol_tables_recursive(file_node.fullname(), file_node.names)
     old_symbols = {name: names.copy() for name, names in old_symbols.items()}
+    old_symbols_snapshot = snapshot_symbol_table(file_node.fullname(), file_node.names)
 
     def key(node: DeferredNode) -> int:
         # Unlike modules which are sorted by name within SCC,
@@ -829,9 +830,6 @@ def key(node: DeferredNode) -> int:
     # TODO: ignore_all argument to set_file_ignored_lines
     manager.errors.set_file_ignored_lines(file_node.path, file_node.ignored_lines)
 
-    # Keep track of potentially affected attribute types before type checking.
-    old_types_map = get_enclosing_namespace_types(nodes)
-
     # Strip semantic analysis information.
     for deferred in nodes:
         strip_target(deferred.node)
@@ -878,8 +876,12 @@ def key(node: DeferredNode) -> int:
         if graph[module_id].type_checker().check_second_pass():
             more = True
 
+    new_symbols_snapshot = snapshot_symbol_table(file_node.fullname(), file_node.names)
     # Check if any attribute types were changed and need to be propagated further.
-    new_triggered = get_triggered_namespace_items(old_types_map)
+    changed = compare_symbol_table_snapshots(file_node.fullname(),
+                                             old_symbols_snapshot,
+                                             new_symbols_snapshot)
+    new_triggered = {make_trigger(name) for name in changed}
 
     # Dependencies may have changed.
     update_deps(module_id, nodes, graph, deps, manager.options)
@@ -909,40 +911,6 @@ def find_symbol_tables_recursive(prefix: str, symbols: SymbolTable) -> Dict[str,
     return result
 
 
-NamespaceNode = Union[TypeInfo, MypyFile]
-
-
-def get_enclosing_namespace_types(nodes: List[DeferredNode]) -> Dict[NamespaceNode,
-                                                                     Dict[str, Type]]:
-    types = {}  # type: Dict[NamespaceNode, Dict[str, Type]]
-    for deferred in nodes:
-        info = deferred.active_typeinfo
-        if info:
-            target = info  # type: Optional[NamespaceNode]
-        elif isinstance(deferred.node, MypyFile):
-            target = deferred.node
-        else:
-            target = None
-        if target and target not in types:
-            local_types = {name: node.node.type
-                         for name, node in target.names.items()
-                         if isinstance(node.node, Var) and node.node.type}
-            types[target] = local_types
-    return types
-
-
-def get_triggered_namespace_items(old_types_map: Dict[NamespaceNode, Dict[str, Type]]) -> Set[str]:
-    new_triggered = set()
-    for namespace_node, old_types in old_types_map.items():
-        for name, node in namespace_node.names.items():
-            if (name in old_types and
-                    (not isinstance(node.node, Var) or
-                     node.node.type and not is_identical_type(node.node.type, old_types[name]))):
-                # Type checking a method changed an attribute type.
-                new_triggered.add(make_trigger('{}.{}'.format(namespace_node.fullname(), name)))
-    return new_triggered
-
-
 def update_deps(module_id: str,
                 nodes: List[DeferredNode],
                 graph: Dict[str, State],

test-data/unit/fine-grained-modules.test

@@ -1,7 +1,7 @@
 -- Test cases for fine-grained incremental mode related to modules
 --
--- Covers adding and deleting modules, changes to multiple modules, and
--- changes to import graph.
+-- Covers adding and deleting modules, changes to multiple modules,
+-- changes to import graph, and changes to module references.
 --
 -- The comments in fine-grained.test explain how these tests work.
 
@@ -808,3 +808,41 @@ def main() -> None:
 [file config.py]
 [out]
 ==
+
+
+-- Misc
+-- ----
+
+
+[case testChangeModuleToVariable]
+from a import m
+m.x
+[file a.py]
+from b import m
+[file b.py]
+import m
+[file b.py.2]
+m = ''
+[file m.py]
+x = 1
+[file m2.py]
+[out]
+==
+main:2: error: "str" has no attribute "x"
+
+[case testChangeVariableToModule]
+from a import m
+y: str = m
+[file a.py]
+from b import m
+[file b.py]
+m = ''
+[file b.py.2]
+import m
+[file m.py]
+x = 1
+[file m2.py]
+[builtins fixtures/module.pyi]
+[out]
+==
+main:2: error: Incompatible types in assignment (expression has type Module, variable has type "str")