Skip to content

stubtest: emit error if a stub defines a public type alias that doesn't exist at runtime #12608

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 18, 2022
Merged

stubtest: emit error if a stub defines a public type alias that doesn't exist at runtime #12608

merged 5 commits into from
Apr 18, 2022

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Apr 17, 2022 •
edited
Loading

Description

A followup to python/typeshed#7634 (comment)

Stubtest currently declines to error if a stub defines a public type alias that doesn't exist at runtime. But it should!

These are the new error reported from running stubtest on the typeshed stdlib, with this patch applied:

error: _threading_local.localdict is not present at runtime
Stub: at line 6
Type alias for: builtins.dict
Runtime:
MISSING

error: audioop.AdpcmState is not present at runtime
Stub: at line 3
Type alias for: Tuple[builtins.int, builtins.int]
Runtime:
MISSING

error: audioop.RatecvState is not present at runtime
Stub: at line 4
Type alias for: Tuple[builtins.int, builtins.tuple[Tuple[builtins.int, builtins.int], ...]]
Runtime:
MISSING

error: itertools.Predicate is not present at runtime
Stub: at line 14
Type alias for: def (_T?) -> builtins.object
Runtime:
MISSING

error: sqlite3.dbapi2.OptimizedUnicode is not present at runtime
Stub: at line 413
Type alias for: builtins.str
Runtime:
MISSING

They all look like true positives to me (or at least should be allowlisted in typeshed, rather than being ignored by stubtest altogether).

Test Plan

Two test cases added

@AlexWaygood AlexWaygood changed the title stubtest: emit error if a stub defines a public type alias that's missing at runtime stubtest: emit error if a stub defines a public type alias that doesn't exist at runtime Apr 17, 2022
hauntsaninja
hauntsaninja approved these changes Apr 18, 2022
View reviewed changes
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.

Thanks! I think it can be easier to preserve line numbers... I took the liberty of pushing changes directly to the branch, let me know what you think!

@AlexWaygood
Copy link
Member Author

I think it can be easier to preserve line numbers... I took the liberty of pushing changes directly to the branch, let me know what you think!

Nice, thank you!! That's... much simpler than my method 🤦‍♂️

Just one nit about error messages -- we currently have:

Stub: at line 4
Type alias for Tuple[builtins.int, builtins.tuple[Tuple[builtins.int, builtins.int], ...]]

I'd much prefer either this, separating the English from the code using a colon:

Stub: at line 4
Type alias for: Tuple[builtins.int, builtins.tuple[Tuple[builtins.int, builtins.int], ...]]

or this, separating the English from the code using quotation marks:

Stub: at line 4
Type alias for "Tuple[builtins.int, builtins.tuple[Tuple[builtins.int, builtins.int], ...]]"

What do you think?

hauntsaninja
hauntsaninja reviewed Apr 18, 2022
View reviewed changes
AlexWaygood
AlexWaygood commented Apr 18, 2022
View reviewed changes
Copy link
Member Author

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

I approve my own PR

@hauntsaninja hauntsaninja merged commit 9e9de71 into python:master Apr 18, 2022
@AlexWaygood AlexWaygood deleted the stubtest-typealiases branch April 18, 2022 20:27
@AlexWaygood
Copy link
Member Author

Thanks @hauntsaninja :)

@AlexWaygood AlexWaygood mentioned this pull request Apr 18, 2022
Merged
@hauntsaninja hauntsaninja mentioned this pull request May 26, 2022
Closed
@AlexWaygood AlexWaygood mentioned this pull request Sep 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hauntsaninja hauntsaninja hauntsaninja approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AlexWaygood @hauntsaninja