Make requests easier to extend (#972)

* Introduce Listener::Extensible module

* Introduce ExtensibleListener to simplify response merging

* Use `_response` as Listeners' internal interface

This will allow `ExternalListener` to encapsulate response merging logic
under `response`, which means we can always call `response` on the listener
regardless of whether it's an extensible or not.

Author: st0012

SERVER_EXTENSIONS.md

@@ -134,7 +134,7 @@ module RubyLsp
       ResponseType = type_member { { fixed: T.nilable(::RubyLsp::Interface::Hover) } }
 
       sig { override.returns(ResponseType) }
-      attr_reader :response
+      attr_reader :_response
 
       # Listeners are initialized with the EventEmitter. This object is used by the Ruby LSP to emit the events when it
       # finds nodes during AST analysis. Listeners must register which nodes they want to handle with the emitter (see
@@ -145,7 +145,7 @@ module RubyLsp
       def initialize(config, emitter, message_queue)
         super
 
-        @response = T.let(nil, ResponseType)
+        @_response = T.let(nil, ResponseType)
         @config = config
 
         # Register that this listener will handle `on_const` events (i.e.: whenever a constant is found in the code)
@@ -159,7 +159,7 @@ module RubyLsp
         # Certain helpers are made available to listeners to build LSP responses. The classes under `RubyLsp::Interface`
         # are generally used to build responses and they match exactly what the specification requests.
         contents = RubyLsp::Interface::MarkupContent.new(kind: "markdown", value: "Hello!")
-        @response = RubyLsp::Interface::Hover.new(range: range_from_syntax_tree_node(node), contents: contents)
+        @_response = RubyLsp::Interface::Hover.new(range: range_from_syntax_tree_node(node), contents: contents)
       end
     end
   end

lib/ruby_lsp/check_docs.rb

@@ -53,7 +53,8 @@ def run_task
       # documented
       features = ObjectSpace.each_object(Class).filter_map do |k|
         klass = T.unsafe(k)
-        klass if klass < RubyLsp::Requests::BaseRequest || klass < RubyLsp::Listener
+        klass if klass < RubyLsp::Requests::BaseRequest ||
+          (klass < RubyLsp::Listener && klass != RubyLsp::ExtensibleListener)
       end
 
       missing_docs = T.let(Hash.new { |h, k| h[k] = [] }, T::Hash[String, T::Array[String]])

lib/ruby_lsp/executor.rb

@@ -103,9 +103,6 @@ def run(request)
         semantic_highlighting = Requests::SemanticHighlighting.new(emitter, @message_queue)
         emitter.visit(document.tree) if document.parsed?
 
-        code_lens.merge_external_listeners_responses!
-        document_symbol.merge_external_listeners_responses!
-
         # Store all responses retrieve in this round of visits in the cache and then return the response for the request
         # we actually received
         document.cache_set("textDocument/documentSymbol", document_symbol.response)
@@ -299,7 +296,6 @@ def hover(uri, position)
       # Emit events for all listeners
       emitter.emit_for_target(target)
 
-      hover.merge_external_listeners_responses!
       hover.response
     end
 

lib/ruby_lsp/listener.rb

@@ -18,13 +18,42 @@ class Listener
     def initialize(emitter, message_queue)
       @emitter = emitter
       @message_queue = message_queue
-      @external_listeners = T.let([], T::Array[RubyLsp::Listener[ResponseType]])
+    end
+
+    sig { returns(ResponseType) }
+    def response
+      _response
     end
 
     # Override this method with an attr_reader that returns the response of your listener. The listener should
     # accumulate results in a @response variable and then provide the reader so that it is accessible
     sig { abstract.returns(ResponseType) }
-    def response; end
+    def _response; end
+  end
+
+  # ExtensibleListener is an abstract class to be used by requests that accept extensions.
+  class ExtensibleListener < Listener
+    extend T::Sig
+    extend T::Generic
+
+    ResponseType = type_member
+
+    abstract!
+
+    # When inheriting from ExtensibleListener, the `super` of constructor must be called **after** the subclass's own
+    # ivars have been initialized. This is because the constructor of ExtensibleListener calls
+    # `initialize_external_listener` which may depend on the subclass's ivars.
+    sig { params(emitter: EventEmitter, message_queue: Thread::Queue).void }
+    def initialize(emitter, message_queue)
+      super
+      @response_merged = T.let(false, T::Boolean)
+      @external_listeners = T.let(
+        Extension.extensions.filter_map do |ext|
+          initialize_external_listener(ext)
+        end,
+        T::Array[RubyLsp::Listener[ResponseType]],
+      )
+    end
 
     # Merge responses from all external listeners into the base listener's response. We do this to return a single
     # response to the editor including the results of all extensions
@@ -33,11 +62,21 @@ def merge_external_listeners_responses!
       @external_listeners.each { |l| merge_response!(l) }
     end
 
+    sig { returns(ResponseType) }
+    def response
+      merge_external_listeners_responses! unless @response_merged
+      super
+    end
+
+    sig do
+      abstract.params(extension: RubyLsp::Extension).returns(T.nilable(RubyLsp::Listener[ResponseType]))
+    end
+    def initialize_external_listener(extension); end
+
     # Does nothing by default. Requests that accept extensions should override this method to define how to merge
     # responses coming from external listeners
-    sig { overridable.params(other: Listener[T.untyped]).returns(T.self_type) }
+    sig { abstract.params(other: Listener[T.untyped]).returns(T.self_type) }
     def merge_response!(other)
-      self
     end
   end
 end

lib/ruby_lsp/requests/code_lens.rb

@@ -18,7 +18,7 @@ module Requests
     # class Test < Minitest::Test
     # end
     # ```
-    class CodeLens < Listener
+    class CodeLens < ExtensibleListener
       extend T::Sig
       extend T::Generic
 
@@ -29,23 +29,20 @@ class CodeLens < Listener
       SUPPORTED_TEST_LIBRARIES = T.let(["minitest", "test-unit"], T::Array[String])
 
       sig { override.returns(ResponseType) }
-      attr_reader :response
+      attr_reader :_response
 
       sig { params(uri: URI::Generic, emitter: EventEmitter, message_queue: Thread::Queue, test_library: String).void }
       def initialize(uri, emitter, message_queue, test_library)
-        super(emitter, message_queue)
-
         @uri = T.let(uri, URI::Generic)
-        @external_listeners.concat(
-          Extension.extensions.filter_map { |ext| ext.create_code_lens_listener(uri, emitter, message_queue) },
-        )
         @test_library = T.let(test_library, String)
-        @response = T.let([], ResponseType)
+        @_response = T.let([], ResponseType)
         @path = T.let(uri.to_standardized_path, T.nilable(String))
         # visibility_stack is a stack of [current_visibility, previous_visibility]
         @visibility_stack = T.let([["public", "public"]], T::Array[T::Array[T.nilable(String)]])
         @class_stack = T.let([], T::Array[String])
 
+        super(emitter, message_queue)
+
         emitter.register(
           self,
           :on_class,
@@ -149,9 +146,14 @@ def on_vcall(node)
         end
       end
 
+      sig { override.params(extension: RubyLsp::Extension).returns(T.nilable(Listener[ResponseType])) }
+      def initialize_external_listener(extension)
+        extension.create_code_lens_listener(@uri, @emitter, @message_queue)
+      end
+
       sig { override.params(other: Listener[ResponseType]).returns(T.self_type) }
       def merge_response!(other)
-        @response.concat(other.response)
+        @_response.concat(other.response)
         self
       end
 
@@ -174,23 +176,23 @@ def add_test_code_lens(node, name:, command:, kind:)
           },
         ]
 
