diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index 8f4e28d49..6413cdabe 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -39,7 +39,12 @@ def initialize(response_builder, global_state, language_id, uri, node_context, d :on_block_argument_node_enter, :on_constant_read_node_enter, :on_constant_path_node_enter, + :on_global_variable_and_write_node_enter, + :on_global_variable_operator_write_node_enter, + :on_global_variable_or_write_node_enter, :on_global_variable_read_node_enter, + :on_global_variable_target_node_enter, + :on_global_variable_write_node_enter, :on_instance_variable_read_node_enter, :on_instance_variable_write_node_enter, :on_instance_variable_and_write_node_enter, @@ -121,23 +126,34 @@ def on_constant_read_node_enter(node) find_in_index(name) end + sig { params(node: Prism::GlobalVariableAndWriteNode).void } + def on_global_variable_and_write_node_enter(node) + handle_global_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::GlobalVariableOperatorWriteNode).void } + def on_global_variable_operator_write_node_enter(node) + handle_global_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::GlobalVariableOrWriteNode).void } + def on_global_variable_or_write_node_enter(node) + handle_global_variable_definition(node.name.to_s) + end + sig { params(node: Prism::GlobalVariableReadNode).void } def on_global_variable_read_node_enter(node) - entries = @index[node.name.to_s] - - return unless entries + handle_global_variable_definition(node.name.to_s) + end - entries.each do |entry| - location = entry.location + sig { params(node: Prism::GlobalVariableTargetNode).void } + def on_global_variable_target_node_enter(node) + handle_global_variable_definition(node.name.to_s) + end - @response_builder << Interface::Location.new( - uri: URI::Generic.from_path(path: entry.file_path).to_s, - range: Interface::Range.new( - start: Interface::Position.new(line: location.start_line - 1, character: location.start_column), - end: Interface::Position.new(line: location.end_line - 1, character: location.end_column), - ), - ) - end + sig { params(node: Prism::GlobalVariableWriteNode).void } + def on_global_variable_write_node_enter(node) + handle_global_variable_definition(node.name.to_s) end sig { params(node: Prism::InstanceVariableReadNode).void } @@ -197,6 +213,25 @@ def handle_super_node_definition ) end + sig { params(name: String).void } + def handle_global_variable_definition(name) + entries = @index[name] + + return unless entries + + entries.each do |entry| + location = entry.location + + @response_builder << Interface::Location.new( + uri: URI::Generic.from_path(path: entry.file_path).to_s, + range: Interface::Range.new( + start: Interface::Position.new(line: location.start_line - 1, character: location.start_column), + end: Interface::Position.new(line: location.end_line - 1, character: location.end_column), + ), + ) + end + end + sig { params(name: String).void } def handle_instance_variable_definition(name) # Sorbet enforces that all instance variables be declared on typed strict or higher, which means it will be able diff --git a/lib/ruby_lsp/requests/definition.rb b/lib/ruby_lsp/requests/definition.rb index 59f3592c3..8305f0ec6 100644 --- a/lib/ruby_lsp/requests/definition.rb +++ b/lib/ruby_lsp/requests/definition.rb @@ -12,12 +12,6 @@ class Definition < Request extend T::Sig extend T::Generic - SPECIAL_METHOD_CALLS = [ - :require, - :require_relative, - :autoload, - ].freeze - sig do params( document: T.any(RubyDocument, ERBDocument), @@ -46,7 +40,12 @@ def initialize(document, global_state, position, dispatcher, sorbet_level) Prism::ConstantReadNode, Prism::ConstantPathNode, Prism::BlockArgumentNode, + Prism::GlobalVariableAndWriteNode, + Prism::GlobalVariableOperatorWriteNode, + Prism::GlobalVariableOrWriteNode, Prism::GlobalVariableReadNode, + Prism::GlobalVariableTargetNode, + Prism::GlobalVariableWriteNode, Prism::InstanceVariableReadNode, Prism::InstanceVariableAndWriteNode, Prism::InstanceVariableOperatorWriteNode, @@ -72,11 +71,7 @@ def initialize(document, global_state, position, dispatcher, sorbet_level) parent, 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 + elsif position_outside_target?(position, target) target = nil # For methods with block arguments using symbol-to-proc elsif target.is_a?(Prism::SymbolNode) && parent.is_a?(Prism::BlockArgumentNode) @@ -107,6 +102,26 @@ def perform @dispatcher.dispatch_once(@target) if @target @response_builder.response end + + private + + sig { params(position: T::Hash[Symbol, T.untyped], target: T.nilable(Prism::Node)).returns(T::Boolean) } + def position_outside_target?(position, target) + case target + when Prism::GlobalVariableAndWriteNode, + Prism::GlobalVariableOperatorWriteNode, + Prism::GlobalVariableOrWriteNode, + Prism::GlobalVariableWriteNode, + Prism::InstanceVariableAndWriteNode, + Prism::InstanceVariableOperatorWriteNode, + Prism::InstanceVariableOrWriteNode, + Prism::InstanceVariableWriteNode + + !covers_position?(target.name_loc, position) + else + false + end + end end end end diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index 17c78f3ab..b74090c7b 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -673,6 +673,11 @@ def baz def test_definition_for_global_variables source = <<~RUBY + $bar &&= 1 + $bar += 1 + $foo ||= 1 + $bar, $foo = 1 + $foo = 1 $DEBUG RUBY @@ -685,6 +690,31 @@ def test_definition_for_global_variables method: "textDocument/definition", params: { textDocument: { uri: uri }, position: { character: 1, line: 0 } }, ) + + response = server.pop_response.response + assert_equal(3, response.size) + assert_equal(0, response[0].range.start.line) + assert_equal(1, response[1].range.start.line) + assert_equal(3, response[2].range.start.line) + + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: { character: 1, line: 2 } }, + ) + + response = server.pop_response.response + assert_equal(3, response.size) + assert_equal(2, response[0].range.start.line) + assert_equal(3, response[1].range.start.line) + assert_equal(4, response[2].range.start.line) + + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: { character: 1, line: 5 } }, + ) + response = server.pop_response.response.first assert_match(%r{/gems/rbs-.*/core/global_variables.rbs}, response.uri) assert_equal(response.range.start.line, response.range.end.line) @@ -692,6 +722,35 @@ def test_definition_for_global_variables end end + def test_definition_apply_target_correction + source = <<~RUBY + $foo &&= 1 + $foo += 1 + $foo ||= 1 + $foo = 1 + class Foo + @foo &&= 1 + @foo += 1 + @foo ||= 1 + @foo = 1 + end + RUBY + + lines_with_target_correction = [0, 1, 2, 3, 5, 6, 7, 8] + + with_server(source) do |server, uri| + lines_with_target_correction.each do |line| + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: { character: 7, line: line } }, + ) + + assert_empty(server.pop_response.response) + end + end + end + def test_definition_for_instance_variables source = <<~RUBY class Foo diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index b42224a80..6b08c8d24 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -109,7 +109,7 @@ def test_hover_apply_target_correction $foo = 1 RUBY - lines_with_target_correction = [1, 2, 3, 4] + lines_with_target_correction = [0, 1, 2, 3] with_server(source) do |server, uri| lines_with_target_correction.each do |line|