Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[red-knot] Goto type definition #16901

Draft
wants to merge 6 commits into
base: micha/knot-ide
Choose a base branch
from
Draft

[red-knot] Goto type definition #16901

wants to merge 6 commits into from

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Mar 21, 2025 •
edited
Loading

Summary

Implement Goto type definition support for Red Knot's LSP.

TODOs:

Known limitations

  • Types defined in the vendored typeshed aren't supported because the client can't open those files (we can either implement our own file protocol and handler OR extract the typeshed files and point there)
  • Some nodes and types aren't supported yet. I added inline TODOs explaining what's missing
  • We may want to have a higher level API for the LSP that doesn't call semantic queries directly. I intentionally decided not to design that API just yet.

MichaReiser
MichaReiser commented Mar 21, 2025
View reviewed changes
crates/red_knot_server/src/document/text_document.rs Outdated
if new_contents != old_contents {
active_index = LineIndex::from_source_text(&new_contents);
}
active_index = LineIndex::from_source_text(&new_contents);

This comment was marked as outdated.

Sign in to view

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 21, 2025 •
edited
Loading

mypy_primer results

Changes were detected when running on open source projects 
rich (https://github.com/Textualize/rich)
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/rich/rich/control.py:198:27: Object of type `dict | @Todo[crates/red_knot_python_semantic/src/types/infer.rs:3615]` cannot be assigned to parameter 2 (`table`) of bound method `translate`; expected type `_TranslateTable`
+ error[lint:invalid-argument-type] /tmp/mypy_primer/projects/rich/rich/control.py:198:27: Object of type `dict | @Todo[crates/red_knot_python_semantic/src/types/infer.rs:3616]` cannot be assigned to parameter 2 (`table`) of bound method `translate`; expected type `_TranslateTable`

@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch from f66cabc to 9b7cbc7 Compare March 21, 2025 16:43
@MichaReiser MichaReiser added the red-knot Multi-file analysis & type inference label Mar 21, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 21, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch 2 times, most recently from e42a8da to 6b8b185 Compare March 28, 2025 15:10
@MichaReiser MichaReiser mentioned this pull request Mar 28, 2025
Merged
MichaReiser added a commit that referenced this pull request Mar 28, 2025
@MichaReiser
Add as_group methods to AnyNodeRef (#17048)
e07741e 
## Summary

This PR adds `as_<group>` methods to `AnyNodeRef` to e.g. convert an
`AnyNodeRef` to an `ExprRef`.

I need this for go to definition where the fallback is to test if
`AnyNodeRef` is an expression and then call `inferred_type` (listing
this mapping at every call site where we need to convert `AnyNodeRef` to
an `ExprRef` is a bit painful ;))

Split out from #16901

## Test Plan

`cargo test`
@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch from 6b8b185 to 1d6dc13 Compare March 28, 2025 20:12
@MichaReiser MichaReiser changed the base branch from main to micha/knot-ide March 28, 2025 20:12
@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch from 1d6dc13 to 886e017 Compare March 28, 2025 20:13
@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch from 886e017 to 0ee9fd8 Compare March 28, 2025 20:19
@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch from bfd6a9b to 0a07abb Compare March 28, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carljm carljm Awaiting requested review from carljm carljm will be requested when the pull request is marked ready for review carljm is a code owner

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood will be requested when the pull request is marked ready for review AlexWaygood is a code owner

@sharkdp sharkdp Awaiting requested review from sharkdp sharkdp will be requested when the pull request is marked ready for review sharkdp is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager will be requested when the pull request is marked ready for review dcreager is a code owner

@dhruvmanila dhruvmanila Awaiting requested review from dhruvmanila dhruvmanila will be requested when the pull request is marked ready for review dhruvmanila is a code owner

Assignees
No one assigned
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@MichaReiser