[ty] Support __setitem__ and improve __getitem__ related diagnostics#19578
Merged
sharkdp merged 22 commits intoastral-sh:mainfrom Aug 1, 2025
Merged
[ty] Support __setitem__ and improve __getitem__ related diagnostics#19578sharkdp merged 22 commits intoastral-sh:mainfrom
__setitem__ and improve __getitem__ related diagnostics#19578sharkdp merged 22 commits intoastral-sh:mainfrom
Conversation
MatthewMckee4
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
MatthewMckee4
commented
Jul 27, 2025
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
Contributor
|
Contributor
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
call-non-callable |
0 | 182 | 0 |
invalid-argument-type |
154 | 0 | 0 |
invalid-assignment |
114 | 0 | 0 |
possibly-unbound-implicit-call |
75 | 0 | 0 |
unused-ignore-comment |
0 | 20 | 0 |
possibly-unresolved-reference |
0 | 2 | 0 |
| Total | 343 | 204 | 0 |
sharkdp
reviewed
Jul 29, 2025
Contributor
sharkdp
left a comment
sharkdp
left a comment
There was a problem hiding this comment.
Thank you very much, this looks good.
Did you have a look at the ecosystem changes? You can download a detailed diff with links to the original source here. It looks like some diagnostics simply transformed from non-subscriptable to invalid-item-assignment, but there are also a few new diagnostics which might be worth looking into.
| s = c.x # error: [invalid-assignment] | ||
|
|
||
| # TODO: This assignment is invalid and should result in an error. | ||
| # error: [call-non-callable] "Method `__setitem__` of type `Overload[(key: SupportsIndex, value: int, /) -> None, (key: slice[Any, Any, Any], value: Iterable[int], /) -> None]` is not callable on object of type `list[int]`" |
Contributor
There was a problem hiding this comment.
Maybe similar to your comment below, but this error message is really confusing, because it seems to imply that something is wrong with the type on which __setitem__ is called. But it's just the value argument which doesn't match.
Contributor
Author
There was a problem hiding this comment.
Yeah exactly, I'll try to change that in this PR then, for __getitem__ too
Contributor
Author
There was a problem hiding this comment.
Updated them, they look better but not sure if they cover all cases of BindingError
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/subscript/instance.md
Outdated
Show resolved
Hide resolved
| /// ```python | ||
| /// 4[1] = 2 # TypeError: 'int' object does not support item assignment | ||
| /// ``` | ||
| pub(crate) static INVALID_ITEM_ASSIGNMENT = { |
Contributor
There was a problem hiding this comment.
For invalid attribute assignments, we simply use the invalid-assignment diagnostic, instead of creating a dedicated new lint rule (there is invalid-attribute-access, but that's used for something slightly different, like when trying to assign to an instance attribute from a class object). So I think we should do the same here?
/cc @MichaReiser
Member
There was a problem hiding this comment.
I don't have much context on the existing rules. The reasons for separate rules are:
- They have different accuracy (which may result in different default severities or categories)
- If they are conceptually different enough or people have different opinions on one aspect and the other (there are good reasons to only enable one but not the other)
Otherwise, I'd go with invalid-assignment
Contributor
There was a problem hiding this comment.
I would claim that none of these criteria apply here, so I would suggest we remove that new lint rule and use invalid-assignment.
Co-authored-by: David Peter <[email protected]>
Co-authored-by: David Peter <[email protected]>
Co-authored-by: David Peter <[email protected]>
Co-authored-by: David Peter <[email protected]>
Co-authored-by: David Peter <[email protected]>
Co-authored-by: David Peter <[email protected]>
Contributor
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-08-01 07:08:56.976522715 +0000
+++ new-output.txt 2025-08-01 07:08:57.039522808 +0000
@@ -368,8 +368,8 @@
generics_basic.py:34:12: error[unsupported-operator] Operator `+` is unsupported between objects of type `AnyStr` and `AnyStr`
generics_basic.py:139:5: error[type-assertion-failure] Argument does not have asserted type `int`
generics_basic.py:140:5: error[type-assertion-failure] Argument does not have asserted type `int`
-generics_basic.py:157:5: error[call-non-callable] Method `__getitem__` of type `bound method MyMap1[str, int].__getitem__(key: str, /) -> int` is not callable on object of type `MyMap1[str, int]`
-generics_basic.py:158:5: error[call-non-callable] Method `__getitem__` of type `bound method MyMap2[int, str].__getitem__(key: str, /) -> int` is not callable on object of type `MyMap2[int, str]`
+generics_basic.py:157:5: error[invalid-argument-type] Method `__getitem__` of type `bound method MyMap1[str, int].__getitem__(key: str, /) -> int` cannot be called with key of type `Literal[0]` on object of type `MyMap1[str, int]`
+generics_basic.py:158:5: error[invalid-argument-type] Method `__getitem__` of type `bound method MyMap2[int, str].__getitem__(key: str, /) -> int` cannot be called with key of type `Literal[0]` on object of type `MyMap2[int, str]`
generics_basic.py:162:12: error[invalid-argument-type] `<class 'int'>` is not a valid argument to `Generic`
generics_basic.py:163:12: error[invalid-argument-type] `<class 'int'>` is not a valid argument to `Protocol`
generics_basic.py:171:1: error[invalid-generic-class] `Generic` base class must include all type variables used in other base classes
@@ -704,6 +704,7 @@
namedtuples_usage.py:30:1: error[type-assertion-failure] Argument does not have asserted type `str`
namedtuples_usage.py:31:1: error[type-assertion-failure] Argument does not have asserted type `int`
namedtuples_usage.py:32:1: error[type-assertion-failure] Argument does not have asserted type `int`
+namedtuples_usage.py:41:1: error[invalid-assignment] Cannot assign to object of type `Point` with no `__setitem__` method
namedtuples_usage.py:49:1: error[type-assertion-failure] Argument does not have asserted type `int`
namedtuples_usage.py:50:1: error[type-assertion-failure] Argument does not have asserted type `str`
narrowing_typeguard.py:17:9: error[type-assertion-failure] Argument does not have asserted type `tuple[str, str]`
@@ -724,7 +725,7 @@
narrowing_typeis.py:96:9: error[type-assertion-failure] Argument does not have asserted type `B`
narrowing_typeis.py:132:20: error[invalid-argument-type] Argument to function `takes_callable_str` is incorrect: Expected `(object, /) -> str`, found `def simple_typeguard(val: object) -> TypeIs[int]`
narrowing_typeis.py:191:18: error[invalid-argument-type] Argument to function `takes_int_typeis` is incorrect: Expected `(object, /) -> TypeIs[int]`, found `def bool_typeis(val: object) -> TypeIs[bool]`
-overloads_basic.py:39:1: error[call-non-callable] Method `__getitem__` of type `Overload[(__i: int) -> int, (__s: slice[Any, Any, Any]) -> bytes]` is not callable on object of type `Bytes`
+overloads_basic.py:39:1: error[invalid-argument-type] Method `__getitem__` of type `Overload[(__i: int) -> int, (__s: slice[Any, Any, Any]) -> bytes]` cannot be called with key of type `Literal[""]` on object of type `Bytes`
overloads_definitions.py:20:5: error[invalid-overload] Overloaded function `func1` requires at least two overloads
overloads_definitions.py:33:5: error[invalid-overload] Overloaded non-stub function `func2` must have an implementation
overloads_definitions.py:63:9: error[invalid-overload] Overloaded non-stub function `not_abstract` must have an implementation
@@ -888,4 +889,4 @@
tuples_type_form.py:36:1: error[invalid-assignment] Object of type `tuple[Literal[1], Literal[2], Literal[3], Literal[""]]` is not assignable to `tuple[int, ...]`
typeddicts_operations.py:60:1: error[type-assertion-failure] Argument does not have asserted type `str | None`
typeddicts_type_consistency.py:101:1: error[invalid-assignment] Object of type `Unknown | None` is not assignable to `str`
-Found 889 diagnostics
+Found 890 diagnostics |
Contributor
Just to clarify: I haven't reviewed again since it's still not clear to me if the ecosystem impact is correct. Let me know if you need help with that, happy to take a more detailed look. |
Contributor
Author
|
I'll review again after removing the new lint rule. It's also probably beneficial if I split this PR up. |
Contributor
Author
|
hmm class Foo:
def __setitem__(self, index: int, value: int) -> None:
pass
Foo()["a"] = 1Should invalid assignment or invalid argument type be emitted here? |
Contributor
I think we should emit |
MatthewMckee4
changed the title
__setitem____setitem__ and improve __getitem__ related diagnostics
Contributor
Author
|
@sharkdp would you be able to redo the ecosystem-analyzer? Thanks |
sharkdp
force-pushed
the
support-setitem
branch
from
78c15db to
24fe6e3
Compare
sharkdp
approved these changes
Aug 1, 2025
dcreager
added a commit
that referenced
this pull request
Aug 1, 2025
* main: (39 commits) [ty] Initial test suite for `TypedDict` (#19686) [ty] Improve debuggability of protocol types (#19662) [ty] Simplify lifetime requirements for `PySlice` trait (#19687) [ty] Improve `isinstance()` truthiness analysis for generic types (#19668) [`refurb`] Make example error out-of-the-box (`FURB164`) (#19673) Fix link: unused_import.rs (#19648) [ty] Remove `Specialization::display` (full) (#19682) [ty] Remove `KnownModule::is_enum` (#19681) [ty] Support `__setitem__` and improve `__getitem__` related diagnostics (#19578) [ty] Sync vendored typeshed stubs (#19676) [`flake8-use-pathlib`] Expand `PTH201` to check all `PurePath` subclasses (#19440) [`refurb`] Make example error out-of-the-box (`FURB180`) (#19672) [`pyupgrade`] Prevent infinite loop with `I002` (`UP010`, `UP035`) (#19413) [ty] Improve the `Display` for generic `type[]` types (#19667) [ty] Refactor `TypeInferenceBuilder::infer_subscript_expression_types` (#19658) Fix tests on 32-bit architectures (#19652) [ty] Move `pandas-stubs` to bad.txt (#19659) [ty] Remove special casing for string-literal-in-tuple `__contains__` (#19642) Update pre-commit's `ruff` id (#19654) Update salsa (#19449) ...
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
4 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
Adds validation to subscript assignment expressions.
Also improves error messages on invalid
__getitem__expressionsTest Plan
Update mdtests and add more to
subscript/instance.md