From 9a211db6e303fadb32d7b782865bd9d88c3e7a51 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Tue, 5 Sep 2023 15:50:36 +0100 Subject: [PATCH] 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. --- SERVER_EXTENSIONS.md | 6 +++--- lib/ruby_lsp/executor.rb | 6 +++--- lib/ruby_lsp/listener.rb | 11 ++++++++--- lib/ruby_lsp/requests/code_lens.rb | 14 +++++++------- lib/ruby_lsp/requests/definition.rb | 10 +++++----- lib/ruby_lsp/requests/document_highlight.rb | 6 +++--- lib/ruby_lsp/requests/document_link.rb | 6 +++--- lib/ruby_lsp/requests/document_symbol.rb | 6 +++--- lib/ruby_lsp/requests/hover.rb | 12 ++++++------ lib/ruby_lsp/requests/inlay_hints.rb | 6 +++--- lib/ruby_lsp/requests/path_completion.rb | 6 +++--- lib/ruby_lsp/requests/semantic_highlighting.rb | 8 ++++---- test/requests/code_lens_expectations_test.rb | 4 ++-- test/requests/document_symbol_expectations_test.rb | 4 ++-- test/requests/hover_expectations_test.rb | 4 ++-- 15 files changed, 57 insertions(+), 52 deletions(-) diff --git a/SERVER_EXTENSIONS.md b/SERVER_EXTENSIONS.md index ec5d34bd44..a99c1f2d55 100644 --- a/SERVER_EXTENSIONS.md +++ b/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 diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb index 0bcf4bca33..d45653cac1 100644 --- a/lib/ruby_lsp/executor.rb +++ b/lib/ruby_lsp/executor.rb @@ -105,9 +105,9 @@ def run(request) # 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.merged_response) + document.cache_set("textDocument/documentSymbol", document_symbol.response) document.cache_set("textDocument/documentLink", document_link.response) - document.cache_set("textDocument/codeLens", code_lens.merged_response) + document.cache_set("textDocument/codeLens", code_lens.response) document.cache_set( "textDocument/semanticTokens/full", Requests::Support::SemanticTokenEncoder.new.encode(semantic_highlighting.response), @@ -291,7 +291,7 @@ def hover(uri, position) # Emit events for all listeners emitter.emit_for_target(target) - hover.merged_response + hover.response end sig { params(uri: URI::Generic, content_changes: T::Array[Document::EditShape], version: Integer).returns(Object) } diff --git a/lib/ruby_lsp/listener.rb b/lib/ruby_lsp/listener.rb index 7ad1f67f5f..4dc8b46b8e 100644 --- a/lib/ruby_lsp/listener.rb +++ b/lib/ruby_lsp/listener.rb @@ -20,10 +20,15 @@ def initialize(emitter, message_queue) @message_queue = message_queue 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. @@ -55,9 +60,9 @@ def merge_external_listeners_responses! end sig { returns(ResponseType) } - def merged_response + def response merge_external_listeners_responses! unless @response_merged - response + super end sig do diff --git a/lib/ruby_lsp/requests/code_lens.rb b/lib/ruby_lsp/requests/code_lens.rb index 7390bebb02..c8ec78f2f2 100644 --- a/lib/ruby_lsp/requests/code_lens.rb +++ b/lib/ruby_lsp/requests/code_lens.rb @@ -29,13 +29,13 @@ class CodeLens < ExtensibleListener 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) @uri = T.let(uri, URI::Generic) @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)]]) @@ -153,7 +153,7 @@ def initialize_external_listener(extension) sig { override.params(other: Listener[ResponseType]).returns(T.self_type) } def merge_response!(other) - @response.concat(other.response) + @_response.concat(other.response) self end @@ -176,7 +176,7 @@ def add_test_code_lens(node, name:, command:, kind:) }, ] - @response << create_code_lens( + @_response << create_code_lens( node, title: "Run", command_name: "rubyLsp.runTest", @@ -184,7 +184,7 @@ def add_test_code_lens(node, name:, command:, kind:) data: { type: "test", kind: kind }, ) - @response << create_code_lens( + @_response << create_code_lens( node, title: "Run In Terminal", command_name: "rubyLsp.runTestInTerminal", @@ -192,7 +192,7 @@ def add_test_code_lens(node, name:, command:, kind:) data: { type: "test_in_terminal", kind: kind }, ) - @response << create_code_lens( + @_response << create_code_lens( node, title: "Debug", command_name: "rubyLsp.debugTest", @@ -241,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", diff --git a/lib/ruby_lsp/requests/definition.rb b/lib/ruby_lsp/requests/definition.rb index f2147c33a9..8efac1eda7 100644 --- a/lib/ruby_lsp/requests/definition.rb +++ b/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 @@ -74,7 +74,7 @@ def on_command(node) candidate = find_file_in_load_path(required_file) if candidate - @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), @@ -88,7 +88,7 @@ def on_command(node) candidate = File.expand_path(File.join(current_folder, required_file)) if candidate - @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), @@ -112,7 +112,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 diff --git a/lib/ruby_lsp/requests/document_highlight.rb b/lib/ruby_lsp/requests/document_highlight.rb index bb0f1dd27c..da25948f21 100644 --- a/lib/ruby_lsp/requests/document_highlight.rb +++ b/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 diff --git a/lib/ruby_lsp/requests/document_link.rb b/lib/ruby_lsp/requests/document_link.rb index 69f69b55e3..9b7be56a79 100644 --- a/lib/ruby_lsp/requests/document_link.rb +++ b/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}", diff --git a/lib/ruby_lsp/requests/document_symbol.rb b/lib/ruby_lsp/requests/document_symbol.rb index d2f5f27fe3..e0cf36e3f2 100644 --- a/lib/ruby_lsp/requests/document_symbol.rb +++ b/lib/ruby_lsp/requests/document_symbol.rb @@ -47,12 +47,12 @@ 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) @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)], @@ -83,7 +83,7 @@ def initialize_external_listener(extension) # 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 diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb index b5d5497dff..d7d5c6a84c 100644 --- a/lib/ruby_lsp/requests/hover.rb +++ b/lib/ruby_lsp/requests/hover.rb @@ -30,7 +30,7 @@ class Hover < ExtensibleListener ) sig { override.returns(ResponseType) } - attr_reader :response + attr_reader :_response sig do params( @@ -43,7 +43,7 @@ class Hover < ExtensibleListener def initialize(index, nesting, emitter, message_queue) @nesting = nesting @index = index - @response = T.let(nil, ResponseType) + @_response = T.let(nil, ResponseType) super(emitter, message_queue) emitter.register(self, :on_const_path_ref, :on_const) @@ -60,10 +60,10 @@ def merge_response!(other) other_response = other.response return self unless other_response - if @response.nil? - @response = other.response + if @_response.nil? + @_response = other.response else - @response.contents.value << "\n\n" << other_response.contents.value + @_response.contents.value << "\n\n" << other_response.contents.value end self @@ -113,7 +113,7 @@ def generate_hover(name, node) kind: "markdown", value: "#{title}\n\n**Definitions**: #{definitions.join(" | ")}\n\n#{content}", ) - @response = Interface::Hover.new(range: range_from_syntax_tree_node(node), contents: contents) + @_response = Interface::Hover.new(range: range_from_syntax_tree_node(node), contents: contents) end end end diff --git a/lib/ruby_lsp/requests/inlay_hints.rb b/lib/ruby_lsp/requests/inlay_hints.rb index 7525808996..cbe6549837 100644 --- a/lib/ruby_lsp/requests/inlay_hints.rb +++ b/lib/ruby_lsp/requests/inlay_hints.rb @@ -27,13 +27,13 @@ class InlayHints < Listener RESCUE_STRING_LENGTH = T.let("rescue".length, Integer) sig { override.returns(ResponseType) } - attr_reader :response + attr_reader :_response sig { params(range: T::Range[Integer], emitter: EventEmitter, message_queue: Thread::Queue).void } def initialize(range, emitter, message_queue) super(emitter, message_queue) - @response = T.let([], ResponseType) + @_response = T.let([], ResponseType) @range = range emitter.register(self, :on_rescue) @@ -47,7 +47,7 @@ def on_rescue(node) loc = node.location return unless visible?(node, @range) - @response << Interface::InlayHint.new( + @_response << Interface::InlayHint.new( position: { line: loc.start_line - 1, character: loc.start_column + RESCUE_STRING_LENGTH }, label: "StandardError", padding_left: true, diff --git a/lib/ruby_lsp/requests/path_completion.rb b/lib/ruby_lsp/requests/path_completion.rb index 27f366c4b5..bf28e916e5 100644 --- a/lib/ruby_lsp/requests/path_completion.rb +++ b/lib/ruby_lsp/requests/path_completion.rb @@ -20,12 +20,12 @@ class PathCompletion < Listener ResponseType = type_member { { fixed: T::Array[Interface::CompletionItem] } } sig { override.returns(ResponseType) } - attr_reader :response + attr_reader :_response sig { params(emitter: EventEmitter, message_queue: Thread::Queue).void } def initialize(emitter, message_queue) super - @response = T.let([], ResponseType) + @_response = T.let([], ResponseType) @tree = T.let(Support::PrefixTree.new(collect_load_path_files), Support::PrefixTree) emitter.register(self, :on_tstring_content) @@ -34,7 +34,7 @@ def initialize(emitter, message_queue) sig { params(node: SyntaxTree::TStringContent).void } def on_tstring_content(node) @tree.search(node.value).sort.each do |path| - @response << build_completion(path, node) + @_response << build_completion(path, node) end end diff --git a/lib/ruby_lsp/requests/semantic_highlighting.rb b/lib/ruby_lsp/requests/semantic_highlighting.rb index a08512d348..d7f840e92a 100644 --- a/lib/ruby_lsp/requests/semantic_highlighting.rb +++ b/lib/ruby_lsp/requests/semantic_highlighting.rb @@ -105,7 +105,7 @@ def initialize(location:, length:, type:, modifier:) end sig { override.returns(ResponseType) } - attr_reader :response + attr_reader :_response sig do params( @@ -117,7 +117,7 @@ def initialize(location:, length:, type:, modifier:) def initialize(emitter, message_queue, range: nil) super(emitter, message_queue) - @response = T.let([], ResponseType) + @_response = T.let([], ResponseType) @range = range @special_methods = T.let(nil, T.nilable(T::Array[String])) @@ -174,7 +174,7 @@ def on_const(node) # When finding a module or class definition, we will have already pushed a token related to this constant. We # need to look at the previous two tokens and if they match this locatione exactly, avoid pushing another token # on top of the previous one - return if @response.last(2).any? { |token| token.location == node.location } + return if @_response.last(2).any? { |token| token.location == node.location } add_token(node.location, :namespace) end @@ -327,7 +327,7 @@ def on_module(node) def add_token(location, type, modifiers = []) length = location.end_char - location.start_char modifiers_indices = modifiers.filter_map { |modifier| TOKEN_MODIFIERS[modifier] } - @response.push( + @_response.push( SemanticToken.new( location: location, length: length, diff --git a/test/requests/code_lens_expectations_test.rb b/test/requests/code_lens_expectations_test.rb index 850f7b802a..c8145ada5c 100644 --- a/test/requests/code_lens_expectations_test.rb +++ b/test/requests/code_lens_expectations_test.rb @@ -130,7 +130,7 @@ def create_code_lens_listener(uri, emitter, message_queue) raise "uri can't be nil" unless uri klass = Class.new(RubyLsp::Listener) do - attr_reader :response + attr_reader :_response def initialize(uri, emitter, message_queue) super(emitter, message_queue) @@ -140,7 +140,7 @@ def initialize(uri, emitter, message_queue) def on_class(node) T.bind(self, RubyLsp::Listener[T.untyped]) - @response = [RubyLsp::Interface::CodeLens.new( + @_response = [RubyLsp::Interface::CodeLens.new( range: range_from_syntax_tree_node(node), command: RubyLsp::Interface::Command.new( title: "Run #{node.constant.constant.value}", diff --git a/test/requests/document_symbol_expectations_test.rb b/test/requests/document_symbol_expectations_test.rb index a225fef33f..473ebf69de 100644 --- a/test/requests/document_symbol_expectations_test.rb +++ b/test/requests/document_symbol_expectations_test.rb @@ -36,7 +36,7 @@ def name def create_document_symbol_listener(emitter, message_queue) klass = Class.new(RubyLsp::Listener) do - attr_reader :response + attr_reader :_response def initialize(emitter, message_queue) super @@ -51,7 +51,7 @@ def on_command(node) first_argument = node.arguments.parts.first test_name = first_argument.parts.map(&:value).join - @response = [RubyLsp::Interface::DocumentSymbol.new( + @_response = [RubyLsp::Interface::DocumentSymbol.new( name: test_name, kind: LanguageServer::Protocol::Constant::SymbolKind::METHOD, selection_range: range_from_syntax_tree_node(node), diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index 1049a2ef49..567ecc86be 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -63,7 +63,7 @@ def name def create_hover_listener(emitter, message_queue) klass = Class.new(RubyLsp::Listener) do - attr_reader :response + attr_reader :_response def initialize(emitter, message_queue) super @@ -76,7 +76,7 @@ def on_const(node) kind: "markdown", value: "Hello from middleware: #{node.value}", ) - @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