Skip to content

Commit

Permalink
Reduce duplicate errors when __all__ is missing from the stub
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood committed Feb 20, 2022
1 parent 2b41db4 commit 859df5a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 10 deletions.
14 changes: 9 additions & 5 deletions mypy/stubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def _verify_exported_names(
(
"module: names exported from the stub "
"do not correspond to the names exported at runtime. "
"(Note: This may be due to a missing or inaccurate "
"(Note: This is probably due to an inaccurate "
"`__all__` in the stub.)"
),
# pass in MISSING instead of the stub and runtime objects,
Expand Down Expand Up @@ -245,7 +245,11 @@ def verify_mypyfile(

if hasattr(runtime, "__all__"):
runtime_all_as_set = set(runtime.__all__)
yield from _verify_exported_names(object_path, stub, runtime_all_as_set)
if "__all__" in stub.names:
# Only verify the contents of the stub's __all__
# if the stub actually defines __all__
# Otherwise we end up with duplicate errors when __all__ is missing
yield from _verify_exported_names(object_path, stub, runtime_all_as_set)
else:
runtime_all_as_set = None

Expand All @@ -267,16 +271,16 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
return not isinstance(obj, types.ModuleType)

runtime_public_contents = (
["__all__", *runtime_all_as_set]
runtime_all_as_set | {"__all__"}
if runtime_all_as_set is not None
else [
else {
m
for m in dir(runtime)
if not is_probably_private(m)
# Ensure that the object's module is `runtime`, since in the absence of __all__ we
# don't have a good way to detect re-exports at runtime.
and _belongs_to_runtime(runtime, m)
]
}
)
# Check all things declared in module's __all__, falling back to our best guess
to_check.update(runtime_public_contents)
Expand Down
16 changes: 11 additions & 5 deletions mypy/test/teststubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,10 @@ def f(): return 3
error=None,
)

@collect_cases
def test_missing_all(self) -> Iterator[Case]:
yield Case(stub="", runtime="__all__ = []", error="__all__")

@collect_cases
def test_missing(self) -> Iterator[Case]:
yield Case(stub="x = 5", runtime="", error="x")
Expand All @@ -708,12 +712,14 @@ def h(x: str): ...
runtime="",
error="h",
)
# __all__ present at runtime, but not in stub -> error
yield Case(stub="", runtime="__all__ = []", error="__all__")
# If runtime has __all__ but stub does not,
# we should raise an error with the module name itself
# if there are any names defined in the stub that are not in the runtime __all__
yield Case(stub="_Z = int", runtime="", error="")
# we'll already raise an error for a missing __all__,
# so we shouldn't raise another error for inconsistent exported names
yield Case(stub="_Z = int", runtime="__all__ = []", error=None)
# But we *should* raise an error with the module name itself,
# if the stub *does* define __all__,
# but the stub's __all__ is inconsistent with the runtime's __all__
yield Case(stub="__all__ = ['foo']", runtime="", error="")
yield Case(stub="", runtime="__all__ += ['y']\ny = 5", error="y")
yield Case(stub="", runtime="__all__ += ['g']\ndef g(): pass", error="g")
# Here we should only check that runtime has B, since the stub explicitly re-exports it
Expand Down

0 comments on commit 859df5a

Please sign in to comment.