Skip to content

Add go to definition for classes and modules - #878

Merged
vinistock merged 1 commit into
mainfrom
vs/add_go_to_definition
Aug 16, 2023
Merged

Add go to definition for classes and modules#878
vinistock merged 1 commit into
mainfrom
vs/add_go_to_definition

Conversation

@vinistock

Copy link
Copy Markdown
Member

Motivation

Closes#199

Closing the related issue since the exploration is complete. This PR adds go to definition for classes and modules, which is currently what's covered in the index.

Implementation

The idea is to find constant/constant path nodes and look for them inside the index and then allow jumping to those.

If the project uses Sorbet, we avoid returning locations for classes and modules that are defined inside the project - since Sorbet can find those even for typed: false files. But we still return all results for definitions from gems, which should provide a nice merged experience.

Automated Tests

Added a test for this.

Manual Tests

  1. Boot the LSP on this branch
  2. Try to go to the definition of a class defined in the Ruby LSP itself (e.g.: EventEmitter)
  3. Verify you jump directly to it, without having multiple options (this is coming from Sorbet only)
  4. Try to go to definition of a class defined in a gem (e.g.: SyntaxTree::Const)
  5. Verify you get both RBIs and the actual gem source as results

@vinistockvinistock added the enhancement New feature or request label Aug 10, 2023
@vinistockvinistock added this to the 2023-Q3 milestone Aug 10, 2023
@vinistockvinistock self-assigned this Aug 10, 2023
@vinistock
vinistock requested a review from a team as a code ownerAugust 10, 2023 16:30
Comment threadlib/ruby_lsp/requests/definition.rb
@github-actions

github-actionsBot commented Aug 10, 2023

Copy link
Copy Markdown
Contributor
Benchmark results in seconds (slowest at top)
textDocument/completion average: 0.305521 std_dev: 0.006352
textDocument/diagnostic average: 0.048978 std_dev: 0.011798
textDocument/definition average: 0.006183 std_dev: 0.003456
textDocument/selectionRange average: 0.00484 std_dev: 0.00061
textDocument/documentHighlight average: 0.002988 std_dev: 0.000323
textDocument/codeLens average: 0.002953 std_dev: 0.000268
textDocument/documentLink average: 0.002935 std_dev: 0.000201
textDocument/semanticTokens/full average: 0.002932 std_dev: 0.000303
textDocument/foldingRange average: 0.002906 std_dev: 0.0002
textDocument/documentSymbol average: 0.002883 std_dev: 0.000134
textDocument/semanticTokens/range average: 0.001933 std_dev: 0.000165
codeAction/resolve average: 0.001714 std_dev: 0.000167
textDocument/hover average: 0.001655 std_dev: 0.000109
textDocument/inlayHint average: 0.001605 std_dev: 0.000155
textDocument/formatting average: 0.000992 std_dev: 0.000231
textDocument/onTypeFormatting average: 0.000984 std_dev: 0.000118
textDocument/codeAction average: 0.000963 std_dev: 0.000109
================================================================================
Comparison with main branch:
textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range unchanged
textDocument/documentSymbol unchanged
textDocument/foldingRange unchanged
textDocument/formatting unchanged
textDocument/diagnostic unchanged
textDocument/documentLink unchanged
textDocument/inlayHint unchanged
textDocument/selectionRange unchanged
textDocument/documentHighlight unchanged
textDocument/hover unchanged
textDocument/codeAction unchanged
textDocument/onTypeFormatting unchanged
codeAction/resolve unchanged
textDocument/completion unchanged
textDocument/codeLens unchanged
textDocument/definition unchanged
================================================================================
Missing benchmarks:
RubyLsp::Requests::ShowSyntaxTree

Comment threadlib/ruby_lsp/requests/definition.rb Outdated
Comment threadlib/ruby_lsp/requests/definition.rb Outdated
@vinistock
vinistockforce-pushed the vs/add_go_to_definition branch from 2bb1791 to 7c7212aCompareAugust 10, 2023 17:47
@vinistock
vinistockforce-pushed the vs/index_files branch 3 times, most recently from d6559e2 to fc5ec69CompareAugust 10, 2023 19:38
Comment thread.irb_history Outdated
@vinistock
vinistockforce-pushed the vs/add_go_to_definition branch from 7c7212a to 742ee6cCompareAugust 11, 2023 14:45
Comment threadlib/ruby_lsp/executor.rb
@vinistock
vinistockforce-pushed the vs/index_files branch 2 times, most recently from 34cb9ed to c5a146fCompareAugust 11, 2023 20:04
@vinistock
vinistockforce-pushed the vs/index_files branch 3 times, most recently from 32c2048 to 419e101CompareAugust 16, 2023 17:24
Base automatically changed from vs/index_files to mainAugust 16, 2023 17:57
@vinistock
vinistockforce-pushed the vs/add_go_to_definition branch from 742ee6c to f78affbCompareAugust 16, 2023 18:03
@vinistock
vinistock enabled auto-merge (squash) August 16, 2023 18:04
@vinistock
vinistock merged commit bb05387 into mainAug 16, 2023
@vinistock
vinistock deleted the vs/add_go_to_definition branch August 16, 2023 18:11
vinistock pushed a commit that referenced this pull request Feb 28, 2024
…d-patch-cb33631c2e
Bump the minor-and-patch group with 2 updates
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explore adding an indexer to provide go to definition

3 participants

@vinistock@paracycle@Morriar