diff --git a/ADDONS.md b/ADDONS.md index e4deec8d8..83c53659d 100644 --- a/ADDONS.md +++ b/ADDONS.md @@ -129,6 +129,7 @@ module RubyLsp end # All listeners have to inherit from ::RubyLsp::Listener + # NOTE: Listeners are currently being refactored, but this implementation embodies the current approach for most listeners. class Hover < ::RubyLsp::Listener extend T::Sig extend T::Generic diff --git a/lib/ruby_lsp/addon.rb b/lib/ruby_lsp/addon.rb index 2f009807b..14e3aa538 100644 --- a/lib/ruby_lsp/addon.rb +++ b/lib/ruby_lsp/addon.rb @@ -1,6 +1,8 @@ # typed: strict # frozen_string_literal: true +require_relative "response_builder" + module RubyLsp # To register an addon, inherit from this class and implement both `name` and `activate` # @@ -118,12 +120,13 @@ def create_code_lens_listener(uri, dispatcher); end # Creates a new Hover listener. This method is invoked on every Hover request sig do overridable.params( + response_builder: RubyLsp::HoverResponseBuilder, nesting: T::Array[String], index: RubyIndexer::Index, dispatcher: Prism::Dispatcher, ).returns(T.nilable(Listener[T.nilable(Interface::Hover)])) end - def create_hover_listener(nesting, index, dispatcher); end + def create_hover_listener(response_builder, nesting, index, dispatcher); end # Creates a new DocumentSymbol listener. This method is invoked on every DocumentSymbol request sig do diff --git a/lib/ruby_lsp/listeners/hover.rb b/lib/ruby_lsp/listeners/hover.rb index 8525d5ed7..8424a721b 100644 --- a/lib/ruby_lsp/listeners/hover.rb +++ b/lib/ruby_lsp/listeners/hover.rb @@ -1,13 +1,13 @@ # typed: strict # frozen_string_literal: true +require "ruby_lsp/response_builder" + module RubyLsp module Listeners - class Hover < Listener + class Hover extend T::Sig - extend T::Generic - - ResponseType = type_member { { fixed: T.nilable(Interface::Hover) } } + include Requests::Support::Common ALLOWED_TARGETS = T.let( [ @@ -19,11 +19,9 @@ class Hover < Listener T::Array[T.class_of(Prism::Node)], ) - sig { override.returns(ResponseType) } - attr_reader :_response - sig do params( + response_builder: RubyLsp::HoverResponseBuilder, uri: URI::Generic, nesting: T::Array[String], index: RubyIndexer::Index, @@ -31,14 +29,13 @@ class Hover < Listener typechecker_enabled: T::Boolean, ).void end - def initialize(uri, nesting, index, dispatcher, typechecker_enabled) + def initialize(response_builder, uri, nesting, index, dispatcher, typechecker_enabled) # rubocop:disable Metrics/ParameterLists + @response_builder = response_builder @path = T.let(uri.to_standardized_path, T.nilable(String)) @nesting = nesting @index = index @typechecker_enabled = typechecker_enabled - @_response = T.let(nil, ResponseType) - super(dispatcher) dispatcher.register( self, :on_constant_read_node_enter, @@ -86,12 +83,7 @@ def on_call_node_enter(node) target_method = @index.resolve_method(message, @nesting.join("::")) return unless target_method - location = target_method.location - - @_response = Interface::Hover.new( - range: range_from_location(location), - contents: markdown_from_index_entries(message, target_method), - ) + @response_builder << markdown_from_index_entries(message, target_method) end private @@ -106,10 +98,7 @@ def generate_hover(name, location) first_entry = T.must(entries.first) return if first_entry.visibility == :private && first_entry.name != "#{@nesting.join("::")}::#{name}" - @_response = Interface::Hover.new( - range: range_from_location(location), - contents: markdown_from_index_entries(name, entries), - ) + @response_builder << markdown_from_index_entries(name, entries) end sig { params(node: Prism::CallNode).void } @@ -138,13 +127,7 @@ def generate_gem_hover(node) #{info} MARKDOWN - @_response = Interface::Hover.new( - range: range_from_location(node.location), - contents: Interface::MarkupContent.new( - kind: Constant::MarkupKind::MARKDOWN, - value: markdown, - ), - ) + @response_builder << markdown rescue Gem::MissingSpecError # Do nothing if the spec cannot be found end diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb index 7a3fd6327..c222cc3f1 100644 --- a/lib/ruby_lsp/requests/hover.rb +++ b/lib/ruby_lsp/requests/hover.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "ruby_lsp/listeners/hover" +require "ruby_lsp/response_builder" module RubyLsp module Requests @@ -52,19 +53,16 @@ def initialize(document, index, position, dispatcher, typechecker_enabled) target = parent end - @listeners = T.let([], T::Array[Listener[ResponseType]]) + @listeners = T.let([], T::Array[Listener[HoverResponseBuilder]]) # Don't need to instantiate any listeners if there's no target return unless target uri = document.uri - @listeners = T.let( - [Listeners::Hover.new(uri, nesting, index, dispatcher, typechecker_enabled)], - T::Array[Listener[ResponseType]], - ) + @response_builder = T.let(HoverResponseBuilder.new, HoverResponseBuilder) + Listeners::Hover.new(@response_builder, uri, nesting, index, dispatcher, typechecker_enabled) Addon.addons.each do |addon| - addon_listener = addon.create_hover_listener(nesting, index, dispatcher) - @listeners << addon_listener if addon_listener + addon.create_hover_listener(@response_builder, nesting, index, dispatcher) end @target = T.let(target, Prism::Node) @@ -74,18 +72,15 @@ def initialize(document, index, position, dispatcher, typechecker_enabled) sig { override.returns(ResponseType) } def perform @dispatcher.dispatch_once(@target) - responses = @listeners.map(&:response).compact - - first_response, *other_responses = responses - return unless first_response + return if @response_builder.empty? - # TODO: other_responses should never be nil. Check Sorbet - T.must(other_responses).each do |other_response| - first_response.contents.value << "\n\n" << other_response.contents.value - end - - first_response + Interface::Hover.new( + contents: Interface::MarkupContent.new( + kind: "markdown", + value: @response_builder.build_concatenated_response, + ), + ) end end end diff --git a/lib/ruby_lsp/requests/support/common.rb b/lib/ruby_lsp/requests/support/common.rb index 8996645b4..7f37c741c 100644 --- a/lib/ruby_lsp/requests/support/common.rb +++ b/lib/ruby_lsp/requests/support/common.rb @@ -86,7 +86,7 @@ def self_receiver?(node) params( title: String, entries: T.any(T::Array[RubyIndexer::Entry], RubyIndexer::Entry), - ).returns(Interface::MarkupContent) + ).returns(String) end def markdown_from_index_entries(title, entries) markdown_title = "```ruby\n#{title}\n```" @@ -108,16 +108,13 @@ def markdown_from_index_entries(title, entries) content << "\n\n#{entry.comments.join("\n")}" unless entry.comments.empty? end - Interface::MarkupContent.new( - kind: "markdown", - value: <<~MARKDOWN.chomp, - #{markdown_title} + <<~MARKDOWN.chomp + #{markdown_title} - **Definitions**: #{definitions.join(" | ")} + **Definitions**: #{definitions.join(" | ")} - #{content} - MARKDOWN - ) + #{content} + MARKDOWN end end end diff --git a/lib/ruby_lsp/response_builder.rb b/lib/ruby_lsp/response_builder.rb new file mode 100644 index 000000000..5ddcb3fbb --- /dev/null +++ b/lib/ruby_lsp/response_builder.rb @@ -0,0 +1,50 @@ +# typed: strict +# frozen_string_literal: true + +module RubyLsp + class OperationNotPermitted < StandardError; end + + class ResponseBuilder + extend T::Sig + extend T::Generic + + Elem = type_member { { upper: Object } } + + sig { void } + def initialize + @response = T.let([], T::Array[Elem]) + end + + sig { params(elem: Elem).void } + def <<(elem) + @response << elem + end + + sig { returns(T.nilable(Elem)) } + def pop + @response.pop + end + + sig { returns(T::Boolean) } + def empty? + @response.empty? + end + end + + class HoverResponseBuilder < ResponseBuilder + extend T::Sig + extend T::Generic + + Elem = type_member { { fixed: String } } + + sig { returns(String) } + def build_concatenated_response + @response.join("\n\n") + end + + sig { override.returns(T.nilable(Elem)) } + def pop + raise OperationNotPermitted, "Cannot pop from a HoverResponseBuilder" + end + end +end diff --git a/test/expectations/hover/documented_class.exp.json b/test/expectations/hover/documented_class.exp.json index 332ec3932..6a8cae5a7 100644 --- a/test/expectations/hover/documented_class.exp.json +++ b/test/expectations/hover/documented_class.exp.json @@ -9,16 +9,6 @@ "contents": { "kind": "markdown", "value": "```ruby\nBar\n```\n\n**Definitions**: [fake.rb](file:///fake.rb#L2,1-3,4) | [fake.rb](file:///fake.rb#L6,1-7,4)\n\n\n\nThis is the documentation for Bar\n\nThis is more documentation for Bar" - }, - "range": { - "start": { - "line": 1, - "character": 6 - }, - "end": { - "line": 1, - "character": 9 - } } } } diff --git a/test/expectations/hover/documented_constant.exp.json b/test/expectations/hover/documented_constant.exp.json index 07d3fdac9..cbbe5b37f 100644 --- a/test/expectations/hover/documented_constant.exp.json +++ b/test/expectations/hover/documented_constant.exp.json @@ -9,16 +9,6 @@ "contents": { "kind": "markdown", "value": "```ruby\nBAZ\n```\n\n**Definitions**: [fake.rb](file:///fake.rb#L2,1-2,10)\n\n\n\nThis is the documentation for Baz" - }, - "range": { - "start": { - "line": 1, - "character": 0 - }, - "end": { - "line": 1, - "character": 3 - } } } } diff --git a/test/expectations/hover/documented_module.exp.json b/test/expectations/hover/documented_module.exp.json index abc831ec7..3434c3b9c 100644 --- a/test/expectations/hover/documented_module.exp.json +++ b/test/expectations/hover/documented_module.exp.json @@ -9,16 +9,6 @@ "contents": { "kind": "markdown", "value": "```ruby\nFoo\n```\n\n**Definitions**: [fake.rb](file:///fake.rb#L2,1-3,4)\n\n\n\nThis is the documentation for Foo" - }, - "range": { - "start": { - "line": 1, - "character": 7 - }, - "end": { - "line": 1, - "character": 10 - } } } } diff --git a/test/expectations/hover/documented_namespaced_class.exp.json b/test/expectations/hover/documented_namespaced_class.exp.json index 3c26cd28f..1f95f2a69 100644 --- a/test/expectations/hover/documented_namespaced_class.exp.json +++ b/test/expectations/hover/documented_namespaced_class.exp.json @@ -9,16 +9,6 @@ "contents": { "kind": "markdown", "value": "```ruby\nFoo::Bar\n```\n\n**Definitions**: [fake.rb](file:///fake.rb#L2,1-3,4) | [fake.rb](file:///fake.rb#L6,1-7,4) | [fake.rb](file:///fake.rb#L11,3-12,6)\n\n\n\nThis is the documentation for Foo::Bar\n\nThis is more documentation for Foo::Bar\n\nThis is even more documentation for Foo::Bar" - }, - "range": { - "start": { - "line": 1, - "character": 6 - }, - "end": { - "line": 1, - "character": 14 - } } } } diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index f89f4b8cb..f0cc86280 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -261,26 +261,19 @@ def name "HoverAddon" end - def create_hover_listener(nesting, index, dispatcher) - klass = Class.new(RubyLsp::Listener) do - attr_reader :_response - - def initialize(dispatcher) - super + def create_hover_listener(response_builder, nesting, index, dispatcher) + klass = Class.new do + def initialize(response_builder, dispatcher) + @response_builder = response_builder dispatcher.register(self, :on_constant_read_node_enter) end def on_constant_read_node_enter(node) - T.bind(self, RubyLsp::Listener[T.untyped]) - contents = RubyLsp::Interface::MarkupContent.new( - kind: "markdown", - value: "Hello from middleware: #{node.slice}", - ) - @_response = RubyLsp::Interface::Hover.new(range: range_from_location(node.location), contents: contents) + @response_builder << "Hello from middleware: #{node.slice}" end end - klass.new(dispatcher) + klass.new(response_builder, dispatcher) end end end