[ty] Improve diagnostic range for non-subscriptable diagnostics#21461
[ty] Improve diagnostic range for non-subscriptable diagnostics#21461AlexWaygood merged 1 commit intomainfrom
non-subscriptable diagnostics#21461Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
de9cc2a to
6b45718
Compare
|
I think highlighting the whole expression (as you do here) is best. It is both the type of the base object (which is the type that is not subscriptable), and the fact that you are subscripting it, that's "at fault" for the error. (In the case of not-subscriptable, the actual key is arguably not involved at all! But it still makes sense to highlight the entire expression IMO.) |
|
Similar to attribute expressions, I think it's undesired to highlight the entire object because it can get very noisy if that expression is long (think a call expression) I think the ideal is to highlight .attribute or [value]. Related discussion astral-sh/ty#169 I don't think this should block this PR because it at least makes it consistent but we should update said issue to also mention subscript |
|
Heh, well, I disagree for the same reasons I gave in astral-sh/ty#169 ;) but I'll add a note in that issue to mention that the same discussion also applies to subscript diagnostics! |
* origin/main: (59 commits) [ty] Improve diagnostic range for `non-subscriptable` diagnostics (#21461) [ty] Improve literal promotion heuristics (#21439) [ty] Further improve details around which expressions should be deferred in stub files (#21456) [ty] Improve generic class constructor inference (#21442) [ty] Propagate type context through conditional expressions (#21443) [ty] Suppress completions when introducing names with `as` [ty] Add panic-by-default await methods to `TestServer` (#21451) [ty] name is parameter and global is a syntax error (#21312) [ty] Fixup a few details around version-specific dataclass features (#21453) [ty] Support attribute-expression `TYPE_CHECKING` conditionals (#21449) [ty] Support stringified annotations in value-position `Annotated` instances (#21447) [ty] Type inference for genererator expressions (#21437) [ty] Make `__getattr__` available for `ModuleType` instances (#21450) [ty] Increase default receive timeout in tests to 10s (#21448) [ty] Add synthetic members to completions on dataclasses (#21446) [ty] Support legacy `typing` special forms in implicit type aliases (#21433) Bump 0.14.5 (#21435) [ty] Support `type[…]` and `Type[…]` in implicit type aliases (#21421) [ty] Respect notebook cell boundaries when adding an auto import (#21322) Update PyCharm setup instructions (#21409) ...
* dcreager/deep-comparison: (64 commits) assuming SubtypingAssuming implies_subtype_of name tweak Apply suggestions from code review [ty] Improve diagnostic range for `non-subscriptable` diagnostics (#21461) [ty] Improve literal promotion heuristics (#21439) [ty] Further improve details around which expressions should be deferred in stub files (#21456) [ty] Improve generic class constructor inference (#21442) [ty] Propagate type context through conditional expressions (#21443) [ty] Suppress completions when introducing names with `as` [ty] Add panic-by-default await methods to `TestServer` (#21451) [ty] name is parameter and global is a syntax error (#21312) [ty] Fixup a few details around version-specific dataclass features (#21453) [ty] Support attribute-expression `TYPE_CHECKING` conditionals (#21449) [ty] Support stringified annotations in value-position `Annotated` instances (#21447) [ty] Type inference for genererator expressions (#21437) [ty] Make `__getattr__` available for `ModuleType` instances (#21450) [ty] Increase default receive timeout in tests to 10s (#21448) [ty] Add synthetic members to completions on dataclasses (#21446) ...
Summary
Currently our diagnostic only covers the range of the thing being subscripted:
But it should probably cover the whole subscript expression (arguably the more "incorrect" bit is the
["foo"]part of this expression, not thexpart of this expression!)Test Plan
Added a snapshot
Co-authored-by: Brent Westbrook 36778786+ntBre@users.noreply.github.com