[ty] Update goto range for attribute access to only target the attribute #19848
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
Thank you. This makes sense to me. I still think it should be consistent with how we highlight attribute access in diagnostics, where other Python LSPs also only highlight the attribute part (and not the object). Which makes sense to me, but it seems we don't have alignment as a team. |
|
I'm not sure I see a strong argument for consistency here -- to me, the range we show for an invalid attribute-access operation seems like a pretty different concept to the range we use as the target for jumping to the original definition/declaration of an attribute. I don't have a very strong opinion, though. |
|
I guess that depends on the reasoning why it's preferable to use I did a quick check and TypeScript, Pyright, and Rust all highlight the attribute portion only in diagnostics and in goto definition. I still think we should just change our diagnostics. Maybe I should stop arguing and just merge this to have another reason why we should change the diagnostic ;) |
|
I'm okay going forward as it brings me closer to my goal, changing the range on the diagnostic and it matches my expectation for go to definition ;) It's also something that's easy to revert. Thank you |
* dcreager/bound-typevar: (41 commits) [ty] Use separate Rust types for bound and unbound type variables (#19796) fix ide tests better unbound typevar rendering Apply suggestions from code review [ty] Add `static-frame` as a walltime benchmark (#19844) add explanatory comment [ty] Update goto range for attribute access to only target the attribute (#19848) remove unneeded ord add TODO for broken hover test better PEP 695 binding context Add AIR301 rule (#17707) Avoid underflow in default ranges before a BOM (#19839) Update actions/download-artifact digest to de96f46 (#19852) Update docker/login-action action to v3.5.0 (#19860) Update rui314/setup-mold digest to 7344740 (#19853) Update cargo-bins/cargo-binstall action to v1.14.4 (#19855) Update actions/cache action to v4.2.4 (#19854) Update Rust crate hashbrown to v0.15.5 (#19858) Update Rust crate camino to v1.1.11 (#19857) Update Rust crate proc-macro2 to v1.0.96 (#19859) ...
Summary
We previously highlighted the whole attribute access for goto definition. This does not align with other language servers (rust-analyzer, python, etc).
More related discussion has been had here astral-sh/ty#169.
But I think for goto definition it seems unintuitive to highlight the whole expression
Test Plan
Update ty_ide tests