Skip to content

Comments

[red-knot] Add 'Goto type definition' to the playground#17055

Merged
MichaReiser merged 1 commit intomainfrom
micha/playground-goto-type-definition
Apr 2, 2025
Merged

[red-knot] Add 'Goto type definition' to the playground#17055
MichaReiser merged 1 commit intomainfrom
micha/playground-goto-type-definition

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Mar 29, 2025

Summary

This PR adds Goto type definition to the playground, using the same infrastructure as the LSP.

The main challenge with implementing this feature was that the editor can now participate in which tab is open.

Known limitations

The same as for the LSP. Most notably, navigating to types defined in typeshed isn't supported.

Test Plan

Screen.Recording.2025-04-01.at.13.20.13.mov

@MichaReiser MichaReiser added playground A playground-specific issue ty Multi-file analysis & type inference labels Mar 29, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2025

mypy_primer results

No ecosystem changes detected ✅

@MichaReiser MichaReiser force-pushed the micha/playground-goto-type-definition branch from b1f53a8 to a950f23 Compare March 29, 2025 11:48
@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch 2 times, most recently from 285e071 to 54c359b Compare April 1, 2025 09:20
@MichaReiser MichaReiser force-pushed the micha/playground-goto-type-definition branch from a950f23 to debd0c8 Compare April 1, 2025 11:39
@MichaReiser MichaReiser marked this pull request as ready for review April 1, 2025 11:41
@MichaReiser

This comment was marked as outdated.

@MichaReiser MichaReiser requested a review from Copilot April 1, 2025 11:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a “Goto type definition” feature to the playground by leveraging the existing LSP infrastructure and updating both the frontend editor and the Rust backend.

  • Introduces a helper method to detect Python files in Files.tsx.
  • Updates the Editor.tsx to register a type definition provider and handle file model creation and switching.
  • Adds a new Rust function in red_knot_wasm to power go-to-type-definition and updates dependency configuration.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
playground/knot/src/Editor/Files.tsx Added helper function to check if a file is a Python file
playground/knot/src/Editor/Editor.tsx Updated editor API to support type definition operations and file switching
playground/knot/src/Editor/Chrome.tsx Modified integration to pass new props to the editor
crates/red_knot_wasm/src/lib.rs Added Rust function for type definition lookup and enhanced range conversion
crates/red_knot_wasm/Cargo.toml Added dependency on red_knot_ide
crates/red_knot_ide/src/lib.rs Implemented file_range accessor for improved integration
Comments suppressed due to low confidence (2)

playground/knot/src/Editor/Editor.tsx:210

  • [nitpick] The variable name 'selectedFile' might be misleading if it represents a file identifier. Consider renaming it to 'selectedFileId' for enhanced clarity.
const selectedFile = this.props.current.files.selected;

playground/knot/src/Editor/Editor.tsx:260

  • [nitpick] The name 'fileId' might benefit from additional context (e.g. 'targetFileId') to clarify that it is derived from a file lookup based on the resource URI.
const fileId = files.index.find((file) => { return Uri.file(file.name).toString() === resource.toString(); })?.id;

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2025

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 from 9cf6900 to d6aa201 Compare April 1, 2025 16:19
@MichaReiser MichaReiser requested a review from dhruvmanila as a code owner April 1, 2025 16:19
@MichaReiser MichaReiser force-pushed the micha/playground-goto-type-definition branch from debd0c8 to ae875e7 Compare April 1, 2025 16:19
@MichaReiser MichaReiser force-pushed the micha/go-to-type-definition branch from d6aa201 to c8b07ce Compare April 2, 2025 09:28
Base automatically changed from micha/go-to-type-definition to main April 2, 2025 12:12
@MichaReiser MichaReiser force-pushed the micha/playground-goto-type-definition branch 2 times, most recently from 0229ed9 to b9b659c Compare April 2, 2025 12:21
@MichaReiser MichaReiser force-pushed the micha/playground-goto-type-definition branch from b9b659c to 68d506e Compare April 2, 2025 14:25
@MichaReiser MichaReiser merged commit 24498e3 into main Apr 2, 2025
23 checks passed
@MichaReiser MichaReiser deleted the micha/playground-goto-type-definition branch April 2, 2025 14:35
dcreager added a commit that referenced this pull request Apr 3, 2025
* origin/main: (35 commits)
  [red-knot] Callable types are disjoint from literals (#17160)
  [red-knot] Fix inference for `pow` between two literal integers (#17161)
  [red-knot] Add GitHub PR annotations when mdtests fail in CI (#17150)
  [red-knot] Fix equivalence of differently ordered unions that contain `Callable` types (#17145)
  [red-knot] Add initial set of tests for unreachable code (#17159)
  [`airflow`] Move `AIR302` to `AIR301` and `AIR303` to `AIR302` (#17151)
  ruff_db: simplify lifetimes on `DiagnosticDisplay`
  [red-knot] Detect division-by-zero in unions and intersections (#17157)
  [`airflow`] Add autofix infrastructure to `AIR302` name checks (#16965)
  [`flake8-bandit`] Mark `str` and `list[str]` literals as trusted input (`S603`) (#17136)
  [`airflow`] Add autofix for `AIR302` attribute checks (#16977)
  [`airflow`] Extend `AIR302` with additional symbols (#17085)
  [`airflow`] Move `AIR301` to `AIR002` (#16978)
  [`airflow`] Add autofix for `AIR302` method checks (#16976)
  ruff_db: switch diagnostic rendering over to `std::fmt::Display`
  [red-knot] Add 'Goto type definition' to the playground (#17055)
  red_knot_ide: update snapshots
  red_knot_python_semantic: remove comment about `TypeCheckDiagnostic`
  ruff_db: delete most of the old diagnostic code
  red_knot: use `Diagnostic` inside of red knot
  ...
maxmynter pushed a commit to maxmynter/ruff that referenced this pull request Apr 3, 2025
)

## Summary

This PR adds Goto type definition to the playground, using the same
infrastructure as the LSP.


The main *challenge* with implementing this feature was that the editor
can now participate in which tab is open.

## Known limitations

The same as for the LSP. Most notably, navigating to types defined in
typeshed isn't supported.

## Test Plan


https://github.com/user-attachments/assets/22dad7c8-7ac7-463f-b066-5d5b2c45d1fe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

playground A playground-specific issue ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant