-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Check typeshed VERSIONS for parent modules when reporting failed stdlib imports #20908
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
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
(Broken into two commits so you can see the before and after on the snapshot) |
|
Can't help but wonder if we could statically precompute a Complete VERSIONS map... |
AlexWaygood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh thank you!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nice -- can you move these existing tests to this file so that all our tests for this feature are in the same place?
ruff/crates/ty_python_semantic/resources/mdtest/import/basic.md
Lines 180 to 207 in 5fb1423
| ## Attempting to import a stdlib module that's not yet been added | |
| <!-- snapshot-diagnostics --> | |
| ```toml | |
| [environment] | |
| python-version = "3.10" | |
| ``` | |
| ```py | |
| import tomllib # error: [unresolved-import] | |
| from string.templatelib import Template # error: [unresolved-import] | |
| from importlib.resources import abc # error: [unresolved-import] | |
| ``` | |
| ## Attempting to import a stdlib module that was previously removed | |
| <!-- snapshot-diagnostics --> | |
| ```toml | |
| [environment] | |
| python-version = "3.13" | |
| ``` | |
| ```py | |
| import aifc # error: [unresolved-import] | |
| from distutils import sysconfig # error: [unresolved-import] | |
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this but I completely misread the direction you wanted this to go so instead my new test file is deleted and I added a new test to basic.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, I don't mind either way, I'd just like all the tests for the feature to be together!
8f49ef0 to
d08d591
Compare
…rable * origin/main: [ty] Support dataclass-transform `field_specifiers` (#20888) Bump 0.14.1 (#20925) Standardize syntax error construction (#20903) [`pydoclint`] Implement `docstring-extraneous-parameter` (`DOC102`) (#20376) [ty] Fix panic 'missing root' when handling completion request (#20917) [ty] Run file watching tests serial when using nextest (#20918) [ty] Add version hint for failed stdlib attribute accesses (#20909) More CI improvements (#20920) [ty] Check typeshed VERSIONS for parent modules when reporting failed stdlib imports (#20908)
* main: [ty] Prefer declared type for invariant collection literals (#20927) [ty] Don't track inferability via different `Type` variants (#20677) [ty] Use declared variable types as bidirectional type context (#20796) [ty] Avoid unnecessarily widening generic specializations (#20875) [ty] Support dataclass-transform `field_specifiers` (#20888) Bump 0.14.1 (#20925) Standardize syntax error construction (#20903) [`pydoclint`] Implement `docstring-extraneous-parameter` (`DOC102`) (#20376) [ty] Fix panic 'missing root' when handling completion request (#20917) [ty] Run file watching tests serial when using nextest (#20918) [ty] Add version hint for failed stdlib attribute accesses (#20909) More CI improvements (#20920) [ty] Check typeshed VERSIONS for parent modules when reporting failed stdlib imports (#20908)
This is a drive-by improvement that I stumbled backwards into while looking into
I was writing some simple tests for "thing not in old version of stdlib" diagnostics and checked what was added in 3.14, and saw
compression.zstdand to my surprise discovered thatimport compression.zstdandfrom compression import zstdhad completely different quality diagnostics.This is because
compressionandcompression.zstdwere both introduced in 3.14, and so per VERSIONS policy only an entry forcompressionwas added, and so we don't actually have any definite info oncompression.zstdand give up on producing a diagnostic. However thefrom compression import zstdform fails on looking upcompressionand we do have an exact match for that, so it gets a better diagnostic!(aside: I have now learned about the VERSIONS format and I really wish they would just enumerate all the submodules but, oh well!)
The fix is, when handling an import failure, if we fail to find an exact match we requery with the parent module. In cases like
compression.zstdthis lets us at least identify that, hey, not evencompressionexists, and luckily that fixes the whole issue. In cases where the parent module and submodule were introduced at different times then we may discover that the parent module is in-range and that's fine, we don't produce the richer stdlib diagnostic.