Shopify / ruby-lsp

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

Add go to definition support for autoloaded constants #1893

Open vinistock opened 2 months ago

vinistock commented 2 months ago

Basically, the same feature as go to definition for require, but in this case for autoloads.

module Foo
  autoload :Bar, "bar" # <- jumping to bar should take you to the right file
end

We can probably add the same for the constant too, since we know that it must resolve to Foo::Bar, which makes it easy to search in the index.

Super-Xray commented 2 months ago

Hello @vinistock , I'm interested in this issue, can I handle it?

vinistock commented 2 months ago

Sure! Feel free to put up a PR.

Super-Xray commented 2 months ago

Excuse me, I have finished jump-to-file logic but I notice that I don't have the permission to push my own branch so that I can't put up my PR, @vinistock, could you please give me the permission?

st0012 commented 2 months ago

@Super-Xray Usually, devs contribute to an OSS project by forking it, pushed the new branch to the fork, and then open a PR from the fork against the target repo.

Super-Xray commented 2 months ago

Thank you very much! Since this is my first OSS contribution, I didn't know this before.

Super-Xray commented 2 months ago

Sorry, I havn't fully understand this completely, "We can probably add the same for the constant too, since we know that it must resolve to Foo::Bar, which makes it easy to search in the index.", do you mean from ':Bar' in this autoload statement jump to the its class definition line?

vinistock commented 1 month ago

That's what I originally meant, but thinking more about it, I think we can simplify this. Since the definition listener will only handle method call events (and not symbol or string events), it'll be hard to differentiate between the arguments. And the most specific jump you can make is to the constant, so the file path doesn't provide much value.

We can change the definition listener to do something like this

class Definition
  def on_call_node_enter(node)
    message = node.name

    case message
    when :require, :require_relative
      handle_require_definition(node)
    when :autoload
      handle_autoload_definition(node)
    else
      handle_method_definition(node)
    end
  end

  private

  def handle_autoload_definition(node)
    # Get the name of the constant from the first argument of the call node
    # Invoke find_in_index with the name of the constant
  end
end
Super-Xray commented 1 month ago

haha! That's what I think about too! using find_in_index to jump to the correct line!

Super-Xray commented 1 month ago

However,I have found that the structure of autoload statement is image there aren't any ConstantNode here so I think it's difficult to use find_in_index straightly (becuase constant_name method requires ConstantPathNode/ConstantReadNode/ConstantTargetNode input). In order to find the correct line, I think maybe we can refer to the mechanism of searching class or method in a file (input '@' + class_name/method name) because we can get the class's name from SymbolNode. For example, if we want to jump to the position where RubyLsp::Requests::Request is defined, we first jump to the file(it was been implemented before), and then search for Request by using @Request. Does this idea work?And I want to know where is this "@"-searching mechanism?Thanks for your help!

vinistock commented 1 month ago

The find_in_index method accepts the name of the constant you're looking for as a string and already takes care of the namespacing for you.

For example

module Foo
  autoload :Bar, "some/file"
end

In this case, you need to invoke find_in_index with the name Bar and the existing functionality will automatically discover that the referenced constant is actually Foo::Bar. Basically, what you need to do is extract the name from the autoload arguments and pass that name to find_in_index.

This will already make the LSP jump to the exact location where that constant is defined. You won't need the @ based search (document symbol), to land on the right place in the file.

Super-Xray commented 1 month ago

Thank you very much! I found this way didn't work before probably because the 'ruby-lsp' project is an sorbet project so that autoload :Request, "ruby_lsp/requests/request" can't activate the function(next if @typechecker_enabled && not_in_dependencies?(file_path) in find_in_index). I create an 'foo.rb' and 'bar.rb' and it does work!