Fix incremental speed regression (#4331)

Fixes #4135. We now decide more carefully whether a module has an error -- only if there's an error for that specific file or for any of its (transitive) dependencies (including the SCC it's a member of). Previously, once an error was detected, *all* modules processed later were considered to be at risk.

Author: gvanrossum

mypy/build.py

@@ -1474,6 +1474,9 @@ class State:
     # Whether to ignore all errors
     ignore_all = False
 
+    # Whether the module has an error or any of its dependencies have one.
+    transitive_error = False
+
     # Type checker used for checking this file.  Use type_checker() for
     # access and to construct this on demand.
     _type_checker = None  # type: Optional[TypeChecker]
@@ -1944,7 +1947,7 @@ def write_cache(self) -> None:
         if self.manager.options.quick_and_dirty:
             is_errors = self.manager.errors.is_errors_for_file(self.path)
         else:
-            is_errors = self.manager.errors.is_errors()
+            is_errors = self.transitive_error
         if is_errors:
             delete_cache(self.id, self.path, self.manager)
             self.meta = None
@@ -2257,6 +2260,12 @@ def process_graph(graph: Graph, manager: BuildManager) -> None:
         else:
             fresh_msg = "stale due to deps (%s)" % " ".join(sorted(stale_deps))
 
+        # Initialize transitive_error for all SCC members from union
+        # of transitive_error of dependencies.
+        if any(graph[dep].transitive_error for dep in deps if dep in graph):
+            for id in scc:
+                graph[id].transitive_error = True
+
         scc_str = " ".join(scc)
         if fresh:
             if not maybe_reuse_in_memory_tree(graph, scc, manager):
@@ -2272,6 +2281,11 @@ def process_graph(graph: Graph, manager: BuildManager) -> None:
                 # single fresh SCC. This is intentional -- we don't need those modules
                 # loaded if there are no more stale SCCs to be rechecked.
                 #
+                # Also note we shouldn't have to worry about transitive_error here,
+                # since modules with transitive errors aren't written to the cache,
+                # and if any dependencies were changed, this SCC would be stale.
+                # (Also, in quick_and_dirty mode we don't care about transitive errors.)
+                #
                 # TODO: see if it's possible to determine if we need to process only a
                 # _subset_ of the past SCCs instead of having to process them all.
                 for prev_scc in fresh_scc_queue:
@@ -2416,6 +2430,7 @@ def can_reuse_in_memory_tree(graph: Graph, scc: List[str], manager: BuildManager
         # Check that all dependencies were loaded from memory.
         # If not, some dependency was reparsed but the interface hash
         # wasn't changed -- in that case we can't reuse the tree.
+        # TODO: Pass deps in from process_graph(), via maybe_reuse_in_memory_tree()?
         deps = set(dep for id in scc for dep in graph[id].dependencies if dep in graph)
         deps -= set(scc)  # Subtract the SCC itself (else nothing will be safe)
         if all(graph[dep].is_from_saved_cache for dep in deps):
@@ -2464,6 +2479,9 @@ def process_stale_scc(graph: Graph, scc: List[str], manager: BuildManager) -> No
         for id in stale:
             if graph[id].type_check_second_pass():
                 more = True
+    if any(manager.errors.is_errors_for_file(graph[id].xpath) for id in stale):
+        for id in stale:
+            graph[id].transitive_error = True
     for id in stale:
         graph[id].finish_passes()
         graph[id].write_cache()

test-data/unit/check-incremental.test

@@ -1,7 +1,7 @@
 -- Checks for incremental mode (see testcheck.py).
--- Each test is run twice, once with a cold cache, once with a warm cache.
+-- Each test is run at least twice, once with a cold cache, once with a warm cache.
 -- Before the tests are run again, in step N any *.py.N files are copied to
--- *.py.
+-- *.py.  There are at least two runs; more as long as there are *.py.N files.
 --
 -- You can add an empty section like `[delete mod.py.2]` to delete `mod.py`
 -- before the second run.
@@ -1113,7 +1113,7 @@ val = "foo"
 
 [builtins fixtures/module_all.pyi]
 [rechecked main, c, c.submodule]
-[stale]
+[stale c]
 [out2]
 tmp/c/submodule.py:2: error: Incompatible types in assignment (expression has type "str", variable has type "int")
 tmp/main.py:7: error: "C" has no attribute "foo"
@@ -3432,3 +3432,18 @@ extra = 1
 
 [out1]
 [out2]
+
+[case testErrorsAffectDependentsOnly]
+# cmd: mypy -m m.a m.b m.c
+[file m/__init__.py]
+[file m/a.py]
+1 + ''  # Deliberate error
+[file m/b.py]
+import m.a  # Depends on module with error
+[file m/c.py]
+import m  # No error here
+[rechecked m.a, m.b]
+[out1]
+tmp/m/a.py:1: error: Unsupported operand types for + ("int" and "str")
+[out2]
+tmp/m/a.py:1: error: Unsupported operand types for + ("int" and "str")