-        @response << create_code_lens(
+        @_response << create_code_lens(
           node,
           title: "Run",
           command_name: "rubyLsp.runTest",
           arguments: arguments,
           data: { type: "test", kind: kind },
         )
 
-        @response << create_code_lens(
+        @_response << create_code_lens(
           node,
           title: "Run In Terminal",
           command_name: "rubyLsp.runTestInTerminal",
           arguments: arguments,
           data: { type: "test_in_terminal", kind: kind },
         )
 
-        @response << create_code_lens(
+        @_response << create_code_lens(
           node,
           title: "Debug",
           command_name: "rubyLsp.debugTest",
@@ -239,7 +241,7 @@ def generate_test_command(class_name:, method_name: nil)
 
       sig { params(node: SyntaxTree::Command, remote: String).void }
       def add_open_gem_remote_code_lens(node, remote)
-        @response << create_code_lens(
+        @_response << create_code_lens(
           node,
           title: "Open remote",
           command_name: "rubyLsp.openLink",

lib/ruby_lsp/requests/definition.rb

@@ -24,7 +24,7 @@ class Definition < Listener
       ResponseType = type_member { { fixed: T.nilable(T.any(T::Array[Interface::Location], Interface::Location)) } }
 
       sig { override.returns(ResponseType) }
-      attr_reader :response
+      attr_reader :_response
 
       sig do
         params(
@@ -41,7 +41,7 @@ def initialize(uri, nesting, index, emitter, message_queue)
         @uri = uri
         @nesting = nesting
         @index = index
-        @response = T.let(nil, ResponseType)
+        @_response = T.let(nil, ResponseType)
         emitter.register(self, :on_command, :on_const, :on_const_path_ref)
       end
 
@@ -76,7 +76,7 @@ def on_command(node)
           if entry
             candidate = entry.full_path
 
-            @response = Interface::Location.new(
+            @_response = Interface::Location.new(
               uri: URI::Generic.from_path(path: candidate).to_s,
               range: Interface::Range.new(
                 start: Interface::Position.new(line: 0, character: 0),
@@ -90,7 +90,7 @@ def on_command(node)
           current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : Dir.pwd
           candidate = File.expand_path(File.join(current_folder, required_file))
 
-          @response = Interface::Location.new(
+          @_response = Interface::Location.new(
             uri: URI::Generic.from_path(path: candidate).to_s,
             range: Interface::Range.new(
               start: Interface::Position.new(line: 0, character: 0),
@@ -113,7 +113,7 @@ def find_in_index(value)
           nil
         end
 
-        @response = entries.filter_map do |entry|
+        @_response = entries.filter_map do |entry|
           location = entry.location
           # If the project has Sorbet, then we only want to handle go to definition for constants defined in gems, as an
           # additional behavior on top of jumping to RBIs. Sorbet can already handle go to definition for all constants

lib/ruby_lsp/requests/document_highlight.rb

@@ -28,7 +28,7 @@ class DocumentHighlight < Listener
       ResponseType = type_member { { fixed: T::Array[Interface::DocumentHighlight] } }
 
       sig { override.returns(ResponseType) }
-      attr_reader :response
+      attr_reader :_response
 
       sig do
         params(
@@ -41,7 +41,7 @@ class DocumentHighlight < Listener
       def initialize(target, parent, emitter, message_queue)
         super(emitter, message_queue)
 
-        @response = T.let([], T::Array[Interface::DocumentHighlight])
+        @_response = T.let([], T::Array[Interface::DocumentHighlight])
 
         return unless target && parent
 
@@ -83,7 +83,7 @@ def on_node(node)
       sig { params(match: Support::HighlightTarget::HighlightMatch).void }
       def add_highlight(match)
         range = range_from_syntax_tree_node(match.node)
-        @response << Interface::DocumentHighlight.new(range: range, kind: match.type)
+        @_response << Interface::DocumentHighlight.new(range: range, kind: match.type)
       end
     end
   end

lib/ruby_lsp/requests/document_link.rb

@@ -73,7 +73,7 @@ def gem_paths
       end
 
       sig { override.returns(ResponseType) }
-      attr_reader :response
+      attr_reader :_response
 
       sig { params(uri: URI::Generic, emitter: EventEmitter, message_queue: Thread::Queue).void }
       def initialize(uri, emitter, message_queue)
@@ -84,7 +84,7 @@ def initialize(uri, emitter, message_queue)
         path = uri.to_standardized_path
         version_match = path ? /(?<=%40)[\d.]+(?=\.rbi$)/.match(path) : nil
         @gem_version = T.let(version_match && version_match[0], T.nilable(String))
-        @response = T.let([], T::Array[Interface::DocumentLink])
+        @_response = T.let([], T::Array[Interface::DocumentLink])
 
         emitter.register(self, :on_comment)
       end
@@ -99,7 +99,7 @@ def on_comment(node)
         file_path = self.class.gem_paths.dig(uri.gem_name, gem_version, CGI.unescape(uri.path))
         return if file_path.nil?
 
-        @response << Interface::DocumentLink.new(
+        @_response << Interface::DocumentLink.new(
           range: range_from_syntax_tree_node(node),
           target: "file://#{file_path}##{uri.line_number}",
           tooltip: "Jump to #{file_path}##{uri.line_number}",

lib/ruby_lsp/requests/document_symbol.rb

@@ -26,7 +26,7 @@ module Requests
     #   end
     # end
     # ```
-    class DocumentSymbol < Listener
+    class DocumentSymbol < ExtensibleListener
       extend T::Sig
       extend T::Generic
 
@@ -47,22 +47,18 @@ def initialize
       end
 
       sig { override.returns(T::Array[Interface::DocumentSymbol]) }
-      attr_reader :response
+      attr_reader :_response
 
       sig { params(emitter: EventEmitter, message_queue: Thread::Queue).void }
       def initialize(emitter, message_queue)
-        super
-
         @root = T.let(SymbolHierarchyRoot.new, SymbolHierarchyRoot)
-        @response = T.let(@root.children, T::Array[Interface::DocumentSymbol])
+        @_response = T.let(@root.children, T::Array[Interface::DocumentSymbol])
         @stack = T.let(
           [@root],
           T::Array[T.any(SymbolHierarchyRoot, Interface::DocumentSymbol)],
         )
 
-        @external_listeners.concat(
-          Extension.extensions.filter_map { |ext| ext.create_document_symbol_listener(emitter, message_queue) },
-        )
+        super
 
         emitter.register(
           self,
@@ -79,10 +75,15 @@ def initialize(emitter, message_queue)
         )
       end
 
+      sig { override.params(extension: RubyLsp::Extension).returns(T.nilable(Listener[ResponseType])) }
+      def initialize_external_listener(extension)
+        extension.create_document_symbol_listener(@emitter, @message_queue)
+      end
+
       # Merges responses from other listeners
       sig { override.params(other: Listener[ResponseType]).returns(T.self_type) }
       def merge_response!(other)
-        @response.concat(other.response)
+        @_response.concat(other.response)
         self
       end