diff --git a/lib/ruby_lsp/document.rb b/lib/ruby_lsp/document.rb index 375740547..573175fb1 100644 --- a/lib/ruby_lsp/document.rb +++ b/lib/ruby_lsp/document.rb @@ -128,6 +128,7 @@ def locate(node, char_position, node_types: []) closest = node parent = T.let(nil, T.nilable(Prism::Node)) nesting = T.let([], T::Array[T.any(Prism::ClassNode, Prism::ModuleNode)]) + call_node = T.let(nil, T.nilable(Prism::CallNode)) until queue.empty? candidate = queue.shift @@ -159,6 +160,15 @@ def locate(node, char_position, node_types: []) nesting << candidate end + if candidate.is_a?(Prism::CallNode) + arg_loc = candidate.arguments&.location + blk_loc = candidate.block&.location + if (arg_loc && (arg_loc.start_offset...arg_loc.end_offset).cover?(char_position)) || + (blk_loc && (blk_loc.start_offset...blk_loc.end_offset).cover?(char_position)) + call_node = candidate + end + end + # If there are node types to filter by, and the current node is not one of those types, then skip it next if node_types.any? && node_types.none? { |type| candidate.class == type } @@ -183,7 +193,7 @@ def locate(node, char_position, node_types: []) nesting.pop if last_level && last_level.constant_path == closest end - NodeContext.new(closest, parent, nesting.map { |n| n.constant_path.location.slice }) + NodeContext.new(closest, parent, nesting.map { |n| n.constant_path.location.slice }, call_node) end sig { returns(T::Boolean) } diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index ff54c9082..8dbad2ec7 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -39,18 +39,27 @@ def initialize(response_builder, global_state, uri, node_context, dispatcher, ty :on_instance_variable_operator_write_node_enter, :on_instance_variable_or_write_node_enter, :on_instance_variable_target_node_enter, + :on_string_node_enter, ) end sig { params(node: Prism::CallNode).void } def on_call_node_enter(node) - message = node.name + message = node.message + return unless message - if message == :require || message == :require_relative - handle_require_definition(node) - else - handle_method_definition(message.to_s, self_receiver?(node)) - end + handle_method_definition(message, self_receiver?(node)) + end + + sig { params(node: Prism::StringNode).void } + def on_string_node_enter(node) + enclosing_call = @node_context.call_node + return unless enclosing_call + + name = enclosing_call.name + return unless name == :require || name == :require_relative + + handle_require_definition(node, name) end sig { params(node: Prism::BlockArgumentNode).void } @@ -159,19 +168,12 @@ def handle_method_definition(message, self_receiver) end end - sig { params(node: Prism::CallNode).void } - def handle_require_definition(node) - message = node.name - arguments = node.arguments - return unless arguments - - argument = arguments.arguments.first - return unless argument.is_a?(Prism::StringNode) - + sig { params(node: Prism::StringNode, message: Symbol).void } + def handle_require_definition(node, message) case message when :require - entry = @index.search_require_paths(argument.content).find do |indexable_path| - indexable_path.require_path == argument.content + entry = @index.search_require_paths(node.content).find do |indexable_path| + indexable_path.require_path == node.content end if entry @@ -186,7 +188,7 @@ def handle_require_definition(node) ) end when :require_relative - required_file = "#{argument.content}.rb" + required_file = "#{node.content}.rb" path = @uri.to_standardized_path current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : Dir.pwd candidate = File.expand_path(File.join(current_folder, required_file)) diff --git a/lib/ruby_lsp/listeners/hover.rb b/lib/ruby_lsp/listeners/hover.rb index 2a9f3cae9..7bd5ded9a 100644 --- a/lib/ruby_lsp/listeners/hover.rb +++ b/lib/ruby_lsp/listeners/hover.rb @@ -19,6 +19,8 @@ class Hover Prism::InstanceVariableOrWriteNode, Prism::InstanceVariableTargetNode, Prism::InstanceVariableWriteNode, + Prism::SymbolNode, + Prism::StringNode, ], T::Array[T.class_of(Prism::Node)], ) diff --git a/lib/ruby_lsp/node_context.rb b/lib/ruby_lsp/node_context.rb index b0bb1ee5c..a67041874 100644 --- a/lib/ruby_lsp/node_context.rb +++ b/lib/ruby_lsp/node_context.rb @@ -2,8 +2,8 @@ # frozen_string_literal: true module RubyLsp - # This class allows listeners to access contextual information about a node in the AST, such as its parent - # and its namespace nesting. + # This class allows listeners to access contextual information about a node in the AST, such as its parent, + # its namespace nesting, and the surrounding CallNode (e.g. a method call). class NodeContext extend T::Sig @@ -13,11 +13,22 @@ class NodeContext sig { returns(T::Array[String]) } attr_reader :nesting - sig { params(node: T.nilable(Prism::Node), parent: T.nilable(Prism::Node), nesting: T::Array[String]).void } - def initialize(node, parent, nesting) + sig { returns(T.nilable(Prism::CallNode)) } + attr_reader :call_node + + sig do + params( + node: T.nilable(Prism::Node), + parent: T.nilable(Prism::Node), + nesting: T::Array[String], + call_node: T.nilable(Prism::CallNode), + ).void + end + def initialize(node, parent, nesting, call_node) @node = node @parent = parent @nesting = nesting + @call_node = call_node end sig { returns(String) } diff --git a/lib/ruby_lsp/requests/definition.rb b/lib/ruby_lsp/requests/definition.rb index b917fbbb4..f30bd3bb7 100644 --- a/lib/ruby_lsp/requests/definition.rb +++ b/lib/ruby_lsp/requests/definition.rb @@ -60,6 +60,8 @@ def initialize(document, global_state, position, dispatcher, typechecker_enabled Prism::InstanceVariableOrWriteNode, Prism::InstanceVariableTargetNode, Prism::InstanceVariableWriteNode, + Prism::SymbolNode, + Prism::StringNode, ], ) @@ -79,6 +81,9 @@ def initialize(document, global_state, position, dispatcher, typechecker_enabled # 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 + # For methods with block arguments using symbol-to-proc + elsif target.is_a?(Prism::SymbolNode) && parent.is_a?(Prism::BlockArgumentNode) + target = parent end if target diff --git a/test/ruby_document_test.rb b/test/ruby_document_test.rb index cadd4d41f..82736382a 100644 --- a/test/ruby_document_test.rb +++ b/test/ruby_document_test.rb @@ -480,6 +480,67 @@ def baz assert_equal(["Foo", "Other"], node_context.nesting) end + def test_locate_returns_call_node + document = RubyLsp::RubyDocument.new(source: <<~RUBY, version: 1, uri: URI("file:///foo/bar.rb")) + module Foo + class Other + def do_it + hello :foo + :bar + end + end + end + RUBY + + node_context = document.locate_node({ line: 3, character: 14 }) + assert_equal(":foo", T.must(node_context.node).slice) + assert_equal(:hello, T.must(node_context.call_node).name) + + node_context = document.locate_node({ line: 4, character: 8 }) + assert_equal(":bar", T.must(node_context.node).slice) + assert_nil(node_context.call_node) + end + + def test_locate_returns_call_node_nested + document = RubyLsp::RubyDocument.new(source: <<~RUBY, version: 1, uri: URI("file:///foo/bar.rb")) + module Foo + class Other + def do_it + goodbye(hello(:foo)) + end + end + end + RUBY + + node_context = document.locate_node({ line: 3, character: 22 }) + assert_equal(":foo", T.must(node_context.node).slice) + assert_equal(:hello, T.must(node_context.call_node).name) + end + + def test_locate_returns_call_node_for_blocks + document = RubyLsp::RubyDocument.new(source: <<~RUBY, version: 1, uri: URI("file:///foo/bar.rb")) + foo do + "hello" + end + RUBY + + node_context = document.locate_node({ line: 1, character: 4 }) + assert_equal(:foo, T.must(node_context.call_node).name) + end + + def test_locate_returns_call_node_ZZZ + document = RubyLsp::RubyDocument.new(source: <<~RUBY, version: 1, uri: URI("file:///foo/bar.rb")) + foo( + if bar(1, 2, 3) + "hello" # this is the target + end + end + RUBY + + node_context = document.locate_node({ line: 2, character: 6 }) + assert_equal(:foo, T.must(node_context.call_node).name) + end + def test_locate_returns_correct_nesting_when_specifying_target_classes document = RubyLsp::RubyDocument.new(source: <<~RUBY, version: 1, uri: URI("file:///foo/bar.rb")) module Foo