Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stubtest: error if typeshed is missing modules from the stdlib #15729

Merged
merged 4 commits into from
Aug 23, 2023

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jul 20, 2023

We currently flag modules missing from third-party stubs in stubtest, but don't do similarly for typeshed's stdlib stubs. This PR adds that functionality for typeshed's stdlib stubs as well.

This PR currently results in quite a few errors when running stubtest on typeshed's stdlib stubs. Some of them are true positives, and some are not. However, I think all of them (except for many test or tests submodules, which would be suppressed if #15725 is merged) are things I'd prefer for us to have explicitly allowlisted in typeshed, so we can easily see what we're missing.

If #15725 is merged and this is merged, we'd have to add the following allowlist entries for typeshed's stdlib stubs to get stubtest passing again:

asyncio.__main__
ctypes.macholib
ctypes.macholib.dyld
ctypes.macholib.dylib
ctypes.macholib.framework
encodings.aliases
encodings.ascii
encodings.base64_codec
encodings.big5
encodings.big5hkscs
encodings.bz2_codec
encodings.charmap
encodings\.cp\d+
encodings\.euc_.+
encodings.gb18030
encodings.gb2312
encodings.gbk
encodings.hex_codec
encodings.hp_roman8
encodings.hz
encodings.idna
encodings\.iso2022_.*
encodings\.iso8859_\d+
encodings.johab
encodings\.koi8_\w
encodings.kz1048
encodings.latin_1
encodings\.mac_.+
encodings.mbcs
encodings.oem
encodings.palmos
encodings.ptcp154
encodings.punycode
encodings.quopri_codec
encodings.raw_unicode_escape
encodings.rot_13
encodings\.shift_jis.*
encodings.tis_620
encodings.undefined
encodings.unicode_escape
encodings.utf_16
encodings.utf_16_be
encodings.utf_16_le
encodings.utf_32
encodings.utf_32_be
encodings.utf_32_le
encodings.utf_7
encodings.uu_codec
encodings.zlib_codec
ensurepip.__main__
idlelib
idlelib\..+
importlib.readers
importlib.resources.readers
importlib.resources.simple
importlib.simple
json.scanner
lib2to3.__main__
lib2to3.btm_utils
lib2to3.fixer_util
lib2to3.patcomp
lib2to3.pgen2.conv
nt
sqlite3.dump
tkinter.__main__
turtledemo
turtledemo\..+
unittest.__main__
venv.__main__
xml.sax.expatreader

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jul 21, 2023

@sobolevn pointed out that #15725 wouldn't have been a good idea as it was based on some bad assumptions, so here's the updated list of allowlist entries that we'd have to add to typeshed if this PR was merged (the previous list was based on the assumption that #15725 would be merged first):

asyncio.__main__
ctypes.macholib
ctypes.macholib.dyld
ctypes.macholib.dylib
ctypes.macholib.framework
ctypes.test
ctypes\.test\..+
encodings.aliases
encodings.ascii
encodings.base64_codec
encodings.big5
encodings.big5hkscs
encodings.bz2_codec
encodings.charmap
encodings\.cp\d+
encodings\.euc_.+
encodings.gb18030
encodings.gb2312
encodings.gbk
encodings.hex_codec
encodings.hp_roman8
encodings.hz
encodings.idna
encodings\.iso2022_.*
encodings\.iso8859_\d+
encodings.johab
encodings\.koi8_\w
encodings.kz1048
encodings.latin_1
encodings\.mac_.+
encodings.mbcs
encodings.oem
encodings.palmos
encodings.ptcp154
encodings.punycode
encodings.quopri_codec
encodings.raw_unicode_escape
encodings.rot_13
encodings\.shift_jis.*
encodings.tis_620
encodings.undefined
encodings.unicode_escape
encodings.utf_16
encodings.utf_16_be
encodings.utf_16_le
encodings.utf_32
encodings.utf_32_be
encodings.utf_32_le
encodings.utf_7
encodings.uu_codec
encodings.zlib_codec
ensurepip.__main__
idlelib
idlelib\..+
importlib.readers
importlib.resources.readers
importlib.resources.simple
importlib.simple
json.scanner
lib2to3.__main__
lib2to3.btm_utils
lib2to3.fixer_util
lib2to3.patcomp
lib2to3.pgen2.conv
lib2to3.tests
lib2to3\.tests\..+
nt
sqlite3.dump
tkinter.__main__
tkinter.test
tkinter\.test\..+
turtledemo
turtledemo\..+
unittest.__main__
unittest.test
unittest\.test\..+
venv.__main__
xml.sax.expatreader

@AlexWaygood
Copy link
Member Author

Friendly ping @hauntsaninja -- do you like the idea here? :)

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, although at this point in typeshed's life it might not be too helpful.

Fwiw, this is the sys.stdlib_module_names equivalent I use on Python 3.9:

all_stdlib = set()
importers = filter(
    lambda x: isinstance(x, importlib.machinery.FileFinder), pkgutil.iter_importers()
)
for importer in importers:
    modules = {m for m, _ in pkgutil.iter_importer_modules(importer)}
    # Hacky, but whatever
    if "json" in modules or "_json" in modules:
        if False:  # if you were to want submodules
            all_stdlib.update(m.name for m in pkgutil.walk_packages([importer.path]))
        else:
            all_stdlib.update(modules)
all_stdlib.update(sys.builtin_module_names)
return all_stdlib

(Hm having posted that, it could actually be nice for this code to not actually import the modules like above, but ofc doesn't matter stubtest's purposed)

@AlexWaygood AlexWaygood marked this pull request as draft August 23, 2023 18:22
@AlexWaygood AlexWaygood marked this pull request as ready for review August 23, 2023 18:30
@AlexWaygood
Copy link
Member Author

Fwiw, this is the sys.stdlib_module_names equivalent I use on Python 3.9

Nice! I pushed a variant of that. I've also pushed some changes so that we actually check submodules are importable before adding them to the set -- there are some which are not importable on all platforms.

although at this point in typeshed's life it might not be too helpful

I'd appreciate being able to keep track, I think -- my motivation for making this PR was that I realised we were missing several public importlib submodules that had been added in recent Python versions, and I was surprised stubtest wasn't complaining about their absence.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, let's do it!

@hauntsaninja hauntsaninja merged commit 0b303b5 into python:master Aug 23, 2023
13 checks passed
@AlexWaygood AlexWaygood deleted the stubtest-missing-stdlib branch August 24, 2023 05:02
@AlexWaygood AlexWaygood restored the stubtest-missing-stdlib branch February 22, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants