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

Properly handle methods for definition, completion and hover #899

Open
3 tasks
vinistock opened this issue Aug 17, 2023 · 7 comments
Open
3 tasks

Properly handle methods for definition, completion and hover #899

vinistock opened this issue Aug 17, 2023 · 7 comments
Labels
enhancement New feature or request pinned This issue or pull request is pinned and won't be marked as stale

Comments

@vinistock
Copy link
Member

vinistock commented Aug 17, 2023

Depends on #898 and #1333

Use the index to provide go to definition for methods.

Might depend on exploring using a control flow graph (CFG) to implement reaching definitions. While we do not provide typechecking, using reaching definitions might allow us to be more precise.

For example

a = "something"
# We know that `a` is a string because it's a literal, so we know 100% sure this has to be
# String#upcase. If we have an implementation of reaching definitions, we can make this work
a.upcase

Checklist

  • Attributes. The LSP protocol has special types for them, so we should create a separate index entry
  • Taking new invocations to initialize. For completion, it's actually the opposite, we need to use the signature for initialize, but complete to new
  • Method aliases through alias_method and alias
@github-actions
Copy link
Contributor

This issue is being marked as stale because there was no activity in the last 2 months

@github-actions github-actions bot added the Stale label Oct 17, 2023
@Kyrluckechuck
Copy link

Just bumping this since it's not stale and I don't want it marked as closed without being formally acknowledged! 😅

@vinistock vinistock added pinned This issue or pull request is pinned and won't be marked as stale and removed Stale labels Oct 17, 2023
@andyw8
Copy link
Contributor

andyw8 commented Oct 17, 2023

It's not usable yet, but a first step towards this was merged today: #1043

@Drowze
Copy link

Drowze commented Mar 13, 2024

Method completion and jump-to-method-definition is the only thing currently holding me to other alternatives. I am anxiously looking towards this feature to jump on ruby-lsp boat! 🛥️ 🛥️

Thank you very much for your hard work on this project!

@andyw8
Copy link
Contributor

andyw8 commented May 14, 2024

It could be helpful to add a checklist to the PR description here indicating what's remaining.

@rosslyn-beckynorville
Copy link

Method completion and jump-to-method-definition is the only thing currently holding me to other alternatives. I am anxiously looking towards this feature to jump on ruby-lsp boat! 🛥️ 🛥️

Thank you very much for your hard work on this project!

I second this, I am using RubyMine but I'd like the other team members to be able to use VS Code which is free. This is the only thing holding us back! Good luck, seems like a very tough task.

@vinistock
Copy link
Member Author

@rosslyn-beckynorville I encourage you (and others!) to give it a try right now. Here's a little progress report so that people can understand the current state.

Note that all features are supported both for project declarations and gems, so you can use these features while navigating inside a gem's source code as well.

In addition to that, note that meta-programming constructs that cannot be analyzed statically are impossible to support for any of the features, so those are excluded. For example

some_var_that_depends_on_user_input.const_get(:Whatever).send(other_var, ...)

Go to definition support

  1. Constants: fully supported
  2. Instance variables: fully supported
  3. Class instance variables: almost fully supported, we need to merge Linearize singleton ancestors #2214 to properly analyze singleton inheritance
  4. Methods: methods are the most difficult to support because they require much more complex type flow analysis than other Ruby constructs. Let's break down how it currently works:
    • Methods invoked directly on self: fully supported (also excludes meta-programming, like invoking a method with instance_exec thus mutating the meaning of self)
    • Singleton methods invoked directly on constants (e.g.: Foo.something): will be fully supported after Linearize singleton ancestors #2214 is merged
    • Other cases are currently falling back to name matching. The plan is to make the analysis smarter with time and include the method arity in the fallback. For example, when invoking methods on a literal (like strings, integers, arrays or hashes), we can provide accurate go to definition. This work is currently ongoing
  5. You can also jump to definition on require paths

Hover support

Exactly the same state as go to definition, but without the method fallback matching. The reason is because if too many methods match the same name, we would end up showing way too much content on hover and it doesn't seem useful.

Completion support

Same state as go to definition, except for methods and locals. Completion for locals will be added in #2248.

For methods, we can only show completion where types are known. So it works for when the receiver is self or when invoking singleton methods on a constant (e.g.: Foo.).

With better type flow analysis, we will be able to provide completion on cases where the types can be known even without a type system and annotations. However, it will simply not be possible to provide completion in cases where the receiver type is unknown, because no fallback would actually prove useful. For example:

a = foo
# If we don't know what type `foo` returns, what would we show as completion below?
# Showing all possible methods would be confusing and frankly useless
a.

Closing

We are also exploring what we're calling guessed types. It's a super simplistic way of trying to determine the type of a variable or method return based on its name. For example, if a variable is named @user, then we would assume that it is a User type.

That's not always correct and identifiers aren't an adequate container for complex type annotations. You wouldn't be able to express nilability, type unions, intersections, generics and all of the things you need for a proper type system. However, we want to test out the hypothesis of whether it would be useful or not, since it allows us to provide precise completion and go to definition where the identifier and type names match exactly.

Thank you for the words of encouragement and if you have feedback about guessed types, please let us know!!!

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

No branches or pull requests

5 participants