[stubtest] Verify __all__ exists in stub

[srittau]

Previously it wasn't an error if runtime included an
__all__ declaration, but the stubs did not. This PR
changes this to reflect the consensus that it would
be a good idea to ensure consistency in this case.

Fixes#13300

[JelleZijlstra]

Does this cause a lot of new errors in typeshed?

[srittau]

How can we check that? If it does, I suggest we just mass add .*\.__all__ # TODO to the stubtest allowlists, like @Avasam suggested.

[JelleZijlstra]

I tried locally (by installing this PR branch and running typeshed tests), and didn't see any new errors—which actually seems a little suspicious.

[srittau]

I tried locally (by installing this PR branch and running typeshed tests), and didn't see any new errors—which actually seems a little suspicious.

Very. I suspect there to be a quite significant number of hits.

[JelleZijlstra]

It was because of #18007; I forgot that stubtest doesn't do anything if there are mypy errors in the stubs.

With this patch:

Details

diff --git a/pyproject.toml b/pyproject.toml
index 46014bcd2..77e5e3127 100644
--- a/pyproject.toml+++ b/pyproject.toml@@ -166,3 +166,6 @@ known-first-party = ["ts_utils", "_utils"]
[tool.typeshed]
oldest_supported_python = "3.8"
++[tool.mypy]+disable_error_code = "deprecated"diff --git a/tests/stubtest_stdlib.py b/tests/stubtest_stdlib.py
index 5134f8270..77dab3d20 100755
--- a/tests/stubtest_stdlib.py+++ b/tests/stubtest_stdlib.py@@ -29,6 +29,8 @@ def run_stubtest(typeshed_dir: Path) -> int:
"--check-typeshed",
"--custom-typeshed-dir",
str(typeshed_dir),
+ "--mypy-config-file",+ str(typeshed_dir / "pyproject.toml"),
*allowlist_stubtest_arguments("stdlib"),
]
if sys.version_info < (3, 10):

I get these in the stdlib on macOS and 3.12:

error: asyncio.__all__ is not present in stub
error: email.__all__ is not present in stub
error: email._policybase.__all__ is not present in stub
error: importlib.resources.abc.__all__ is not present in stub
error: os.__all__ is not present in stub
error: socket.__all__ is not present in stub
error: xml.__all__ is not present in stub
error: zipfile._path.__all__ is not present in stub
note: unused allowlist entry concurrent.futures.__all__

Didn't run it on all the third-party stubs.

[srittau]

Interesting. I just commented out __all__ in stubs/passlib/passlib/utils/__init__.pyi, ran stubtest on passlib and didn't get any warnings. There is __all__ at runtime.

[JelleZijlstra]

Did you use stubtest_third_party.py? Looks like it always installs the mypy from our requirements.txt, so it wouldn't pick up local changes.

[srittau]

Yes, it seems that way. I added debug statements to the installed mypy and those are not printed either.

[JelleZijlstra]

Yes, if you want to test you'll have to edit requirements.txt or otherwise hack the script. I think the stdlib results are enough for me to believe that this works as expected, so I'm happy to merge this, but I'll wait a little bit in case Shantanu/Alex/someone else seeing this has more feedback.

[Avasam]

I do have a question: Do we want to report cases where __all__ is missing in stubs, but __all__ at runtime is redundant? (ie: re-exports all symbols anyway, so star-imports wouldn't be affected). I don't feel strongly either way, I just want to make sure the decision is explicit.

[srittau]

I do have a question: Do we want to report cases where __all__ is missing in stubs, but __all__ at runtime is redundant? (ie: re-exports all symbols anyway, so star-imports wouldn't be affected). I don't feel strongly either way, I just want to make sure the decision is explicit.

I would say yes. Stubs beings closer to the implementation has value in itself, instead of special casing specific cases. Also, it means that x.__all__ type checks correctly. There might be other use cases that we don't know about yet.

[hauntsaninja]

Thank you!