Skip to content

Comments

[ty] Fix panic when trying to pull types for subscript expressions inside Callable type expressions#18534

Merged
AlexWaygood merged 4 commits intomainfrom
alex/concatenate-crash
Jun 9, 2025
Merged

[ty] Fix panic when trying to pull types for subscript expressions inside Callable type expressions#18534
AlexWaygood merged 4 commits intomainfrom
alex/concatenate-crash

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jun 7, 2025 •
edited
Loading

Summary

We weren't storing types for subscript expressions inside the parameter list of Callable type expressions; this was the cause of the corpus test failures for two files in our vendored copy of typeshed.

Test Plan

  • Removed the remaining typeshed entries from the KNOWN_FAILURES corpus list
  • Added a standalone corpus test so that we still have coverage for this in case typeshed changes in the future
  • Added an mdtest to cover the regression surfaced by primer on the first version of this PR

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Jun 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅

@AlexWaygood AlexWaygood marked this pull request as draft June 7, 2025 13:37
@AlexWaygood AlexWaygood marked this pull request as ready for review June 7, 2025 14:39
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jun 7, 2025

The new primer hits are because for this snippet:

from basedtyping import P

reveal_type(P)

We infer the type of P as ParamSpec | Unknown rather than ParamSpec, which means that we don't see it as a valid first argument to Callable. This is a pre-existing issue (and something of an edge case), we just see it in more cases now because we're now recursing inside Concatenate slices when inferring types.

@AlexWaygood AlexWaygood force-pushed the alex/concatenate-crash branch 2 times, most recently from 6f76d88 to 7f6f3e2 Compare June 7, 2025 15:27
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jun 7, 2025

With this PR, we also succeed in pulling types for the entirety of typeshed's stubs/ directory as well as its stdlib/ directory! Except for any files where the final segment is types.pyi: we still overflow our stack on all of those.

@AlexWaygood AlexWaygood force-pushed the alex/concatenate-crash branch from ecf92a6 to 8e30df3 Compare June 7, 2025 15:38
@AlexWaygood AlexWaygood mentioned this pull request Jun 7, 2025
Merged
dhruvmanila
dhruvmanila approved these changes Jun 9, 2025
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

crates/ty_python_semantic/src/types/infer.rs
Comment on lines 9372 to 9465
if arguments_slice.is_tuple_expr() {
self.store_expression_type(arguments_slice, ty);
}
Copy link
Member

@dhruvmanila dhruvmanila Jun 9, 2025

Choose a reason for hiding this comment

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

This is only being done for tuple expressions because otherwise it would already be present via the self.infer_type_expression(argument) call?

Copy link
Member Author

@AlexWaygood AlexWaygood Jun 9, 2025

Choose a reason for hiding this comment

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

Yup, exactly!

@AlexWaygood AlexWaygood force-pushed the alex/concatenate-crash branch from 8e30df3 to bd4a0f2 Compare June 9, 2025 10:18
@AlexWaygood AlexWaygood force-pushed the alex/concatenate-crash branch from bd4a0f2 to 021b2db Compare June 9, 2025 10:19
@AlexWaygood AlexWaygood merged commit aa3c312 into main Jun 9, 2025
35 checks passed
@AlexWaygood AlexWaygood deleted the alex/concatenate-crash branch June 9, 2025 10:26
med1844 pushed a commit to med1844/ruff that referenced this pull request Jun 10, 2025
@AlexWaygood @med1844
[ty] Fix panic when trying to pull types for subscript expressions in…
75f074b 
…side `Callable` type expressions (astral-sh#18534)
@dhruvmanila dhruvmanila added the bug Something isn't working label Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhruvmanila dhruvmanila dhruvmanila approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@sharkdp sharkdp Awaiting requested review from sharkdp sharkdp is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

@MichaReiser MichaReiser Awaiting requested review from MichaReiser MichaReiser is a code owner

Assignees

No one assigned

Labels

bug Something isn't working ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AlexWaygood @dhruvmanila