[ty] Further improve subscript assignment diagnostics #21411
Merged
Conversation
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
sharkdp
requested review from
AlexWaygood,
carljm and
dcreager
as code owners
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-11-13 12:10:32.296955997 +0000
+++ new-output.txt 2025-11-13 12:10:35.753979066 +0000
@@ -745,7 +745,7 @@
namedtuples_usage.py:34:7: error[index-out-of-bounds] Index 3 is out of bounds for tuple `Point` with length 3
namedtuples_usage.py:35:7: error[index-out-of-bounds] Index -4 is out of bounds for tuple `Point` with length 3
namedtuples_usage.py:40:1: error[invalid-assignment] Cannot assign to read-only property `x` on object of type `Point`
-namedtuples_usage.py:41:1: error[invalid-assignment] Cannot assign to a subscript on an object of type `Point` with no `__setitem__` method
+namedtuples_usage.py:41:1: error[invalid-assignment] Cannot assign to a subscript on an object of type `Point`
namedtuples_usage.py:52:1: error[invalid-assignment] Too many values to unpack: Expected 2
namedtuples_usage.py:53:1: error[invalid-assignment] Not enough values to unpack: Expected 4
narrowing_typeguard.py:17:9: error[type-assertion-failure] Argument does not have asserted type `tuple[str, str]`
@@ -970,8 +970,8 @@
typeddicts_extra_items.py:337:1: error[unresolved-attribute] Object of type `IntDictWithNum` has no attribute `clear`
typeddicts_extra_items.py:339:1: error[type-assertion-failure] Argument does not have asserted type `tuple[str, int]`
typeddicts_extra_items.py:339:13: error[unresolved-attribute] Object of type `IntDictWithNum` has no attribute `popitem`
-typeddicts_extra_items.py:342:27: error[invalid-key] Cannot access `IntDictWithNum` with a key of type `str`. Only string literals are allowed as keys on TypedDicts.
-typeddicts_extra_items.py:343:31: error[invalid-key] Invalid key of type `str` for TypedDict `IntDictWithNum`
+typeddicts_extra_items.py:342:27: error[invalid-key] TypedDict `IntDictWithNum` can only be subscripted with a string literal key, got key of type `str`.
+typeddicts_extra_items.py:343:31: error[invalid-key] TypedDict `IntDictWithNum` can only be subscripted with string literal keys, got key of type `str`
typeddicts_extra_items.py:352:5: error[invalid-assignment] Object of type `dict[str, int]` is not assignable to `IntDict`
typeddicts_operations.py:22:17: error[invalid-assignment] Invalid assignment to key "name" with declared type `str` on TypedDict `Movie`: value of type `Literal[1982]`
typeddicts_operations.py:23:17: error[invalid-assignment] Invalid assignment to key "year" with declared type `int` on TypedDict `Movie`: value of type `Literal[""]`
|
sharkdp
force-pushed
the
david/more-subscript-assignment-improvements
branch
from
0a77072 to
485003b
Compare
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-assignment |
0 | 0 | 630 |
invalid-key |
0 | 0 | 229 |
| Total | 0 | 0 | 859 |
Contributor
Author
|
|
carljm
approved these changes
Nov 12, 2025
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Thank you, this is fantastic!
|
|
||
| ``` | ||
| error[invalid-assignment]: Method `__setitem__` of type `bound method dict[str, int].__setitem__(key: str, value: int, /) -> None` cannot be called with a key of type `Literal[0]` and a value of type `Literal[3]` on object of type `dict[str, int]` | ||
| error[invalid-assignment]: Cannot assign to `dict[str, int]` with key of type `Literal[0]` and a value of type `Literal[3]` |
Contributor
There was a problem hiding this comment.
I notice that in the "not subscriptable" case we take care to say "Cannot assign to a subscript on object of type ...", but here we shorthand it to "Cannot assign to ...".
In general we've taken care in our diagnostics to say "object of type ..." rather than just conflating types and inhabitants, though tbh I'm not clear if that's worth the extra words.
I do think it's a bit odd to say "Cannot assign to dict[str, int]" when what we mean is "Cannot assign to subscript of dict[str, int]". But I also don't feel strongly if you prefer the concision.
Contributor
Author
There was a problem hiding this comment.
I wanted it to be more concise, yes. But I agree that the slightly longer form "Cannot assign to a subscript on object of type …" is better. Changed.
Comment on lines
3876
to
3877
| // Use `KnownClass` as a crude proxy for checking if this is not a user-defined class. Otherwise, | ||
| // we end up suggesting things like "`None` may be missing a `__setitem__` method." |
Contributor
There was a problem hiding this comment.
I don't think saying "None may be missing a __setitem__ method." would actually be the worst thing -- I wondered why it wasn't there when reading the tests. It is in fact the case that the reason for the diagnostic is that None has no __setitem__ method! But I'm also not opposed to what you do here.
Contributor
Author
There was a problem hiding this comment.
I chose the wording "Xyz may be missing a __setitem__ method" because it suggests to me that the user may want to consider adding such a method to Xyz, and that is certainly not a helpful hint for stdlib types. I changed it now to say "None does not have a __setitem__ method" for known classes.
AlexWaygood
approved these changes
Nov 13, 2025
...hots/assignment_diagnosti…_-_Subscript_assignment…_-_Invalid_key_type_(d3d47de65fb3bad).snap
Outdated
Show resolved
Hide resolved
...ssignment_diagnosti…_-_Subscript_assignment…_-_Invalid_key_type_for…_(815dae276e2fd2b7).snap
Outdated
Show resolved
Hide resolved
...ssignment_diagnosti…_-_Subscript_assignment…_-_No_`__setitem__`_met…_(468f62a3bdd1d60c).snap
Outdated
Show resolved
Hide resolved
AlexWaygood
reviewed
Nov 13, 2025
AlexWaygood
reviewed
Nov 13, 2025
Comment on lines
+3798
to
+3807
| if let Some([expected_key_ty, expected_value_ty]) = | ||
| object_ty | ||
| .known_specialization(db, KnownClass::Dict) | ||
| .map(|s| s.types(db)) | ||
| { | ||
| let mut diagnostic = builder.into_diagnostic(format_args!( | ||
| "Invalid subscript assignment with key of type `{}` and value of \ | ||
| type `{assigned_d}` on object of type `{object_d}`", | ||
| slice_ty.display(db), | ||
| )); |
Member
There was a problem hiding this comment.
I see that we can only give the nice subdiagnostic if it's a dict, but the top-level diagnostic summary here seems like it would be an improvement for non-dictionaries as well as dictionaries? The terms "key", "value", and "subscript assignment" aren't specific to dictionaries; they apply to any object with a __setitem__ method.
But I suppose that can be done as a separate change
sharkdp
added a commit
that referenced
this pull request
Nov 17, 2025
## Summary Follow up from #21411. Again, there are more things that could be improved here (like the diagnostics for `lists`, or extending what we have for `dict` to `OrderedDict` etc), but that will have to be postponed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Further improve subscript assignment diagnostics, especially for
dicts:I have many more ideas, but this looks like a reasonable first step. Thank you @AlexWaygood for some of the suggestions here.
Test Plan
Update tests