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.

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

Explore adding an indexer to provide go to definition #199

Closed
vinistock opened this issue Jul 15, 2022 · 20 comments · Fixed by #878
Closed

Explore adding an indexer to provide go to definition #199

vinistock opened this issue Jul 15, 2022 · 20 comments · Fixed by #878
Assignees
Labels
enhancement New feature or request pinned This issue or pull request is pinned and won't be marked as stale
Milestone

Comments

@vinistock
Copy link
Member

Despite not being able to provide a fully correct go to definition implementation (which requires type checking), the Ruby LSP could provide a simpler version of it.

One possible idea is to index all existing classes and methods in the code base and then providing go to definition results for methods that match the same name. Despite being naive, it's better than no support at all.

Ideas for exploration

  • The indexer can run in a separate process (maybe a Ractor?) to not block the LSP start up
  • The indexer can cache indices from files that haven't been modified and re-use those to speed up the initial start up
  • The indexer can try to be a bit smart about argument count. In this case, it would find method definitions by searching not only the name, but argument count. Need to be careful about optionals and splats

Note: the Ruby LSP should not provide any go to definition at all if Sorbet is present, since Sorbet does proper type checking and returns correct go to definition results.

@Morriar
Copy link
Contributor

Morriar commented Jul 15, 2022

in the code base

And the gems I guess?

@vinistock
Copy link
Member Author

vinistock commented Jul 15, 2022

And the gems I guess?

Yes, those too.

@vinistock
Copy link
Member Author

The LSIF spec may be useful for what we're trying to do.

@matteeyah
Copy link

matteeyah commented Mar 7, 2023

