From 8500e06bcbd4764df2898be3cc32d21863ddf24d Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 5 Apr 2023 15:41:37 -0400 Subject: [PATCH 1/3] Add extension activation point --- lib/ruby_lsp/executor.rb | 9 +++++++ lib/ruby_lsp/extension.rb | 53 +++++++++++++++++++++++++++++++++++++++ lib/ruby_lsp/internal.rb | 1 + test/extension_test.rb | 47 ++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+) create mode 100644 lib/ruby_lsp/extension.rb create mode 100644 test/extension_test.rb diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb index 59a4f7f1d7..08e6ef92f1 100644 --- a/lib/ruby_lsp/executor.rb +++ b/lib/ruby_lsp/executor.rb @@ -38,6 +38,15 @@ def run(request) when "initialize" initialize_request(request.dig(:params)) when "initialized" + errors = Extension.load_extensions.map(&:message) + @notifications << Notification.new( + message: "window/showMessage", + params: Interface::ShowMessageParams.new( + type: Constant::MessageType::ERROR, + message: "Error loading extensions: #{errors.join(", ")}", + ), + ) if errors.any? + warn("Ruby LSP is ready") VOID when "textDocument/didOpen" diff --git a/lib/ruby_lsp/extension.rb b/lib/ruby_lsp/extension.rb new file mode 100644 index 0000000000..1feaed93c1 --- /dev/null +++ b/lib/ruby_lsp/extension.rb @@ -0,0 +1,53 @@ +# typed: strict +# frozen_string_literal: true + +module RubyLsp + class Extension + class << self + extend T::Sig + extend T::Helpers + + abstract! + + sig { params(child_class: T.class_of(Extension)).void } + def inherited(child_class) + extensions << child_class + super + end + + sig { returns(T::Array[T.class_of(Extension)]) } + def extensions + @extensions ||= T.let([], T.nilable(T::Array[T.class_of(Extension)])) + end + + sig { returns(T::Array[StandardError]) } + def load_extensions + # Require all extensions entry points, which should be placed under + # `some_gem/lib/ruby_lsp/your_gem_name/extension.rb` + errors = ::Gem.find_files("ruby_lsp/*/extension.rb").filter_map do |extension| + require File.expand_path(extension) + nil + rescue => e + e + end + + # Activate each one of the discovered extensions. If any problems occur in the extensions, we don't want to + # fail to boot the server + extensions.each do |extension| + extension.activate + rescue => e + errors << e + end + + errors + end + + # Each extension should implement `MyExtension.activate` and use to: + # - Register request hooks + # - Perform any sort of initialization, such as reading information into memory or even spawning a separate + # process + sig { abstract.void } + def activate; end + end + end +end diff --git a/lib/ruby_lsp/internal.rb b/lib/ruby_lsp/internal.rb index a17e251180..d39f6967a5 100644 --- a/lib/ruby_lsp/internal.rb +++ b/lib/ruby_lsp/internal.rb @@ -15,3 +15,4 @@ require "ruby_lsp/requests" require "ruby_lsp/listener" require "ruby_lsp/store" +require "ruby_lsp/extension" diff --git a/test/extension_test.rb b/test/extension_test.rb new file mode 100644 index 0000000000..865904b663 --- /dev/null +++ b/test/extension_test.rb @@ -0,0 +1,47 @@ +# typed: true +# frozen_string_literal: true + +require "test_helper" + +module RubyLsp + class ExtensionTest < Minitest::Test + def test_registering_an_extension_invokes_activate_on_initialized + extension = Class.new(Extension) do + class << self + attr_reader :activated + + def activate + @activated = true + end + end + end + + Executor.new(RubyLsp::Store.new).execute({ method: "initialized" }) + assert_predicate(extension, :activated) + end + + def test_extensions_are_automatically_tracked + extension = Class.new(Extension) do + class << self + def activate; end + end + end + + assert_includes(Extension.extensions, extension) + end + + def test_load_extensions_returns_errors + Class.new(Extension) do + class << self + def activate + raise StandardError, "Failed to activate" + end + end + end + + error = T.must(Extension.load_extensions.first) + assert_instance_of(StandardError, error) + assert_equal("Failed to activate", error.message) + end + end +end From b4473b96508d14e827d0dd7087b2549879ce558d Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 5 Apr 2023 16:18:19 -0400 Subject: [PATCH 2/3] Add support for hover extensions --- lib/ruby_lsp/event_emitter.rb | 2 ++ lib/ruby_lsp/executor.rb | 13 +++++-- lib/ruby_lsp/listener.rb | 10 ++++++ lib/ruby_lsp/requests/hover.rb | 15 +++++++++ test/requests/hover_expectations_test.rb | 43 ++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 3 deletions(-) diff --git a/lib/ruby_lsp/event_emitter.rb b/lib/ruby_lsp/event_emitter.rb index 9fa7e0a9b3..ee831311b7 100644 --- a/lib/ruby_lsp/event_emitter.rb +++ b/lib/ruby_lsp/event_emitter.rb @@ -46,6 +46,8 @@ def emit_for_target(node) @event_to_listener_map[:on_call]&.each { |listener| T.unsafe(listener).on_call(node) } when SyntaxTree::ConstPathRef @event_to_listener_map[:on_const_path_ref]&.each { |listener| T.unsafe(listener).on_const_path_ref(node) } + when SyntaxTree::Const + @event_to_listener_map[:on_const]&.each { |listener| T.unsafe(listener).on_const(node) } end end end diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb index 08e6ef92f1..6a294fcd18 100644 --- a/lib/ruby_lsp/executor.rb +++ b/lib/ruby_lsp/executor.rb @@ -169,9 +169,16 @@ def hover(uri, position) target = parent end - listener = RubyLsp::Requests::Hover.new - EventEmitter.new(listener).emit_for_target(target) - listener.response + # Instantiate all listeners + base_listener = Requests::Hover.new + listeners = Requests::Hover.listeners.map(&:new) + + # Emit events for all listeners + T.unsafe(EventEmitter).new(base_listener, *listeners).emit_for_target(target) + + # Merge all responses into a single hover + listeners.each { |ext| base_listener.merge_response!(ext) } + base_listener.response end sig { params(uri: String).returns(T::Array[Interface::DocumentLink]) } diff --git a/lib/ruby_lsp/listener.rb b/lib/ruby_lsp/listener.rb index 85f514e6ee..40400ba4a7 100644 --- a/lib/ruby_lsp/listener.rb +++ b/lib/ruby_lsp/listener.rb @@ -20,6 +20,16 @@ class << self sig { returns(T.nilable(T::Array[Symbol])) } attr_reader :events + sig { returns(T::Array[T.class_of(Listener)]) } + def listeners + @listeners ||= T.let([], T.nilable(T::Array[T.class_of(Listener)])) + end + + sig { params(listener: T.class_of(Listener)).void } + def add_listener(listener) + listeners << listener + end + # All listener events must be defined inside of a `listener_events` block. This is to ensure we know which events # have been registered. Defining an event outside of this block will simply not register it and it'll never be # invoked diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb index 36ca4d7448..86952059d8 100644 --- a/lib/ruby_lsp/requests/hover.rb +++ b/lib/ruby_lsp/requests/hover.rb @@ -41,6 +41,21 @@ def initialize super() end + # Merges responses from other hover listeners + sig { params(other: Listener[ResponseType]).returns(T.self_type) } + def merge_response!(other) + other_response = other.response + return self unless other_response + + if @response.nil? + @response = other.response + else + @response.contents.value << other_response.contents.value << "\n\n" + end + + self + end + listener_events do sig { params(node: SyntaxTree::Command).void } def on_command(node) diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index 95ab0d5208..156261c29f 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -51,8 +51,51 @@ def run_expectations(source) }).response end + def test_after_request_hook + create_hover_hook_class + js_content = File.read(File.join(TEST_FIXTURES_DIR, "rails_search_index.js")) + fake_response = FakeHTTPResponse.new("200", js_content) + Net::HTTP.stubs(get_response: fake_response) + + store = RubyLsp::Store.new + store.set(uri: "file:///fake.rb", source: <<~RUBY, version: 1) + class Post + belongs_to :user + end + RUBY + + response = RubyLsp::Executor.new(store).execute({ + method: "textDocument/hover", + params: { textDocument: { uri: "file:///fake.rb" }, position: { line: 1, character: 2 } }, + }).response + + assert_match("Method from middleware: belongs_to", response.contents.value) + assert_match("[Rails Document: `ActiveRecord::Associations::ClassMethods#belongs_to`]", response.contents.value) + ensure + RubyLsp::Requests::Hover.listeners.clear + end + private + def create_hover_hook_class + Class.new(RubyLsp::Listener) do + attr_reader :response + + RubyLsp::Requests::Hover.add_listener(self) + + listener_events do + def on_command(node) + T.bind(self, RubyLsp::Listener[T.untyped]) + contents = RubyLsp::Interface::MarkupContent.new( + kind: "markdown", + value: "Method from middleware: #{node.message.value}", + ) + @response = RubyLsp::Interface::Hover.new(range: range_from_syntax_tree_node(node), contents: contents) + end + end + end + end + def substitute(original) original.gsub("RAILTIES_VERSION", Gem::Specification.find_by_name("railties").version.to_s) end From fd9a62de6166b43560119d8e02787a2bc80de0a5 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 12 Apr 2023 14:27:24 -0400 Subject: [PATCH 3/3] Improve extension activation --- lib/ruby_lsp/executor.rb | 23 ++++++---- lib/ruby_lsp/extension.rb | 89 ++++++++++++++++++++++++++++++--------- test/extension_test.rb | 58 +++++++++++++++---------- 3 files changed, 121 insertions(+), 49 deletions(-) diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb index 6a294fcd18..8a04597f36 100644 --- a/lib/ruby_lsp/executor.rb +++ b/lib/ruby_lsp/executor.rb @@ -38,14 +38,21 @@ def run(request) when "initialize" initialize_request(request.dig(:params)) when "initialized" - errors = Extension.load_extensions.map(&:message) - @notifications << Notification.new( - message: "window/showMessage", - params: Interface::ShowMessageParams.new( - type: Constant::MessageType::ERROR, - message: "Error loading extensions: #{errors.join(", ")}", - ), - ) if errors.any? + Extension.load_extensions + + errored_extensions = Extension.extensions.select(&:error?) + + if errored_extensions.any? + @notifications << Notification.new( + message: "window/showMessage", + params: Interface::ShowMessageParams.new( + type: Constant::MessageType::WARNING, + message: "Error loading extensions:\n\n#{errored_extensions.map(&:formatted_errors).join("\n\n")}", + ), + ) + + warn(errored_extensions.map(&:backtraces).join("\n\n")) + end warn("Ruby LSP is ready") VOID diff --git a/lib/ruby_lsp/extension.rb b/lib/ruby_lsp/extension.rb index 1feaed93c1..8c9a1957c3 100644 --- a/lib/ruby_lsp/extension.rb +++ b/lib/ruby_lsp/extension.rb @@ -2,52 +2,103 @@ # frozen_string_literal: true module RubyLsp + # To register an extension, inherit from this class and implement both `name` and `activate` + # + # # Example + # + # ```ruby + # module MyGem + # class MyExtension < Extension + # def activate + # # Perform any relevant initialization + # end + # + # def name + # "My extension name" + # end + # end + # end + # ``` class Extension + extend T::Sig + extend T::Helpers + + abstract! + class << self extend T::Sig - extend T::Helpers - - abstract! + # Automatically track and instantiate extension classes sig { params(child_class: T.class_of(Extension)).void } def inherited(child_class) - extensions << child_class + extensions << child_class.new super end - sig { returns(T::Array[T.class_of(Extension)]) } + sig { returns(T::Array[Extension]) } def extensions - @extensions ||= T.let([], T.nilable(T::Array[T.class_of(Extension)])) + @extensions ||= T.let([], T.nilable(T::Array[Extension])) end - sig { returns(T::Array[StandardError]) } + # Discovers and loads all extensions. Returns the list of activated extensions + sig { returns(T::Array[Extension]) } def load_extensions # Require all extensions entry points, which should be placed under # `some_gem/lib/ruby_lsp/your_gem_name/extension.rb` - errors = ::Gem.find_files("ruby_lsp/*/extension.rb").filter_map do |extension| + Gem.find_files("ruby_lsp/**/extension.rb").each do |extension| require File.expand_path(extension) - nil rescue => e - e + warn(e.message) + warn(e.backtrace.to_s) end # Activate each one of the discovered extensions. If any problems occur in the extensions, we don't want to # fail to boot the server extensions.each do |extension| extension.activate + nil rescue => e - errors << e + extension.add_error(e) end - - errors end + end + + sig { void } + def initialize + @errors = T.let([], T::Array[StandardError]) + end - # Each extension should implement `MyExtension.activate` and use to: - # - Register request hooks - # - Perform any sort of initialization, such as reading information into memory or even spawning a separate - # process - sig { abstract.void } - def activate; end + sig { params(error: StandardError).returns(T.self_type) } + def add_error(error) + @errors << error + self end + + sig { returns(T::Boolean) } + def error? + @errors.any? + end + + sig { returns(String) } + def formatted_errors + <<~ERRORS + #{name}: + #{@errors.map(&:message).join("\n")} + ERRORS + end + + sig { returns(String) } + def backtraces + @errors.filter_map(&:backtrace).join("\n\n") + end + + # Each extension should implement `MyExtension#activate` and use to perform any sort of initialization, such as + # reading information into memory or even spawning a separate process + sig { abstract.void } + def activate; end + + # Extensions should override the `name` method to return the extension name + sig { abstract.returns(String) } + def name; end end end diff --git a/test/extension_test.rb b/test/extension_test.rb index 865904b663..9b14515308 100644 --- a/test/extension_test.rb +++ b/test/extension_test.rb @@ -5,43 +5,57 @@ module RubyLsp class ExtensionTest < Minitest::Test - def test_registering_an_extension_invokes_activate_on_initialized - extension = Class.new(Extension) do - class << self - attr_reader :activated + def setup + @extension = Class.new(Extension) do + attr_reader :activated + + def activate + @activated = true + end - def activate - @activated = true - end + def name + "My extension" end end + end + def teardown + Extension.extensions.clear + end + + def test_registering_an_extension_invokes_activate_on_initialized Executor.new(RubyLsp::Store.new).execute({ method: "initialized" }) - assert_predicate(extension, :activated) + + extension_instance = T.must(Extension.extensions.find { |ext| ext.is_a?(@extension) }) + assert_predicate(extension_instance, :activated) end def test_extensions_are_automatically_tracked - extension = Class.new(Extension) do - class << self - def activate; end - end - end - - assert_includes(Extension.extensions, extension) + assert( + Extension.extensions.any? { |ext| ext.is_a?(@extension) }, + "Expected extension to be automatically tracked", + ) end def test_load_extensions_returns_errors Class.new(Extension) do - class << self - def activate - raise StandardError, "Failed to activate" - end + def activate + raise StandardError, "Failed to activate" + end + + def name + "My extension" end end - error = T.must(Extension.load_extensions.first) - assert_instance_of(StandardError, error) - assert_equal("Failed to activate", error.message) + Extension.load_extensions + error_extension = T.must(Extension.extensions.find(&:error?)) + + assert_predicate(error_extension, :error?) + assert_equal(<<~MESSAGE, error_extension.formatted_errors) + My extension: + Failed to activate + MESSAGE end end end