Skip to content

Commit 79ff4ff

Browse files
authored
Do not cache module lookup results that may become invalid in future (#19044)
Fixes #19037. See my analysis in #19037 for motivation. As a side note, with this patch the second run behaves as expected (without it mypy tries to reanalyze a lot of modules during the 2nd run, now it only rechecks the inputs with errors).
1 parent f336726 commit 79ff4ff

File tree

7 files changed

+74
-23
lines changed

7 files changed

+74
-23
lines changed

mypy/modulefinder.py

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,21 @@ def find_module(self, id: str, *, fast_path: bool = False) -> ModuleSearchResult
320320
use_typeshed = self._typeshed_has_version(id)
321321
elif top_level in self.stdlib_py_versions:
322322
use_typeshed = self._typeshed_has_version(top_level)
323-
self.results[id] = self._find_module(id, use_typeshed)
324-
if (
325-
not (fast_path or (self.options is not None and self.options.fast_module_lookup))
326-
and self.results[id] is ModuleNotFoundReason.NOT_FOUND
327-
and self._can_find_module_in_parent_dir(id)
328-
):
329-
self.results[id] = ModuleNotFoundReason.WRONG_WORKING_DIRECTORY
323+
result, should_cache = self._find_module(id, use_typeshed)
324+
if should_cache:
325+
if (
326+
not (
327+
fast_path or (self.options is not None and self.options.fast_module_lookup)
328+
)
329+
and result is ModuleNotFoundReason.NOT_FOUND
330+
and self._can_find_module_in_parent_dir(id)
331+
):
332+
self.results[id] = ModuleNotFoundReason.WRONG_WORKING_DIRECTORY
333+
else:
334+
self.results[id] = result
335+
return self.results[id]
336+
else:
337+
return result
330338
return self.results[id]
331339

332340
def _typeshed_has_version(self, module: str) -> bool:
@@ -384,11 +392,16 @@ def _can_find_module_in_parent_dir(self, id: str) -> bool:
384392
while any(is_init_file(file) for file in os.listdir(working_dir)):
385393
working_dir = os.path.dirname(working_dir)
386394
parent_search.search_paths = SearchPaths((working_dir,), (), (), ())
387-
if not isinstance(parent_search._find_module(id, False), ModuleNotFoundReason):
395+
if not isinstance(parent_search._find_module(id, False)[0], ModuleNotFoundReason):
388396
return True
389397
return False
390398

391-
def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
399+
def _find_module(self, id: str, use_typeshed: bool) -> tuple[ModuleSearchResult, bool]:
400+
"""Try to find a module in all available sources.
401+
402+
Returns:
403+
``(result, can_be_cached)`` pair.
404+
"""
392405
fscache = self.fscache
393406

394407
# Fast path for any modules in the current source set.
@@ -424,7 +437,7 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
424437
else None
425438
)
426439
if p:
427-
return p
440+
return p, True
428441

429442
# If we're looking for a module like 'foo.bar.baz', it's likely that most of the
430443
# many elements of lib_path don't even have a subdirectory 'foo/bar'. Discover
@@ -444,6 +457,9 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
444457
for component in (components[0], components[0] + "-stubs")
445458
for package_dir in self.find_lib_path_dirs(component, self.search_paths.package_path)
446459
}
460+
# Caching FOUND_WITHOUT_TYPE_HINTS is not always safe. That causes issues with
461+
# typed subpackages in namespace packages.
462+
can_cache_any_result = True
447463
for pkg_dir in self.search_paths.package_path:
448464
if pkg_dir not in candidate_package_dirs:
449465
continue
@@ -475,6 +491,7 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
475491
if isinstance(non_stub_match, ModuleNotFoundReason):
476492
if non_stub_match is ModuleNotFoundReason.FOUND_WITHOUT_TYPE_HINTS:
477493
found_possible_third_party_missing_type_hints = True
494+
can_cache_any_result = False
478495
else:
479496
third_party_inline_dirs.append(non_stub_match)
480497
self._update_ns_ancestors(components, non_stub_match)
@@ -513,7 +530,7 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
513530
if verify and not verify_module(fscache, id, path_stubs, dir_prefix):
514531
near_misses.append((path_stubs, dir_prefix))
515532
else:
516-
return path_stubs
533+
return path_stubs, True
517534

518535
# Prefer package over module, i.e. baz/__init__.py* over baz.py*.
519536
for extension in PYTHON_EXTENSIONS:
@@ -523,7 +540,7 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
523540
if verify and not verify_module(fscache, id, path, dir_prefix):
524541
near_misses.append((path, dir_prefix))
525542
continue
526-
return path
543+
return path, True
527544

528545
# In namespace mode, register a potential namespace package
529546
if self.options and self.options.namespace_packages:
@@ -541,7 +558,7 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
541558
if verify and not verify_module(fscache, id, path, dir_prefix):
542559
near_misses.append((path, dir_prefix))
543560
continue
544-
return path
561+
return path, True
545562

