Skip to content

Commit 1d18566

Browse files
authored
Fix crash on partially typed namespace package (#20742)
Fixes #16214 Closes #20658 The root cause is similar to the issue fixed by #19044, when there are multiple packages in `google` namespace, some with `py.typed`, some without. As a result, there can be a situation where we first find the untyped one, and mark ancestor (i.e. `google`) as suppressed, but then we find the typed one and actually add ancestor (i.e. `google`) to the graph. This creates a situation where we write some references to `google` in cache, while it may not be loaded on the warm run, thus causing a crash. My solution is simple: un-suppress namespace ancestors if there is at least one package that needs them, so that the build graph is consistent. Unfortunately, this uncovers another (pre-existing) bug where `# type: ignore`s are not handled correctly on warm runs, similar to #20105. In any case, I am going to submit a separate PR for this bug (I think there may be a simple fix for it).
1 parent e2923a1 commit 1d18566

3 files changed

Lines changed: 31 additions & 9 deletions

File tree

mypy/build.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3144,9 +3144,9 @@ def find_module_and_diagnose(
31443144
raise ModuleNotFound
31453145
if is_silent_import_module(manager, result) and not root_source:
31463146
follow_imports = "silent"
3147-
return (result, follow_imports)
3147+
return result, follow_imports
31483148
else:
3149-
# Could not find a module. Typically the reason is a
3149+
# Could not find a module. Typically, the reason is a
31503150
# misspelled module name, missing stub, module not in
31513151
# search path or the module has not been installed.
31523152

@@ -3155,7 +3155,7 @@ def find_module_and_diagnose(
31553155
# Don't honor a global (not per-module) ignore_missing_imports
31563156
# setting for modules that used to have bundled stubs, as
31573157
# otherwise updating mypy can silently result in new false
3158-
# negatives. (Unless there are stubs but they are incomplete.)
3158+
# negatives. (Unless there are stubs, but they are incomplete.)
31593159
global_ignore_missing_imports = manager.options.ignore_missing_imports
31603160
if (
31613161
is_module_from_legacy_bundled_package(id)
@@ -3650,7 +3650,7 @@ def load_graph(
36503650
added = [dep for dep in st.suppressed if find_module_simple(dep, manager)]
36513651
else:
36523652
# During initial loading we don't care about newly added modules,
3653-
# they will be taken care of during fine grained update. See also
3653+
# they will be taken care of during fine-grained update. See also
36543654
# comment about this in `State.__init__()`.
36553655
added = []
36563656
for dep in st.ancestors + dependencies + st.suppressed:
@@ -3710,11 +3710,15 @@ def load_graph(
37103710
assert newst.id not in graph, newst.id
37113711
graph[newst.id] = newst
37123712
new.append(newst)
3713-
if dep in graph and dep in st.suppressed_set:
3714-
# Previously suppressed file is now visible
3713+
# There are two things we need to do after the initial load loop. One is up-suppress
3714+
# modules that are back in graph. We need to do this after the loop to cover an edge
3715+
# case where a namespace package ancestor is shared by a typed and an untyped package.
3716+
for st in graph.values():
3717+
for dep in st.suppressed:
3718+
if dep in graph:
37153719
st.add_dependency(dep)
3716-
# In the loop above we skip indirect dependencies, so to make indirect dependencies behave
3717-
# more consistently with regular ones, we suppress them manually here (when needed).
3720+
# Second, in the initial loop we skip indirect dependencies, so to make indirect dependencies
3721+
# behave more consistently with regular ones, we suppress them manually here (when needed).
37183722
for st in graph.values():
37193723
indirect = [dep for dep in st.dependencies if st.priorities.get(dep) == PRI_INDIRECT]
37203724
for dep in indirect:

mypy/modulefinder.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ def _find_module(self, id: str, use_typeshed: bool) -> tuple[ModuleSearchResult,
471471
if fscache.isdir(path):
472472
if fscache.isfile(stub_typed_file):
473473
# Stub packages can have a py.typed file, which must include
474-
# 'partial\n' to make the package partial
474+
# 'partial\n' to make the package partial.
475475
# Partial here means that mypy should look at the runtime
476476
# package if installed.
477477
if fscache.read(stub_typed_file).decode().strip() == "partial":

test-data/unit/pep561.test

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,21 @@ our/bar.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#
233233
[out2]
234234
our/bar.py:1: error: Skipping analyzing "typedpkg_ns.b": module is installed, but missing library stubs or py.typed marker
235235
our/bar.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
236+
237+
[case testPartiallyTypedNamespacePackageNoCrash]
238+
# pkgs: typedpkg_ns_nested
239+
import a
240+
[file a.py]
241+
import m1
242+
import m2
243+
[file a.py.2]
244+
import m1
245+
[file m1.py]
246+
import typedpkg_ns.b # type: ignore
247+
[file m2.py]
248+
import typedpkg_ns.a
249+
# TODO: the error on second run is a bug, but it is a separate issue, likely #20105.
250+
[out]
251+
[out2]
252+
m1.py:1: error: Skipping analyzing "typedpkg_ns": module is installed, but missing library stubs or py.typed marker
253+
m1.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

0 commit comments

Comments
 (0)