If ruby-lsp could generate lsif files, it would unblock "smart" code navigation in other online code navigation solutions. Sourcegraph uses this (sourcegraph/sourcegraph#10385), so does GitLab, and I think others do as well.

@pheen
Copy link

pheen commented Mar 31, 2023

👋 I wanted to drop-in and say Hi! I have tried to implement a solution for Go To Definition both in Ruby which I couldn't get it to be reliable, so I moved to a Ruby parser written in Rust that I implemented an LSP on-top of here: https://github.com/pheen/fuzzy_ruby_vscode_client

I mentioned it because it's an example of what Go To Definition can look like using only heuristics, without any type annotations. It works reasonably well, but could be much better if more was added like lookbehind, argument counts, argument keyword names, etc. Unfortunately, I have more pressing side-projects and likely won't have time to work on this anymore.

Regarding LSIF, I looked at it and my interpretation is it will require two index passes. First, one to index all of the various tokens, then a second pass to populate all of the connections. That's not a problem if you're exporting that data from a CI build or something, but if you need to build an index quickly when a developer opens up their editor then that sounds rough. Very curious to know if that's wrong.

I'm looking forward to tracking this work and how it develops!

@matteeyah
Copy link

@pheen I imagine the lsif index would be checked into the repository.

Instead of creating it dynamically in a background job it can be generated explicitly by the developer when changes update the class hierarchy or method signatures.

I think of it as a "lock file for code structure".

@matteeyah
Copy link

matteeyah commented Apr 1, 2023

The reason why I think lsif is the best approach is simply because a lot of other tools use it and build their functionality on top of it. Other approaches might be more efficient or easier to implement, but using lsif has the potential to unlock a bunch of tools that are inaccessible to Ruby codebases right now.

Ruby Signatures / RBS is a strong alternative but the adoption rate is simply just too low right now. I imagine as RBS adoption grows, lots of tools that offer intelligence for ruby will adopt RBS as well. But that's all in the future.

Sorbet has type annotations and is able to generate a representation of the codebase AST. It might be easier for Sorbet to generate an index than building an indexer here from scratch 🤔 I opened an issue to explore their perspective on this - sorbet/sorbet#6889

@jmgarnier
Copy link

👋 I wanted to drop-in and say Hi! I have tried to implement a solution for Go To Definition both in Ruby which I couldn't get it to be reliable, so I moved to a Ruby parser written in Rust that I implemented an LSP on-top of here: https://github.com/pheen/fuzzy_ruby_vscode_client

I mentioned it because it's an example of what Go To Definition can look like using only heuristics, without any type annotations. It works reasonably well, but could be much better if more was added like lookbehind, argument counts, argument keyword names, etc. Unfortunately, I have more pressing side-projects and likely won't have time to work on this anymore.

For those who want to try pheen "Go to Definition", I can confirm it works well and fast for the methods defined in the project. Unfortunately, it only parses gems used by the project whose name contains a _. It does not work with gems whose name contains a -

@vinistock vinistock added this to the 2023-Q3 milestone Jul 10, 2023
@vinistock vinistock added enhancement New feature or request pinned This issue or pull request is pinned and won't be marked as stale labels Jul 20, 2023
@vinistock vinistock self-assigned this Aug 10, 2023
@waynehoover
Copy link

I see this was closed, but are there plans to index methods?

@vinistock
Copy link
Member Author

Yes. We will start with class and modules and then work on indexing methods, which are more complicated.

@davidrunger
Copy link

@vinistock : Is there a tracking issue or PR that we can subscribe to, in order to stay updated on the progress toward indexing methods / providing "Go to definition" functionality for methods?

@vinistock
Copy link
Member Author

I created #899 for the sake of having an issue to subscribe to, but please notice that it requires a series of steps to provide proper functionality for methods (which is why we began with classes/modules).

We need to

  • Keep track of the class/module hierarchy (since you might be trying to go to the definition of a method defined in a superclass or in an included module)
  • Add a representation of singleton classes in the index
  • Start keeping track of methods in the index
  • Explore using a control flow graph with reaching definitions so that we can provide precise go to definition when we know for a fact the right type of a variable
  • Finally, implement go to definition for methods

@rbarreca
Copy link

Maybe a naive question, but why not collaborate/merge with @pheen's fuzzy_ruby_server project?

@vinistock
Copy link
Member Author

We're always happy to collaborate. Can you suggest in what specific way you see a path for it?

The one blocker I see is that the project is written in Rust. We've been doing our best to avoid making the Ruby LSP a native extension - trying to achieve the performance we need using Ruby only and relying on improvements to YJIT and the new parser (YARP) to do so.

Using Ruby also allows us to

  • Make key building blocks for static analysis reusable. For example, we would like to extract the indexing mechanism into a gem after we figure out the API for it and fix bugs so that other gems can easily make use of some fundamental static analysis tools
  • Allow other gems to enhance the Ruby LSP's functionality through server extensions
  • Iterate quickly

@rbarreca
Copy link

I am a total n00b in this area so it was a truly naive question and I don’t have any ideas. Your answer is really helpful in understanding the reasons why! Thanks for responding.

@ashok-stack
Copy link

just a noob trying to understand, how sublime text is able to provide go to definition out of the box without any library

@vinistock
Copy link
Member Author

I'm not a Sublime Text user and I don't know for sure how it works. If it has support for go to definition without installing any plugins, then it must have some code indexing implementation built-in.

It's surprising to me that they would have that built-in for a specific language. I haven't heard of a text editor that has complex language support built-in other than for IDEs that are made specifically for a programming language.

@Morriar
Copy link
Contributor

Morriar commented Jan 5, 2024

Sublime Text does indexing by parsing the files of the project: https://lsp.sublimetext.io/features/#goto-definition.

Sublime Text provides a "Goto Definition" feature by indexing the files in your project

The parsing seems to be regex-based and defined through Syntax Definitions files (the same files used for syntax highlighting): https://www.sublimetext.com/docs/syntax.html.

@sandstrom
Copy link

Is there any issue tracking the improvements mentioned in #199 (comment) above? (making it work for methods included from a module on a parent class, for example)

@vinistock
Copy link
Member Author

It's the issue linked in that comment and its dependencies #898 and #1333.

andyw8 pushed a commit to andyw8/ruby-lsp that referenced this issue Mar 2, 2024
…ypescript-eslint/parser-5.34.0

Bump @typescript-eslint/parser from 5.33.1 to 5.34.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned This issue or pull request is pinned and won't be marked as stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants