Shopify / ruby-lsp

An opinionated language server for Ruby
https://shopify.github.io/ruby-lsp/
MIT License
1.59k stars 157 forks source link

Include comments when dispatching listener events #2054

Open vinistock opened 6 months ago

vinistock commented 6 months ago

There are scenarios where addons might need to have access to comments that are close to nodes they are interested in.

For example, for RBS inline annotations, an addon could provide completion, go to definition, hover and semantic highlighting. But currently, the listeners cannot receive any comments content, because the comments are separate from the AST in Prism's parse result.

I believe the easiest way to provide this functionality to listeners might be to create a custom dispatcher that stores the comment information and passes that along to listeners in a way that allows them to easily access it.

Scenario: imagine proper highlighting and the ability to navigate through constants in these inline RBS annotations.

class Foo
  attr_reader :name # ::String

  # :: (String, Integer?) -> void
  def bar(a, b)
  end
end
Super-Xray commented 5 months ago

image Excuse me, it seems that this function has been implemented, though it can only read the annotation which at specific position('test', but not '::String').

vinistock commented 5 months ago

Sorry, that's unrelated. We decided to only index documentation on top of declaration for performance reasons, but one thing we'd like to explore is to make documentation fetching lazy (which will also save lots of memory in the index).

This issue is related to addon listeners not having any means to access comment contents to provide other features, like allowing you to go to definition on a type defined in a comment.

Super-Xray commented 4 months ago

Thanks for your explanation, so this issue's work is about saving memory by fetching lazy in this hover function, in order to support other functions (in the future) such as go to definition on a type defined in a comment?

I guess the related code is at https://github.com/Shopify/ruby-lsp/blob/main/lib/ruby_lsp/requests/support/common.rb#L92 and https://github.com/Shopify/ruby-lsp/blob/main/lib/ruby_lsp/listeners/hover.rb#L97. It seems like the comment come from @index.resolve_method(message, @node_context.fully_qualified_name). Is it right? I'm interested in this work.

vinistock commented 4 months ago

this issue's work is about saving memory by fetching lazy in this hover function

No. Currently, listener objects that register with Prism::Dispatcher can only handle node events, like on_call_node_enter. However, they have no way of accessing comments, because they aren't nodes and we don't give listeners access to comments in any way.

That prevents addons from being able to add functionality based on comments. In the example of the PR description, a Steep addon for type checking is currently unable to provide go to definition in an inline comment signature, because it has no access to any comments.

The work to make comment fetching lazy is a different effort.