Skip to content

Commit 7520f89

Browse files
msullivanmsullivan
committed
Fix the missing_modules related failures.
This has been a bit of an adventure. Since we are no longer doing a full `load_graph()`, we can't rely on it fully populating `missing_modules` if it is cleared between updates. If `missing_modules` isn't fully populated, then semanal will spuriously reject `from x import y` where `x.y` is a missing module. We can't just *not* clear `missing_modules`, however, because `parse_file` uses it to decide that modules are suppressed. But this is actually wrong! It can lead to an import failure message not being generated because some other module has already failed to import it (and perhaps even ignored the error). `parse_file()` shouldn't actually *need* to compute `suppressed`, though. If it is called on a file with no cache, `load_graph()` will handle moving deps to `suppressed`, and if called on a file that has had cache info loaded, then the dependency information has all been loaded from the cache. So we refactor things so that dep information from the cache is used when available and `parse_file` doesn't need to deal with it. I strongly suspect that this refactor obviates the need for `fix_suppressed_dependencies`, but I was not able to quickly produce a test case from the description in #2036, so I am not comfortable making that change.
1 parent 2218c11 commit 7520f89

6 files changed

Lines changed: 250 additions & 43 deletions

File tree

‎mypy/build.py‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,7 +1660,7 @@ def __init__(self,
16601660
else:
16611661
# Parse the file (and then some) to get the dependencies.
16621662
self.parse_file()
1663-
self.suppressed = []
1663+
self.compute_dependencies()
16641664
self.child_modules = set()
16651665

16661666
def skipping_ancestor(self, id: str, path: str, ancestor_for: 'State') -> None:
@@ -1815,6 +1815,8 @@ def fix_suppressed_dependencies(self, graph: Graph) -> None:
18151815
"""
18161816
# TODO: See if it's possible to move this check directly into parse_file in some way.
18171817
# TODO: Find a way to write a test case for this fix.
1818+
# TODO: I suspect that splitting compute_dependencies() out from parse_file
1819+
# obviates the need for this but lacking a test case for the problem this fixed...
18181820
silent_mode = (self.options.ignore_missing_imports or
18191821
self.options.follow_imports == 'skip')
18201822
if not silent_mode:
@@ -1881,49 +1883,53 @@ def parse_file(self) -> None:
18811883
# TODO: Why can't SemanticAnalyzerPass1 .analyze() do this?
18821884
self.tree.names = manager.semantic_analyzer.globals
18831885

1886+
for pri, id, line in manager.all_imported_modules_in_file(self.tree):
1887+
if id == '':
1888+
# Must be from a relative import.
1889+
manager.errors.set_file(self.xpath, self.id)
1890+
manager.errors.report(line, 0,
1891+
"No parent module -- cannot perform relative import",
1892+
blocker=True)
1893+
1894+
self.check_blockers()
1895+
1896+
def compute_dependencies(self):
1897+
"""Compute a module's dependencies after parsing it.
1898+
1899+
This is used when we parse a file that we didn't have
1900+
up-to-date cache information for. When we have an up-to-date
1901+
cache, we just use the cached info.
1902+
"""
1903+
manager = self.manager
1904+
18841905
# Compute (direct) dependencies.
18851906
# Add all direct imports (this is why we needed the first pass).
18861907
# Also keep track of each dependency's source line.
18871908
dependencies = []
1888-
suppressed = []
18891909
priorities = {} # type: Dict[str, int] # id -> priority
18901910
dep_line_map = {} # type: Dict[str, int] # id -> line
18911911
for pri, id, line in manager.all_imported_modules_in_file(self.tree):
18921912
priorities[id] = min(pri, priorities.get(id, PRI_ALL))
18931913
if id == self.id:
18941914
continue
1895-
# Omit missing modules, as otherwise we could not type-check
1896-
# programs with missing modules.
1897-
if id in manager.missing_modules:
1898-
if id not in dep_line_map:
1899-
suppressed.append(id)
1900-
dep_line_map[id] = line
1901-
continue
1902-
if id == '':
1903-
# Must be from a relative import.
1904-
manager.errors.set_file(self.xpath, self.id)
1905-
manager.errors.report(line, 0,
1906-
"No parent module -- cannot perform relative import",
1907-
blocker=True)
1908-
continue
19091915
if id not in dep_line_map:
19101916
dependencies.append(id)
19111917
dep_line_map[id] = line
19121918
# Every module implicitly depends on builtins.
19131919
if self.id != 'builtins' and 'builtins' not in dep_line_map:
19141920
dependencies.append('builtins')
19151921

1916-
# If self.dependencies is already set, it was read from the
1917-
# cache, but for some reason we're re-parsing the file.
19181922
# NOTE: What to do about race conditions (like editing the
19191923
# file while mypy runs)? A previous version of this code
19201924
# explicitly checked for this, but ran afoul of other reasons
19211925
# for differences (e.g. silent mode).
1926+
1927+
# Missing dependencies will be moved from dependencies to
1928+
# suppressed when they fail to be loaded in load_graph.
19221929
self.dependencies = dependencies
1923-
self.suppressed = suppressed
1930+
self.suppressed = []
19241931
self.priorities = priorities
19251932
self.dep_line_map = dep_line_map
1926-
self.check_blockers()
19271933

19281934
def semantic_analysis(self) -> None:
19291935
assert self.tree is not None, "Internal error: method must be called on parsed file only"

‎mypy/server/update.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@
122122

123123
from mypy.build import (
124124
BuildManager, State, BuildSource, Graph, load_graph, find_module_clear_caches,
125-
DEBUG_FINE_GRAINED,
125+
PRI_INDIRECT, DEBUG_FINE_GRAINED,
126126
)
127127
from mypy.checker import DeferredNode
128128
from mypy.errors import Errors, CompileError
@@ -347,7 +347,9 @@ def update_single_isolated(module: str,
347347
old_modules = dict(manager.modules)
348348
sources = get_sources(previous_modules, [(module, path)])
349349

350-
manager.missing_modules.clear()
350+
if module in manager.missing_modules:
351+
manager.missing_modules.remove(module)
352+
351353
try:
352354
if module in graph:
353355
del graph[module]
@@ -524,7 +526,9 @@ def get_all_changed_modules(root_module: str,
524526

525527
def verify_dependencies(state: State, manager: BuildManager) -> None:
526528
"""Report errors for import targets in module that don't exist."""
527-
for dep in state.dependencies + state.suppressed: # TODO: ancestors?
529+
# Strip out indirect dependencies. See comment in build.load_graph().
530+
dependencies = [dep for dep in state.dependencies if state.priorities.get(dep) != PRI_INDIRECT]
531+
for dep in dependencies + state.suppressed: # TODO: ancestors?
528532
if dep not in manager.modules and not manager.options.ignore_missing_imports:
529533
assert state.tree
530534
line = find_import_line(state.tree, dep) or 1

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def f() -> str:
3232
[out2]
3333
tmp/b.py:3: error: Incompatible types in assignment (expression has type "int", variable has type "str")
3434

35-
[case testAddFileFineGrainedIncremental]
35+
[case testAddFileFineGrainedIncremental1]
3636
# cmd: mypy -m a
3737
# cmd2: mypy -m a b
3838
[file a.py]
@@ -46,6 +46,19 @@ tmp/a.py:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-impor
4646
[out2]
4747
tmp/a.py:2: error: Argument 1 to "f" has incompatible type "int"; expected "str"
4848

49+
[case testAddFileFineGrainedIncremental2]
50+
# flags: --follow-imports=skip --ignore-missing-imports
51+
# cmd: mypy -m a
52+
# cmd2: mypy -m a b
53+
[file a.py]
54+
import b
55+
b.f(1)
56+
[file b.py.2]
57+
def f(x: str) -> None: pass
58+
[out1]
59+
[out2]
60+
tmp/a.py:2: error: Argument 1 to "f" has incompatible type "int"; expected "str"
61+
4962
[case testDeleteFileFineGrainedIncremental]
5063
# cmd: mypy -m a b
5164
# cmd2: mypy -m a

‎test-data/unit/check-incremental.test‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4059,3 +4059,71 @@ class Baz:
40594059
[out]
40604060
[out2]
40614061
tmp/a.py:3: error: Unsupported operand types for + ("int" and "str")
4062+
4063+
[case testIncrementalWithIgnoresTwice]
4064+
import a
4065+
[file a.py]
4066+
import b
4067+
import foo # type: ignore
4068+
[file b.py]
4069+
x = 1
4070+
[file b.py.2]
4071+
x = 'hi'
4072+
[file b.py.3]
4073+
x = 1
4074+
[builtins fixtures/module.pyi]
4075+
[out]
4076+
[out2]
4077+
[out3]
4078+
4079+
[case testIgnoredImport2]
4080+
import x
4081+
[file y.py]
4082+
import xyz # type: ignore
4083+
B = 0
4084+
from x import A
4085+
[file x.py]
4086+
A = 0
4087+
from y import B
4088+
[file x.py.2]
4089+
A = 1
4090+
from y import B
4091+
[file x.py.3]
4092+
A = 2
4093+
from y import B
4094+
[out]
4095+
[out2]
4096+
[out3]
4097+
4098+
[case testDeletionOfSubmoduleTriggersImportFrom2]
4099+
from p.q import f
4100+
f()
4101+
[file p/__init__.py]
4102+
[file p/q.py]
4103+
def f() -> None: pass
4104+
[delete p/q.py.2]
4105+
[file p/q.py.3]
4106+
def f(x: int) -> None: pass
4107+
[out]
4108+
[out2]
4109+
main:1: error: Cannot find module named 'p.q'
4110+
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)
4111+
[out3]
4112+
main:2: error: Too few arguments for "f"
4113+
4114+
[case testDeleteIndirectDependency]
4115+
import b
4116+
b.x.foo()
4117+
[file b.py]
4118+
import c
4119+
x = c.Foo()
4120+
[file c.py]
4121+
class Foo:
4122+
def foo(self) -> None: pass
4123+
[delete c.py.2]
4124+
[file b.py.2]
4125+
class Foo:
4126+
def foo(self) -> None: pass
4127+
x = Foo()
4128+
[out]
4129+
[out2]

‎test-data/unit/check-modules.test‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1943,3 +1943,15 @@ main:2: error: Name 'name' is not defined
19431943
main:3: error: Revealed type is 'Any'
19441944

19451945
[builtins fixtures/module.pyi]
1946+
1947+
[case testFailedImportFromTwoModules]
1948+
import c
1949+
import b
1950+
[file b.py]
1951+
import c
1952+
1953+
[out]
1954+
-- TODO: it would be better for this to be in the other order
1955+
tmp/b.py:1: error: Cannot find module named 'c'
1956+
main:1: error: Cannot find module named 'c'
1957+
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

0 commit comments

Comments
 (0)