Skip to content

Commit

Permalink
Fix crash involving explicit reexport, import cycle, wildcard (python…
Browse files Browse the repository at this point in the history
…#11632)

Fixes python#8481
Fixes python#9941
Fixes python#11025
Fixes python#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 python#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.
  • Loading branch information
hauntsaninja authored and tushar-deepsource committed Jan 20, 2022
1 parent 3f6a197 commit eb4fd97
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
4 changes: 3 additions & 1 deletion mypy/semanal.py
Expand Up @@ -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.
Expand Down
25 changes: 25 additions & 0 deletions test-data/unit/check-incremental.test
Expand Up @@ -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]

0 comments on commit eb4fd97

Please sign in to comment.