-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ty] Fix find references for type defined in stub #21732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ty] Fix find references for type defined in stub #21732
Conversation
ty failed to return any references for a type defined in a stub where there's also a definition in a non-stub file. The underlying issue was that `find_references` resolved the name to "go-to definition" whereas the visitor resolved the declaration, which can resolve to a different file and location. This fix uses go to declaration in both steps. I don't think it really matters whether we use go to declaration or go to definition because both should resolve to the same references. I picked go to declaration because it requires less work (no stub mapping)
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
Hmm interesting, ideally we would resolve both..? Especially if you're in a codebase that maintains the |
We only use the first navigation target to test if they're the same. As far as I could tell, ty now shows the same references when finding references to |
Gankra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly this makes more sense than what's currently there.
|
I'm certainly not saying this is the final solution. It's just one of the first issues I found while trying to wrap my head around how find references works |
* origin/main: (67 commits) Move `Token`, `TokenKind` and `Tokens` to `ruff-python-ast` (#21760) [ty] Don't confuse multiple occurrences of `typing.Self` when binding bound methods (#21754) Use our org-wide Renovate preset (#21759) Delete `my-script.py` (#21751) [ty] Move `all_members`, and related types/routines, out of `ide_support.rs` (#21695) [ty] Fix find-references for import aliases (#21736) [ty] add tests for workspaces (#21741) [ty] Stop testing the (brittle) constraint set display implementation (#21743) [ty] Use generator over list comprehension to avoid cast (#21748) [ty] Add a diagnostic for prohibited `NamedTuple` attribute overrides (#21717) [ty] Fix subtyping with `type[T]` and unions (#21740) Use `npm ci --ignore-scripts` everywhere (#21742) [`flake8-simplify`] Fix truthiness assumption for non-iterable arguments in tuple/list/set calls (`SIM222`, `SIM223`) (#21479) [`flake8-use-pathlib`] Mark fixes unsafe for return type changes (`PTH104`, `PTH105`, `PTH109`, `PTH115`) (#21440) [ty] Fix auto-import code action to handle pre-existing import Enable PEP 740 attestations when publishing to PyPI (#21735) [ty] Fix find references for type defined in stub (#21732) Use OIDC instead of codspeed token (#21719) [ty] Exclude `typing_extensions` from completions unless it's really available [ty] Fix false positives for `class F(Generic[*Ts]): ...` (#21723) ...
Summary
ty failed to return any references for a type defined in a stub where
there's also a definition in a non-stub file.
The underlying issue was that
find_referencesresolved the name to"go-to definition" whereas the visitor resolved the declaration, which
can resolve to a different file and location.
This fix uses go to declaration in both steps. I don't think it really
matters whether we use go to declaration or go to definition because
both should resolve to the same references. I picked go to declaration
because it requires less work (no stub mapping)
Test Plan
Added test