Skip to content

Commit

Permalink
Add go-to-definition support to autoloaded constants (#2395)
Browse files Browse the repository at this point in the history
* complete jump-to-the-right-file

* add annotation about autoloaded constants definition in requests/definition.rb

* use find_in_index jump to the correct definition

* add Automated Tests

* completed the modifies suggested by @vinistock

* remove the blank line in lib/ruby_lsp/listeners/definition.rb

* add nil check for �rguments.first in lib/ruby_lsp/listeners/definition.rb#handle_autoload_definition

* finish the modifications from @vinistock in listener/definition.rb

* Trigger autoload definition request on symbol nodes instead of call nodes

* Address review feedback

* Improve tests

---------

Co-authored-by: ShiXiang <21371237@buaa.edu.cn>
Co-authored-by: ShiXiang <15001022180@163.com>
Co-authored-by: Super-Xray <110341228+Super-Xray@users.noreply.github.com>
  • Loading branch information
4 people authored Aug 1, 2024
1 parent 730be2f commit 5d9e12c
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 2 deletions.
23 changes: 23 additions & 0 deletions lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def initialize(response_builder, global_state, language_id, uri, node_context, d
:on_instance_variable_or_write_node_enter,
:on_instance_variable_target_node_enter,
:on_string_node_enter,
:on_symbol_node_enter,
:on_super_node_enter,
:on_forwarding_super_node_enter,
)
Expand Down Expand Up @@ -81,6 +82,17 @@ def on_string_node_enter(node)
handle_require_definition(node, name)
end

sig { params(node: Prism::SymbolNode).void }
def on_symbol_node_enter(node)
enclosing_call = @node_context.call_node
return unless enclosing_call

name = enclosing_call.name
return unless name == :autoload

handle_autoload_definition(enclosing_call)
end

sig { params(node: Prism::BlockArgumentNode).void }
def on_block_argument_node_enter(node)
expression = node.expression
Expand Down Expand Up @@ -251,6 +263,17 @@ def handle_require_definition(node, message)
end
end

sig { params(node: Prism::CallNode).void }
def handle_autoload_definition(node)
argument = node.arguments&.arguments&.first
return unless argument.is_a?(Prism::SymbolNode)

constant_name = argument.value
return unless constant_name

find_in_index(constant_name)
end

sig { params(value: String).void }
def find_in_index(value)
entries = @index.resolve(value, @node_context.nesting)
Expand Down
11 changes: 9 additions & 2 deletions lib/ruby_lsp/requests/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ class Definition < Request
extend T::Sig
extend T::Generic

SPECIAL_METHOD_CALLS = [
:require,
:require_relative,
:autoload,
].freeze

sig do
params(
document: Document,
Expand Down Expand Up @@ -78,8 +84,9 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)
parent,
position,
)
elsif target.is_a?(Prism::CallNode) && target.name != :require && target.name != :require_relative &&
!covers_position?(target.message_loc, position)
elsif target.is_a?(Prism::CallNode) && !SPECIAL_METHOD_CALLS.include?(target.message) && !covers_position?(
target.message_loc, position
)
# If the target is a method call, we need to ensure that the requested position is exactly on top of the
# method identifier. Otherwise, we risk showing definitions for unrelated things
target = nil
Expand Down
56 changes: 56 additions & 0 deletions test/requests/definition_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,62 @@ def bar
end
end

def test_jumping_to_autoload_definition_when_declaration_exists
source = <<~RUBY
# typed: ignore
class Foo
autoload :Bar, "bar"
end
RUBY

with_server(source) do |server, uri|
server.global_state.index.index_single(
RubyIndexer::IndexablePath.new(nil, "/fake/path/bar.rb"), <<~RUBY
class Foo::Bar; end
RUBY
)
server.global_state.index.index_single(
RubyIndexer::IndexablePath.new(nil, "/fake/path/baz.rb"), <<~RUBY
class Foo::Bar; end
RUBY
)
server.process_message(
id: 1,
method: "textDocument/definition",
params: { textDocument: { uri: uri }, position: { character: 12, line: 3 } },
)

response = server.pop_response.response
# This feature simply does a constant lookup on the symbol passed to autoload, instead of jumping into the
# autoloaded path
# This is because there's no guarantee that the autoloaded file actually defines the constant. But if it does,
# then it will be listed in the result anyway
assert_equal(2, response.size)
assert_equal("file:///fake/path/bar.rb", response.first.attributes[:targetUri])
assert_equal("file:///fake/path/baz.rb", response.last.attributes[:targetUri])
end
end

def test_does_nothing_when_autoload_declaration_does_not_exist
source = <<~RUBY
# typed: ignore
class Foo
autoload :Bar, "bar"
end
RUBY

with_server(source) do |server, uri|
server.process_message(
id: 1,
method: "textDocument/definition",
params: { textDocument: { uri: uri }, position: { character: 3, line: 3 } },
)
assert_empty(server.pop_response.response)
end
end

def test_methods_with_dynamic_namespace_is_also_suggested
source = <<~RUBY
# typed: false
Expand Down

0 comments on commit 5d9e12c

Please sign in to comment.