ruby-lsp icon indicating copy to clipboard operation
ruby-lsp copied to clipboard

Include comments when dispatching listener events

Open vinistock opened this issue 1 year ago • 5 comments

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

vinistock avatar May 16 '24 08:05 vinistock

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').

Super-Xray avatar Jun 15 '24 12:06 Super-Xray

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.

vinistock avatar Jun 17 '24 13:06 vinistock

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.

Super-Xray avatar Jun 18 '24 13:06 Super-Xray

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.

vinistock avatar Jun 18 '24 13:06 vinistock

This would be a really great addition now that inline RBS is being added directly to the RBS gem. Having an RBS type parser to show rich return type info would be really useful, but right now it can't be built as an addon, as far as I can tell.

connorshea avatar Nov 14 '25 22:11 connorshea