Fix skipped imports considered stale (#21639)

Fixes #21102

When the option `follow_imports = skip` is used, dependency states that
are initially considered skipped might have their reason changed to not
found in `load_graph`.

If on the first mypy run, a given suppressed module is written to cache
with its suppression reason set to skipped, and it's overridden to not
found in a subsequent mypy run, then `suppressed_deps_opts` between the
cache and the current run won't match and the module will be considered
stale.

To fix the issue, change the unconditional overwrite to `setdefault`.

This also affects mypyc as the dependency being considered stale means
that mypyc will needlessly recompile it instead of reading from cache.
Add a unit test to confirm that the dependency output files are not
overwritten with the fix.

Author: p-sawicki

mypy/build.py

@@ -4345,7 +4345,7 @@ def load_graph(
         for dep in st.ancestors + dependencies + st.suppressed:
             ignored = dep in st.suppressed_set and dep not in entry_points
             if ignored and dep not in added:
-                manager.missing_modules[dep] = SuppressionReason.NOT_FOUND
+                manager.missing_modules.setdefault(dep, SuppressionReason.NOT_FOUND)
                 # TODO: for now we skip this in the daemon as a performance optimization.
                 # This however creates a correctness issue, see #7777 and State.is_fresh().
                 if not manager.use_fine_grained_cache() or manager.options.warn_unused_configs:

mypyc/build.py

@@ -328,34 +328,36 @@ def generate_c(
         emit_messages(options, e.messages, time.time() - t0, serious=(not e.use_stdout))
         sys.exit(1)
 
-    t1 = time.time()
-    if result.errors:
-        emit_messages(options, result.errors, t1 - t0)
-        sys.exit(1)
-
-    if compiler_options.verbose:
-        print(f"Parsed and typechecked in {t1 - t0:.3f}s")
-
-    errors = Errors(options)
-    modules, ctext, mapper = emitmodule.compile_modules_to_c(
-        result, compiler_options=compiler_options, errors=errors, groups=groups
-    )
-    t2 = time.time()
-    emit_messages(options, errors.new_messages(), t2 - t1)
-    if errors.num_errors:
-        # No need to stop the build if only warnings were emitted.
-        sys.exit(1)
-
-    if compiler_options.verbose:
-        print(f"Compiled to C in {t2 - t1:.3f}s")
-
-    if options.mypyc_annotation_file:
-        generate_annotated_html(options.mypyc_annotation_file, result, modules, mapper)
+    try:
+        t1 = time.time()
+        if result.errors:
+            emit_messages(options, result.errors, t1 - t0)
+            sys.exit(1)
 
-    # Collect SourceDep dependencies
-    source_deps = sorted(emitmodule.collect_source_dependencies(modules), key=lambda d: d.path)
+        if compiler_options.verbose:
+            print(f"Parsed and typechecked in {t1 - t0:.3f}s")
 
-    return ctext, "\n".join(format_modules(modules)), source_deps
+        errors = Errors(options)
+        modules, ctext, mapper = emitmodule.compile_modules_to_c(
+            result, compiler_options=compiler_options, errors=errors, groups=groups
+        )
+        t2 = time.time()
+        emit_messages(options, errors.new_messages(), t2 - t1)
+        if errors.num_errors:
+            # No need to stop the build if only warnings were emitted.
+            sys.exit(1)
+
+        if compiler_options.verbose:
+            print(f"Compiled to C in {t2 - t1:.3f}s")
+
+        if options.mypyc_annotation_file:
+            generate_annotated_html(options.mypyc_annotation_file, result, modules, mapper)
+
+        # Collect SourceDep dependencies
+        source_deps = sorted(emitmodule.collect_source_dependencies(modules), key=lambda d: d.path)
+        return ctext, "\n".join(format_modules(modules)), source_deps
+    finally:
+        result.manager.metastore.close()
 
 
 def build_using_shared_lib(

mypyc/codegen/emitmodule.py

@@ -210,15 +210,18 @@ def parse_and_typecheck(
 ) -> BuildResult:
     assert options.strict_optional, "strict_optional must be turned on"
     mypyc_plugin = MypycPlugin(options, compiler_options, groups)
-    result = build(
-        sources=sources,
-        options=options,
-        alt_lib_path=alt_lib_path,
-        fscache=fscache,
-        extra_plugins=[mypyc_plugin],
-    )
-    mypyc_plugin.metastore.close()
+    try:
+        result = build(
+            sources=sources,
+            options=options,
+            alt_lib_path=alt_lib_path,
+            fscache=fscache,
+            extra_plugins=[mypyc_plugin],
+        )
+    finally:
+        mypyc_plugin.metastore.close()
     if result.errors:
+        result.manager.metastore.close()
         raise CompileError(result.errors)
     return result
 

mypyc/test-data/run-multimodule.test

@@ -1246,6 +1246,39 @@ import native
 
 [rechecked other_a]
 
+-- Test that importing a skipped module does not force rechecks.
+[case testIncrementalCompilationFollowImportsSkip]
+from other_dep import f
+assert f() == 1
+
+[file other_dep.py]
+import skipped
+
+def f() -> int:
+    return 1
+
+def g() -> int:
+    return 2
+
+# Files under "skipped" are not typechecked because of the "--follow_imports = skip" option.
+[file skipped/__init__.py]
+x = 1
+
+[file skipped/other_child.py]
+import skipped
+
+def child() -> int:
+    return 1
+
+[file native.py.2]
+from other_dep import g
+assert g() == 2
+
+[file driver.py]
+import native
+
+[rechecked native]
+
 [case testSeparateCompilationWithUndefinedAttribute]
 from other_a import A
 

mypyc/test/test_run.py

@@ -234,6 +234,10 @@ def run_case_step(self, testcase: DataDrivenTestCase, incremental_step: int) ->
         # Avoid checking modules/packages named 'unchecked', to provide a way
         # to test interacting with code we don't have types for.
         options.per_module_options["unchecked.*"] = {"follow_imports": "error"}
+        # Avoid checking modules/packages named 'skipped', to provide a way
+        # to test interacting with code ignored by follow_imports=skip.
+        options.per_module_options["skipped"] = {"follow_imports": "skip"}
+        options.per_module_options["skipped.*"] = {"follow_imports": "skip"}
 
         source = build.BuildSource("native.py", "native", None)
         sources = [source]

test-data/unit/check-incremental.test

@@ -8204,3 +8204,19 @@ reveal_type(x)
 tmp/a.py:2: note: Revealed type is "builtins.int"
 [out2]
 tmp/a.py:2: note: Revealed type is "builtins.str"
+
+[case testIncrementalFollowImportsSkipStubWithSkippedRuntimeDependency]
+# flags: --follow-imports=skip
+import pkg.foo
+[file pkg/__init__.py]
+
+[file pkg/foo.pyi]
+from pkg import helper
+
+x = 1
+[file pkg/helper.py]
+y = 2
+[rechecked]
+[stale]
+[out2]
+[out3]