From cee5d3e9a29f43a10a8eeca760976657bf1689c9 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Fri, 3 Dec 2021 06:11:44 -0800 Subject: [PATCH] Fix crash involving explicit reexport, import cycle, wildcard (#11632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #8481 Fixes #9941 Fixes #11025 Fixes #11038 This is the unresolved crash that's been reported the most times, e.g., it's as easy to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even `import torch`, with the right PyTorch version). I know of multiple popular Python packages that have made changes just to work around this crash. I would love to see this released, potentially along with #11630. Thanks to @rraval for making a minimal repro! The fix ended up being a one-liner, but it took me a bit to track down :-) Since things worked with implicit reexport, I differentially patched in explicit reexport logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, which fixes things: https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028 Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` pkg.a.B was first created with `module_public=True` (resulting in creation of a PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. So tracking down where pkg.a.B symbol was first created with `module_public=True` led me to this "obvious" bug. --- mypy/semanal.py | 4 +++- test-data/unit/check-incremental.test | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index e55fada5689c..21741f90a528 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1933,7 +1933,9 @@ def report_missing_module_attribute( if self.is_incomplete_namespace(import_id): # We don't know whether the name will be there, since the namespace # is incomplete. Defer the current target. - self.mark_incomplete(imported_id, context) + self.mark_incomplete( + imported_id, context, module_public=module_public, module_hidden=module_hidden + ) return message = 'Module "{}" has no attribute "{}"'.format(import_id, source_id) # Suggest alternatives, if any match is found. diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 342850ab24d0..1d2262ab303d 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -5585,3 +5585,28 @@ bar.x + 0 x = 0 [rechecked] [stale] + +[case testExplicitReexportImportCycleWildcard] +# flags: --no-implicit-reexport +import pkg.a +[file pkg/__init__.pyi] + +[file pkg/a.pyi] +MYPY = False +if MYPY: + from pkg.b import B + +[file pkg/b.pyi] +import pkg.a +MYPY = False +if MYPY: + from pkg.c import C +class B: + pass + +[file pkg/c.pyi] +from pkg.a import * +class C: + pass +[rechecked] +[stale]