Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jun 7, 2025

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

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

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

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.

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!

Comment on lines 9372 to 9465
if arguments_slice.is_tuple_expr() {
self.store_expression_type(arguments_slice, ty);
}
Copy link
Member

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

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
@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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants