Add hover for global variables (#2691)

* feat: add hover for global variables

* feat: add hover for all global variable nodes

* feat: apply hover target correction on global variable

* test: remove useless test setup

* chore: remove CallNode condition since it is fixed with adding SymbolNode & StringNode to Hover::ALLOWED_TARGET

lib/ruby_lsp/listeners/hover.rb

@@ -13,6 +13,12 @@ class Hover
           Prism::ConstantReadNode,
           Prism::ConstantWriteNode,
           Prism::ConstantPathNode,
+          Prism::GlobalVariableAndWriteNode,
+          Prism::GlobalVariableOperatorWriteNode,
+          Prism::GlobalVariableOrWriteNode,
+          Prism::GlobalVariableReadNode,
+          Prism::GlobalVariableTargetNode,
+          Prism::GlobalVariableWriteNode,
           Prism::InstanceVariableReadNode,
           Prism::InstanceVariableAndWriteNode,
           Prism::InstanceVariableOperatorWriteNode,
@@ -62,6 +68,12 @@ def initialize(response_builder, global_state, uri, node_context, dispatcher, so
           :on_constant_write_node_enter,
           :on_constant_path_node_enter,
           :on_call_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,
@@ -128,6 +140,36 @@ def on_call_node_enter(node)
         handle_method_hover(message)
       end
 
+      sig { params(node: Prism::GlobalVariableAndWriteNode).void }
+      def on_global_variable_and_write_node_enter(node)
+        handle_global_variable_hover(node.name.to_s)
+      end
+
+      sig { params(node: Prism::GlobalVariableOperatorWriteNode).void }
+      def on_global_variable_operator_write_node_enter(node)
+        handle_global_variable_hover(node.name.to_s)
+      end
+
+      sig { params(node: Prism::GlobalVariableOrWriteNode).void }
+      def on_global_variable_or_write_node_enter(node)
+        handle_global_variable_hover(node.name.to_s)
+      end
+
+      sig { params(node: Prism::GlobalVariableReadNode).void }
+      def on_global_variable_read_node_enter(node)
+        handle_global_variable_hover(node.name.to_s)
+      end
+
+      sig { params(node: Prism::GlobalVariableTargetNode).void }
+      def on_global_variable_target_node_enter(node)
+        handle_global_variable_hover(node.name.to_s)
+      end
+
+      sig { params(node: Prism::GlobalVariableWriteNode).void }
+      def on_global_variable_write_node_enter(node)
+        handle_global_variable_hover(node.name.to_s)
+      end
+
       sig { params(node: Prism::InstanceVariableReadNode).void }
       def on_instance_variable_read_node_enter(node)
         handle_instance_variable_hover(node.name.to_s)
@@ -265,6 +307,16 @@ def handle_instance_variable_hover(name)
         # If by any chance we haven't indexed the owner, then there's no way to find the right declaration
       end
 
+      sig { params(name: String).void }
+      def handle_global_variable_hover(name)
+        entries = @index[name]
+        return unless entries
+
+        categorized_markdown_from_index_entries(name, entries).each do |category, content|
+          @response_builder.push(content, category: category)
+        end
+      end
+
       sig { params(name: String, location: Prism::Location).void }
       def generate_hover(name, location)
         entries = @index.resolve(name, @node_context.nesting)

lib/ruby_lsp/requests/hover.rb

@@ -46,17 +46,13 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)
         target = node_context.node
         parent = node_context.parent
 
-        if (Listeners::Hover::ALLOWED_TARGETS.include?(parent.class) &&
-            !Listeners::Hover::ALLOWED_TARGETS.include?(target.class)) ||
-            (parent.is_a?(Prism::ConstantPathNode) && target.is_a?(Prism::ConstantReadNode))
+        if should_refine_target?(parent, target)
           target = determine_target(
             T.must(target),
             T.must(parent),
             position,
           )
-        elsif target.is_a?(Prism::CallNode) && target.name != :require && target.name != :require_relative &&
-            !covers_position?(target.message_loc, position)
-
+        elsif position_outside_target?(position, target)
           target = nil
         end
 
@@ -89,6 +85,28 @@ def perform
           ),
         )
       end
+
+      private
+
+      sig { params(parent: T.nilable(Prism::Node), target: T.nilable(Prism::Node)).returns(T::Boolean) }
+      def should_refine_target?(parent, target)
+        (Listeners::Hover::ALLOWED_TARGETS.include?(parent.class) &&
+        !Listeners::Hover::ALLOWED_TARGETS.include?(target.class)) ||
+          (parent.is_a?(Prism::ConstantPathNode) && target.is_a?(Prism::ConstantReadNode))
+      end
+
+      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
+          !covers_position?(target.name_loc, position)
+        else
+          false
+        end
+      end
     end
   end
 end

test/requests/hover_expectations_test.rb

@@ -60,6 +60,73 @@ def test_hovering_on_erb
     end
   end
 
+  def test_hovering_for_global_variables
+    source = <<~RUBY
+      # and write node
+      $bar &&= 1
+      # operator write node
+      $baz += 1
+      # or write node
+      $qux ||= 1
+      # target write node
+      $quux, $corge = 1
+      # write node
+      $foo = 1
+      # read node
+      $DEBUG
+    RUBY
+
+    expectations = [
+      { line: 1, documentation: "and write node" },
+      { line: 3, documentation: "operator write node" },
+      { line: 5, documentation: "or write node" },
+      { line: 7, documentation: "target write node" },
+      { line: 9, documentation: "write node" },
+      { line: 11, documentation: "The debug flag" },
+    ]
+
+    with_server(source) do |server, uri|
+      index = server.instance_variable_get(:@global_state).index
+      RubyIndexer::RBSIndexer.new(index).index_ruby_core
+
+      expectations.each do |expectation|
+        server.process_message(
+          id: 1,
+          method: "textDocument/hover",
+          params: { textDocument: { uri: uri }, position: { line: expectation[:line], character: 1 } },
+        )
+
+        assert_match(expectation[:documentation], server.pop_response.response.contents.value)
+      end
+    end
+  end
+
+  def test_hover_apply_target_correction
+    source = <<~RUBY
+      $bar &&= 1
+      $baz += 1
+      $qux ||= 1
+      $foo = 1
+    RUBY
+
+    lines_with_target_correction = [1, 2, 3, 4]
+
+    with_server(source) do |server, uri|
+      lines_with_target_correction.each do |line|
+        server.process_message(
+          id: 1,
+          method: "textDocument/hover",
+          params: {
+            textDocument: { uri: uri },
+            position: { line: line, character: 5 },
+          },
+        )
+
+        assert_nil(server.pop_response.response)
+      end
+    end
+  end
+
   def test_hovering_precision
     source = <<~RUBY
       module Foo
@@ -310,13 +377,13 @@ def bar
       end
     RUBY
 
-    # Going to definition on `argument` should not take you to the `foo` method definition
     with_server(source) do |server, uri|
       server.process_message(
         id: 1,
         method: "textDocument/hover",
         params: { textDocument: { uri: uri }, position: { character: 12, line: 5 } },
       )
+      # Hover on `argument` should not show you the `foo` documentation
       assert_nil(server.pop_response.response)
 
       server.process_message(