Skip to content

Commit 9ff0f0b

Browse files
Michael0x2agvanrossum
authored andcommitted
Fix accessing qualified import in incremental mode (#3548)
Fixes #3274. The problem was that mypy was previously doing the following, given an empty cache: 1. Analyze the SCCs (ignoring the builtins) in this exact order: `['c.d']`, then `['c']`, then `['b']`, then `['a']`. No issues here. 2. Parse, typecheck, and write `c.d` to cache -- also no issues here. 2. Parse, typecheck, and write `c` to cache. The error occurs here -- mypy doesn't recognize that `c` has any submodules, and so will not record `c.d` into `c`'s symbol table. This means the saved cache for `c` will be essentially empty. 3. When parsing `b`, mypy *will* recognize that `c.d` is a submodule of `c` due to the import. During the semantic analysis phase, mypy will then actually modify `c`'s symbol table to include `d`. This exact process takes place in `SemanticAnalyzer.add_submodules_to_parent_modules`. This is why typechecking succeeds for `a` and `b` during a fresh However, this change wasn't ever written to the cache, so won't be remembered in the next run! 4. Will parse and typecheck `a`, using the modified (but not preserved) symbol table. Or to put it more succinctly, the code sometimes seems to be relying on the assumption that a symbol table for a given module will not be modified after that SCC is processed. However, this invariant is false due to the 'parent patching' mechanism. It's worth nothing that this patching also occurs during Python's runtime -- it isn't just an artifact of mypy's parsing process. This commit opts for a relatively conservative course of action by simply re-running this patching process when handling fresh SCCs. Other potential fixes include deferring writing to cache until *all* SCCs are processed (which initially seemed like a more robust solution but broke multiple tests when I tried it), or replacing the current parent patching mechanism with something entirely different (which seems like the sort of thing that could subtly break code).
1 parent 870d1d8 commit 9ff0f0b

File tree

2 files changed

+21
-2
lines changed

2 files changed

+21
-2
lines changed

mypy/build.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,6 +1510,24 @@ def calculate_mros(self) -> None:
15101510
fixup_module_pass_two(self.tree, self.manager.modules,
15111511
self.manager.options.quick_and_dirty)
15121512

1513+
def patch_dependency_parents(self) -> None:
1514+
"""
1515+
In Python, if a and a.b are both modules, running `import a.b` will
1516+
modify not only the current module's namespace, but a's namespace as
1517+
well -- see SemanticAnalyzer.add_submodules_to_parent_modules for more
1518+
details.
1519+
1520+
However, this patching process can occur after `a` has been parsed and
1521+
serialized during increment mode. Consequently, we need to repeat this
1522+
patch when deserializing a cached file.
1523+
1524+
This function should be called only when processing fresh SCCs -- the
1525+
semantic analyzer will perform this patch for us when processing stale
1526+
SCCs.
1527+
"""
1528+
for dep in self.dependencies:
1529+
self.manager.semantic_analyzer.add_submodules_to_parent_modules(dep, True)
1530+
15131531
def fix_suppressed_dependencies(self, graph: Graph) -> None:
15141532
"""Corrects whether dependencies are considered stale in silent mode.
15151533
@@ -2042,6 +2060,8 @@ def process_fresh_scc(graph: Graph, scc: List[str]) -> None:
20422060
graph[id].fix_cross_refs()
20432061
for id in scc:
20442062
graph[id].calculate_mros()
2063+
for id in scc:
2064+
graph[id].patch_dependency_parents()
20452065

20462066

20472067
def process_stale_scc(graph: Graph, scc: List[str], manager: BuildManager) -> None:

test-data/unit/check-serialize.test

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,8 +1103,7 @@ main:3: error: Too few arguments for "g"
11031103
[out2]
11041104
main:3: error: Too few arguments for "g"
11051105

1106-
[case testSerializeQualifiedImport-skip]
1107-
# This fails currently: https://github.com/python/mypy/issues/3274
1106+
[case testSerializeQualifiedImport]
11081107
import b
11091108
b.c.d.f()
11101109
b.c.d.g()

0 commit comments

Comments
 (0)