Repository navigation
stubtest: import submodules mentioned in __all__ - #9943
Conversation
This kind of helps resolve python#9935
| and getattr(getattr(runtime, m), "__module__", None) == runtime.__name__ | ||
| ] | ||
| # Check all things declared in module's __all__, falling back to runtime_public_contents | ||
| to_check.update(getattr(runtime, "__all__", runtime_public_contents)) |
There was a problem hiding this comment.
I believe this is incorrect, you just have to updated with both __all__ and runtime_public_contents.
__all__ is not about things being public or private, it is just about from pgk import *. For example, you could have tons of public stuff in a package, but just list just the most useful things in __all__.
However, consider that __all__ can list names starting with understcore (_), not sure if those should be considered public or private in mypy, but from pgk import * do import them.
There was a problem hiding this comment.
Type checkers use __all__ as a way to determine the public API:
Line 483 in 5aa707d
Note that due to the logic on L218 (on the right), there's a class of stuff that stubtest will check if it's present in the stub and runtime, but won't complain about if it's missing from the stub.
|
@hauntsaninja I'm certainly not an expert in this field. From your words, I understand that you are trying to explain to me that typecheckers have to (or decided to) use different rules that the usual, documented, years-old runtime Python rules. The situation with I tested your branch a bit, and because of the way you handle Please consider the following patch, it passes all the tests (with diff --git a/mypy/stubtest.py b/mypy/stubtest.py
index 3e52eb8be..0c554df0a 100644
--- a/mypy/stubtest.py
+++ b/mypy/stubtest.py
@@ -214,9 +214,9 @@ def verify_mypyfile(
to_check = set(
m
for m, o in stub.names.items()
- # TODO: change `o.module_public` to `not o.module_hidden`
- if o.module_public and (not m.startswith("_") or hasattr(runtime, m))
+ if not o.module_hidden and (not m.startswith("_") or hasattr(runtime, m))
)
+ # Check things in the module that are public
runtime_public_contents = [
m
for m in dir(runtime)
@@ -225,8 +225,9 @@ def verify_mypyfile(
# have a good way to detect re-exports at runtime.
and getattr(getattr(runtime, m), "__module__", None) == runtime.__name__
]
- # Check all things declared in module's __all__, falling back to runtime_public_contents
- to_check.update(getattr(runtime, "__all__", runtime_public_contents))
+ to_check.update(runtime_public_contents)
+ # Check all things declared in module's __all__
+ to_check.update(getattr(runtime, "__all__", []))
to_check.difference_update({"__file__", "__doc__", "__name__", "__builtins__", "__package__"})
for entry in sorted(to_check): |
|
Yeah, stubtest does have to strike a tricky balance, so thank you for acknowledging that! Note that the Still, that doesn't help with identifying missing entries from the stub if you use |
I guess you are afraid of false positives? Well, IMHO false positives are better than missing stuff in the to_check list. You have allowlists to silence false positives, errors should never pass silently, Anyway, it seems we will not fully agree on this one, but I understand your compromises, and you have the bragging rights.
I'm not really good at naming, but let me try:
|
This kind of helps resolve #9935