[ty] Pull types on synthesized Python files created by mdtest#18539
Merged
AlexWaygood merged 2 commits intomainfrom Jun 12, 2025
Merged
[ty] Pull types on synthesized Python files created by mdtest#18539AlexWaygood merged 2 commits intomainfrom
AlexWaygood merged 2 commits intomainfrom
Conversation
AlexWaygood
added
testing
AlexWaygood
force-pushed
the
alex/pulltypes-mdtest
branch
from
a172553 to
92631b8
Compare
AlexWaygood
commented
Jun 7, 2025
Contributor
|
AlexWaygood
force-pushed
the
alex/pulltypes-mdtest
branch
from
92631b8 to
331b000
Compare
Contributor
|
AlexWaygood
requested review from
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
AlexWaygood
force-pushed
the
alex/pulltypes-mdtest
branch
from
331b000 to
1a73898
Compare
MichaReiser
reviewed
Jun 9, 2025
MichaReiser
approved these changes
Jun 9, 2025
AlexWaygood
force-pushed
the
alex/pulltypes-mdtest
branch
from
1a73898 to
eac35f7
Compare
AlexWaygood
force-pushed
the
alex/pulltypes-mdtest
branch
2 times, most recently
from
56d9ab2 to
9be052f
Compare
AlexWaygood
force-pushed
the
alex/pulltypes-mdtest
branch
from
9be052f to
377eb7d
Compare
Member
Author
|
Okay, this has now been reimplemented as a "proper mdtest feature", that can be disabled on a per-test basis by adding the This is what it looks like if the step fails with an unexpected error:
And this is what an unexpected success looks like:
|
MichaReiser
reviewed
Jun 12, 2025
| [dev-dependencies] | ||
| ruff_db = { workspace = true, features = ["testing", "os"] } | ||
| ruff_python_parser = { workspace = true } | ||
| ty_python_semantic = { workspace = true, features = ["testing"] } |
Member
There was a problem hiding this comment.
Huh, I'm surprised that a crate is allowed to depend on itself.
MichaReiser
approved these changes
Jun 12, 2025
dcreager
added a commit
that referenced
this pull request
Jun 12, 2025
* main: [ty] Add some "inside string" tests for `object.<CURSOR>` completions [ty] Pull types on synthesized Python files created by mdtest (#18539) Update Rust crate anstyle to v1.0.11 (#18583) [`pyupgrade`] Fix `super(__class__, self)` detection in UP008 (super-call-with-parameters) (#18478) [ty] Generate the top and bottom materialization of a type (#18594) `SourceOrderVisitor` should visit the `Identifier` part of the `PatternKeyword` node (#18635) Update salsa (#18636) [ty] Update mypy_primer doc (#18638) [ty] Improve support for `object.<CURSOR>` completions [ty] Add `CoveringNode::find_last` [ty] Refactor covering node representation [ty] Infer the Python version from `--python=<system installation>` on Unix (#18550) [`flake8-return`] Fix `RET504` autofix generating a syntax error (#18428) Fix incorrect salsa `return_ref` attribute (#18605) Move corpus tests to `ty_python_semantic` (#18609) [`pyupgrade`] Don't offer fix for `Optional[None]` in non-pep604-annotation-optional (`UP045)` or non-pep604-annotation-union (`UP007`) (#18545) [`pep8-naming`] Suppress fix for `N804` and `N805` if the recommend name is already used (#18472) [`ruff`] skip fix for `RUF059` if dummy name is already bound (unused-unpacked-variable) (#18509)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Our "corpus tests" run an AST visitor that attempts to exhaustively check that each sub-expression in an AST has a type inferred for it. This is a great way to catch potential panics that might happen when you're hovering over an expression in an IDE, for example, since ty will try to retrieve the type of that node so that the IDE can show a nice tooltip to the user.
The corpus tests have a problem, though. There are a number of typing symbols to which ty applies heavy special casing. It's quite common for us to forget to store a type for a subexpression when inferring types for these heavily special-cased symbols, especially in cases where these symbols are being used invalidly. Our corpus of Python snippets that we run the "corpus tests" over doesn't provide great coverage for these symbols, so it's easy for these mistakes to go unnnoticed.
We do have a corpus of Python snippets that do have good coverage for these symbols, however, and which even have pretty good coverage of these symbols being used invalidly: the Python files embedded inside our mdtests! Running the
PullTypesVisitorover all snippets in our Markdown files should give us a much higher level of confidence that we're not forgetting to store types for any subexpressions in any files.This PR adds a step to mdtest that runs the
PullTypesVisitorover every synthesized file created by the mdtest framework. There are six existing mdtest suites that fail this new step; these Markdown files have been added to aKNOWN_PULL_TYPES_FAILURESlist in thety_testcrate.Test Plan
cargo test