546563
# In namespace mode, re-check those entries that had 'verify'.
547564
# Assume search path entries xxx, yyy and zzz, and we're
@@ -570,35 +587,35 @@ def _find_module(self, id: str, use_typeshed: bool) -> ModuleSearchResult:
570587
for path, dir_prefix in near_misses
571588
]
572589
index = levels.index(max(levels))
573-
return near_misses[index][0]
590+
return near_misses[index][0], True
574591

575592
# Finally, we may be asked to produce an ancestor for an
576593
# installed package with a py.typed marker that is a
577594
# subpackage of a namespace package. We only fess up to these
578595
# if we would otherwise return "not found".
579596
ancestor = self.ns_ancestors.get(id)
580597
if ancestor is not None:
581-
return ancestor
598+
return ancestor, True
582599

583600
approved_dist_name = stub_distribution_name(id)
584601
if approved_dist_name:
585602
if len(components) == 1:
586-
return ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED
603+
return ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED, True
587604
# If we're a missing submodule of an already installed approved stubs, we don't want to
588605
# error with APPROVED_STUBS_NOT_INSTALLED, but rather want to return NOT_FOUND.
589606
for i in range(1, len(components)):
590607
parent_id = ".".join(components[:i])
591608
if stub_distribution_name(parent_id) == approved_dist_name:
592609
break
593610
else:
594-
return ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED
611+
return ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED, True
595612
if self.find_module(parent_id) is ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED:
596-
return ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED
597-
return ModuleNotFoundReason.NOT_FOUND
613+
return ModuleNotFoundReason.APPROVED_STUBS_NOT_INSTALLED, True
614+
return ModuleNotFoundReason.NOT_FOUND, True
598615

599616
if found_possible_third_party_missing_type_hints:
600-
return ModuleNotFoundReason.FOUND_WITHOUT_TYPE_HINTS
601-
return ModuleNotFoundReason.NOT_FOUND
617+
return ModuleNotFoundReason.FOUND_WITHOUT_TYPE_HINTS, can_cache_any_result
618+
return ModuleNotFoundReason.NOT_FOUND, True
602619

603620
def find_modules_recursive(self, module: str) -> list[BuildSource]:
604621
module_path = self.find_module(module, fast_path=True)

mypy/test/testpep561.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,11 @@ def test_pep561(testcase: DataDrivenTestCase) -> None:
145145
output.append(line[len(test_temp_dir + os.sep) :].rstrip("\r\n"))
146146
else:
147147
# Normalize paths so that the output is the same on Windows and Linux/macOS.
148-
line = line.replace(test_temp_dir + os.sep, test_temp_dir + "/")
149-
output.append(line.rstrip("\r\n"))
148+
# Yes, this is naive: replace all slashes preceding first colon, if any.
149+
path, *rest = line.split(":", maxsplit=1)
150+
if rest:
151+
path = path.replace(os.sep, "/")
152+
output.append(":".join([path, *rest]).rstrip("\r\n"))
150153
iter_count = "" if i == 0 else f" on iteration {i + 1}"
151154
expected = testcase.output if i == 0 else testcase.output2.get(i + 1, [])
152155

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
[project]
2+
name = 'typedpkg_namespace.nested'
3+
version = '0.1'
4+
description = 'Two namespace packages, one of them typed'
5+
6+
[tool.hatch.build]
7+
include = ["**/*.py", "**/*.pyi", "**/py.typed"]
8+
9+
[build-system]
10+
requires = ["hatchling==1.18"]
11+
build-backend = "hatchling.build"

test-data/packages/typedpkg_ns_nested/typedpkg_ns/a/__init__.py

Whitespace-only changes.

test-data/packages/typedpkg_ns_nested/typedpkg_ns/a/py.typed

Whitespace-only changes.

test-data/packages/typedpkg_ns_nested/typedpkg_ns/b/__init__.py

Whitespace-only changes.

test-data/unit/pep561.test

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,3 +213,23 @@ from typedpkg_ns.a.bbb import bf
213213
[file dummy.py.2]
214214
[out]
215215
[out2]
216+
217+
[case testTypedNamespaceSubpackage]
218+
# pkgs: typedpkg_ns_nested
219+
import our
220+
[file our/__init__.py]
221+
import our.bar
222+
import our.foo
223+
[file our/bar.py]
224+
from typedpkg_ns.b import Something
225+
[file our/foo.py]
226+
import typedpkg_ns.a
227+
228+
[file dummy.py.2]
229+
230+
[out]
231+
our/bar.py:1: error: Skipping analyzing "typedpkg_ns.b": module is installed, but missing library stubs or py.typed marker
232+
our/bar.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
233+
[out2]
234+
our/bar.py:1: error: Skipping analyzing "typedpkg_ns.b": module is installed, but missing library stubs or py.typed marker
235+
our/bar.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

0 commit comments

Comments
 (0)