Conversation
When a union of TypedDicts is defined via a PEP 695 type alias the narrowing logic now correctly resolves the type alias and narrows the union based on literal field comparisons. Previously, the `is_typeddict_or_union_with_typeddicts` and `all_matching_typeddict_fields_have_literal_types` functions did not handle `Type::TypeAlias`, causing narrowing to fail for PEP 695 type aliases while working correctly for direct unions and legacy type aliases. Fixes astral-sh/ty#2645
Typing conformance resultsNo changes detected ✅ |
AlexWaygood
approved these changes
Jan 27, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
you could consider making these match statements exhaustive so it's impossible for us to forget to update them if/when we add another variant like Type::TypeAlias that just has to delegate
|
Contributor
Author
|
Ok, this PR kind of metastasized a bit; I could try to split it up but since the changes are all in one file, that would be slightly painful -- I don't think it's too big to be reasonably reviewable. All changes are tested and I verified every code change here is necessary to fix some test that would otherwise fail. |
AlexWaygood
approved these changes
Jan 27, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
LGTM if the primer report is clean!
Comment on lines
+1213
to
+1221
| /// If this type is a `Type::TypeAlias`, recursively resolves it to its | ||
| /// underlying value type. Otherwise, returns `self` unchanged. | ||
| pub(crate) fn resolve_type_alias(self, db: &'db dyn Db) -> Type<'db> { | ||
| let mut ty = self; | ||
| while let Type::TypeAlias(alias) = ty { | ||
| ty = alias.value_type(db); | ||
| } | ||
| ty | ||
| } |
Member
There was a problem hiding this comment.
this relates to @mtshiba's comment in #22786 (comment) -- ideally we'd have all of our Type::as_* methods account for type aliases and use this method under the hood; they're too much of a bug magnet otherwise.
But even that doesn't fully solve the problem, because there are other Type variants for which delegation is also often or always important: NewType, TypeVar, Union, Intersection, etc.
Anyway, what you have seems fine for now
Contributor
Author
There was a problem hiding this comment.
I think doing this for type aliases would make sense for sure. The other cases are a bit more situational. I'll do that as a follow-up.
carljm
changed the title
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
type-assertion-failure |
0 | 2 | 0 |
invalid-return-type |
0 | 0 | 1 |
| Total | 0 | 2 | 1 |
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
2 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.
A number of our narrowing cases didn't account for PEP 695 type aliases. The narrowing logic now correctly resolves the type alias and narrows accordingly.
Fixes astral-sh/ty#2645