Skip to content

Conversation

@MichaReiser
Copy link
Member

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_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)

Test Plan

Added test

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)
@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Dec 1, 2025
@astral-sh-bot
Copy link

astral-sh-bot bot commented Dec 1, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@AlexWaygood AlexWaygood removed their request for review December 1, 2025 15:06
@MichaReiser MichaReiser requested review from Gankra and removed request for carljm, dcreager and sharkdp December 1, 2025 15:07
@astral-sh-bot
Copy link

astral-sh-bot bot commented Dec 1, 2025

mypy_primer results

No ecosystem changes detected ✅

No memory usage changes detected ✅

@Gankra
Copy link
Contributor

Gankra commented Dec 1, 2025

Hmm interesting, ideally we would resolve both..? Especially if you're in a codebase that maintains the .py and .pyi. I wonder what different LSPs do...

@MichaReiser
Copy link
Member Author

Hmm interesting, ideally we would resolve both..? Especially if you're in a codebase that maintains the .py and .pyi. I wonder what different LSPs do...

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 Path in ty_benchmark as Pylance

Copy link
Contributor

@Gankra Gankra left a 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.

@MichaReiser
Copy link
Member Author

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

@MichaReiser MichaReiser merged commit 3738ab1 into main Dec 1, 2025
43 checks passed
@MichaReiser MichaReiser deleted the micha/fix-references-for-type-defined-in-stub branch December 1, 2025 16:53
dcreager added a commit that referenced this pull request Dec 2, 2025
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants