From ae97f89309700b3dfe14c80632cb348e8f3441d0 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Mon, 3 Apr 2023 15:06:23 -0400 Subject: [PATCH 1/8] Extract locate to Document --- lib/ruby_lsp/document.rb | 55 ++++++++++++++++++++++++++++++++++++++++ test/document_test.rb | 38 +++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/lib/ruby_lsp/document.rb b/lib/ruby_lsp/document.rb index f6733c17e2..ede3595029 100644 --- a/lib/ruby_lsp/document.rb +++ b/lib/ruby_lsp/document.rb @@ -99,6 +99,61 @@ def create_scanner Scanner.new(@source, @encoding) end + sig do + params( + position: PositionShape, + node_types: T::Array[T.class_of(SyntaxTree::Node)], + ).returns([T.nilable(SyntaxTree::Node), T.nilable(SyntaxTree::Node)]) + end + def locate_node(position, node_types: []) + return [nil, nil] unless parsed? + + locate(T.must(@tree), create_scanner.find_char_position(position)) + end + + sig do + params( + node: SyntaxTree::Node, + char_position: Integer, + node_types: T::Array[T.class_of(SyntaxTree::Node)], + ).returns([T.nilable(SyntaxTree::Node), T.nilable(SyntaxTree::Node)]) + end + def locate(node, char_position, node_types: []) + queue = T.let(node.child_nodes.compact, T::Array[T.nilable(SyntaxTree::Node)]) + closest = node + parent = T.let(nil, T.nilable(SyntaxTree::Node)) + + until queue.empty? + candidate = queue.shift + + # Skip nil child nodes + next if candidate.nil? + + # Add the next child_nodes to the queue to be processed + queue.concat(candidate.child_nodes) + + # Skip if the current node doesn't cover the desired position + loc = candidate.location + next unless (loc.start_char...loc.end_char).cover?(char_position) + + # If the node's start character is already past the position, then we should've found the closest node + # already + break if char_position < loc.start_char + + # If there are node types to filter by, and the current node is not one of those types, then skip it + next if node_types.any? && node_types.none? { |type| candidate.class == type } + + # If the current node is narrower than or equal to the previous closest node, then it is more precise + closest_loc = closest.location + if loc.end_char - loc.start_char <= closest_loc.end_char - closest_loc.start_char + parent = closest + closest = candidate + end + end + + [closest, parent] + end + class Scanner extend T::Sig diff --git a/test/document_test.rb b/test/document_test.rb index 21cc590ca5..7b461e8e45 100644 --- a/test/document_test.rb +++ b/test/document_test.rb @@ -435,6 +435,44 @@ def foo assert_predicate(document, :syntax_error?) end + def test_locate + document = RubyLsp::Document.new(source: <<~RUBY, version: 1, uri: "file:///foo/bar.rb") + class Post < ActiveRecord::Base + scope :published do + # find posts that are published + where(published: true) + end + end + RUBY + + # Locate the `ActiveRecord` module + found, parent = document.locate_node({ line: 0, character: 19 }) + assert_instance_of(SyntaxTree::Const, found) + assert_equal("ActiveRecord", T.cast(found, SyntaxTree::Const).value) + + assert_instance_of(SyntaxTree::VarRef, parent) + assert_equal("ActiveRecord", T.cast(parent, SyntaxTree::VarRef).value.value) + + # Locate the `Base` class + found, parent = T.cast( + document.locate_node({ line: 0, character: 27 }), + [SyntaxTree::Const, SyntaxTree::ConstPathRef], + ) + assert_instance_of(SyntaxTree::Const, found) + assert_equal("Base", found.value) + + assert_instance_of(SyntaxTree::ConstPathRef, parent) + assert_equal("Base", parent.constant.value) + assert_equal("ActiveRecord", T.cast(parent.parent, SyntaxTree::VarRef).value.value) + + # Locate the `where` invocation + found, parent = T.cast(document.locate_node({ line: 3, character: 4 }), [SyntaxTree::Ident, SyntaxTree::CallNode]) + assert_instance_of(SyntaxTree::Ident, found) + assert_equal("where", found.value) + + assert_instance_of(SyntaxTree::CallNode, parent) + end + private def assert_error_edit(actual, error_range) From 10445326a5499f60c14218c6be3f687f9bb54dcf Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Mon, 3 Apr 2023 15:06:53 -0400 Subject: [PATCH 2/8] Migrate requests to use Document#locate --- lib/ruby_lsp/requests/code_action_resolve.rb | 4 +++- lib/ruby_lsp/requests/document_highlight.rb | 10 ++++------ lib/ruby_lsp/requests/path_completion.rb | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 15a0c5107d..2aedf5862f 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -54,7 +54,9 @@ def run extracted_source = T.must(@document.source[start_index...end_index]) # Find the closest statements node, so that we place the refactor in a valid position - closest_statements = locate(T.must(@document.tree), start_index, node_types: [SyntaxTree::Statements]).first + closest_statements = @document + .locate(T.must(@document.tree), start_index, node_types: [SyntaxTree::Statements]) + .first return Error::InvalidTargetRange if closest_statements.nil? # Find the node with the end line closest to the requested position, so that we can place the refactor diff --git a/lib/ruby_lsp/requests/document_highlight.rb b/lib/ruby_lsp/requests/document_highlight.rb index ba8944178b..584f676570 100644 --- a/lib/ruby_lsp/requests/document_highlight.rb +++ b/lib/ruby_lsp/requests/document_highlight.rb @@ -32,8 +32,7 @@ def initialize(document, position) @highlights = T.let([], T::Array[Interface::DocumentHighlight]) return unless document.parsed? - position = document.create_scanner.find_char_position(position) - @target = T.let(find(T.must(document.tree), position), T.nilable(Support::HighlightTarget)) + @target = T.let(find(position), T.nilable(Support::HighlightTarget)) end sig { override.returns(T.all(T::Array[Interface::DocumentHighlight], Object)) } @@ -68,12 +67,11 @@ def visit(node) sig do params( - node: SyntaxTree::Node, - position: Integer, + position: Document::PositionShape, ).returns(T.nilable(Support::HighlightTarget)) end - def find(node, position) - matched, parent = locate(node, position) + def find(position) + matched, parent = @document.locate_node(position) return unless matched && parent return unless matched.is_a?(SyntaxTree::Ident) || DIRECT_HIGHLIGHTS.include?(matched.class) diff --git a/lib/ruby_lsp/requests/path_completion.rb b/lib/ruby_lsp/requests/path_completion.rb index 89e9220c87..441def4189 100644 --- a/lib/ruby_lsp/requests/path_completion.rb +++ b/lib/ruby_lsp/requests/path_completion.rb @@ -29,8 +29,7 @@ def run # We can't verify if we're inside a require when there are syntax errors return [] if @document.syntax_error? - char_position = @document.create_scanner.find_char_position(@position) - target = T.let(find(char_position), T.nilable(SyntaxTree::TStringContent)) + target = T.let(find, T.nilable(SyntaxTree::TStringContent)) # no target means the we are not inside a `require` call return [] unless target @@ -51,11 +50,12 @@ def collect_load_path_files end end - sig { params(position: Integer).returns(T.nilable(SyntaxTree::TStringContent)) } - def find(position) - matched, parent = locate( + sig { returns(T.nilable(SyntaxTree::TStringContent)) } + def find + char_position = @document.create_scanner.find_char_position(@position) + matched, parent = @document.locate( T.must(@document.tree), - position, + char_position, node_types: [SyntaxTree::Command, SyntaxTree::CommandCall, SyntaxTree::CallNode], ) @@ -76,7 +76,7 @@ def find(position) path_node = argument.parts.first return unless path_node.is_a?(SyntaxTree::TStringContent) - return unless (path_node.location.start_char..path_node.location.end_char).cover?(position) + return unless (path_node.location.start_char..path_node.location.end_char).cover?(char_position) path_node end From 8b19c73d07529d923166b34909cec8eeae871ae3 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Mon, 3 Apr 2023 15:07:16 -0400 Subject: [PATCH 3/8] Create EventEmitter and Listener --- lib/ruby_lsp/event_emitter.rb | 29 +++++++++++++++++++++++++++++ lib/ruby_lsp/internal.rb | 2 ++ lib/ruby_lsp/listener.rb | 21 +++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 lib/ruby_lsp/event_emitter.rb create mode 100644 lib/ruby_lsp/listener.rb diff --git a/lib/ruby_lsp/event_emitter.rb b/lib/ruby_lsp/event_emitter.rb new file mode 100644 index 0000000000..bc81f11e85 --- /dev/null +++ b/lib/ruby_lsp/event_emitter.rb @@ -0,0 +1,29 @@ +# typed: strict +# frozen_string_literal: true + +module RubyLsp + class EventEmitter < SyntaxTree::Visitor + extend T::Sig + + sig { params(listeners: Listener).void } + def initialize(*listeners) + @listeners = listeners + + super() + end + + # Emit events for a specific node. This is similar to the regular `visit` method, but avoids going deeper into the + # tree for performance + sig { params(node: T.nilable(SyntaxTree::Node)).void } + def emit_for_position(node) + case node + when SyntaxTree::Command + @listeners.each { |listener| listener.on_command(node) } + when SyntaxTree::CallNode + @listeners.each { |listener| listener.on_call(node) } + when SyntaxTree::ConstPathRef + @listeners.each { |listener| listener.on_const_path_ref(node) } + end + end + end +end diff --git a/lib/ruby_lsp/internal.rb b/lib/ruby_lsp/internal.rb index f3508ec7be..a17e251180 100644 --- a/lib/ruby_lsp/internal.rb +++ b/lib/ruby_lsp/internal.rb @@ -11,5 +11,7 @@ require "ruby_lsp/utils" require "ruby_lsp/server" require "ruby_lsp/executor" +require "ruby_lsp/event_emitter" require "ruby_lsp/requests" +require "ruby_lsp/listener" require "ruby_lsp/store" diff --git a/lib/ruby_lsp/listener.rb b/lib/ruby_lsp/listener.rb new file mode 100644 index 0000000000..e4627f8478 --- /dev/null +++ b/lib/ruby_lsp/listener.rb @@ -0,0 +1,21 @@ +# typed: strict +# frozen_string_literal: true + +module RubyLsp + class Listener + extend T::Sig + extend T::Helpers + include Requests::Support::Common + + abstract! + + sig { overridable.params(node: SyntaxTree::Command).void } + def on_command(node); end + + sig { overridable.params(node: SyntaxTree::CallNode).void } + def on_call(node); end + + sig { overridable.params(node: SyntaxTree::ConstPathRef).void } + def on_const_path_ref(node); end + end +end From c03155acd4b9b26d67a0b5c191fd8cf6ccd94def Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Mon, 3 Apr 2023 15:07:43 -0400 Subject: [PATCH 4/8] Extract common helpers into module --- lib/ruby_lsp/requests.rb | 1 + lib/ruby_lsp/requests/base_request.rb | 87 +------------------------ lib/ruby_lsp/requests/support/common.rb | 55 ++++++++++++++++ test/requests/base_request_test.rb | 26 -------- 4 files changed, 58 insertions(+), 111 deletions(-) create mode 100644 lib/ruby_lsp/requests/support/common.rb diff --git a/lib/ruby_lsp/requests.rb b/lib/ruby_lsp/requests.rb index 3cd9e7a884..c1ad6dd0ed 100644 --- a/lib/ruby_lsp/requests.rb +++ b/lib/ruby_lsp/requests.rb @@ -48,6 +48,7 @@ module Support autoload :HighlightTarget, "ruby_lsp/requests/support/highlight_target" autoload :RailsDocumentClient, "ruby_lsp/requests/support/rails_document_client" autoload :PrefixTree, "ruby_lsp/requests/support/prefix_tree" + autoload :Common, "ruby_lsp/requests/support/common" end end end diff --git a/lib/ruby_lsp/requests/base_request.rb b/lib/ruby_lsp/requests/base_request.rb index 0dce4d7628..e6de33d701 100644 --- a/lib/ruby_lsp/requests/base_request.rb +++ b/lib/ruby_lsp/requests/base_request.rb @@ -7,6 +7,7 @@ module Requests class BaseRequest < SyntaxTree::Visitor extend T::Sig extend T::Helpers + include Support::Common abstract! @@ -31,94 +32,10 @@ def run; end # Syntax Tree implements `visit_all` using `map` instead of `each` for users who want to use the pattern # `result = visitor.visit(tree)`. However, we don't use that pattern and should avoid producing a new array for # every single node visited - sig { params(nodes: T::Array[SyntaxTree::Node]).void } + sig { params(nodes: T::Array[T.nilable(SyntaxTree::Node)]).void } def visit_all(nodes) nodes.each { |node| visit(node) } end - - sig { params(node: SyntaxTree::Node).returns(Interface::Range) } - def range_from_syntax_tree_node(node) - loc = node.location - - Interface::Range.new( - start: Interface::Position.new( - line: loc.start_line - 1, - character: loc.start_column, - ), - end: Interface::Position.new(line: loc.end_line - 1, character: loc.end_column), - ) - end - - sig do - params(node: T.any(SyntaxTree::ConstPathRef, SyntaxTree::ConstRef, SyntaxTree::TopConstRef)).returns(String) - end - def full_constant_name(node) - name = +node.constant.value - constant = T.let(node, SyntaxTree::Node) - - while constant.is_a?(SyntaxTree::ConstPathRef) - constant = constant.parent - - case constant - when SyntaxTree::ConstPathRef - name.prepend("#{constant.constant.value}::") - when SyntaxTree::VarRef - name.prepend("#{constant.value.value}::") - end - end - - name - end - - sig do - params( - node: SyntaxTree::Node, - position: Integer, - node_types: T::Array[T.class_of(SyntaxTree::Node)], - ).returns([T.nilable(SyntaxTree::Node), T.nilable(SyntaxTree::Node)]) - end - def locate(node, position, node_types: []) - queue = T.let(node.child_nodes.compact, T::Array[T.nilable(SyntaxTree::Node)]) - closest = node - - until queue.empty? - candidate = queue.shift - - # Skip nil child nodes - next if candidate.nil? - - # Add the next child_nodes to the queue to be processed - queue.concat(candidate.child_nodes) - - # Skip if the current node doesn't cover the desired position - loc = candidate.location - next unless (loc.start_char...loc.end_char).cover?(position) - - # If the node's start character is already past the position, then we should've found the closest node already - break if position < loc.start_char - - # If there are node types to filter by, and the current node is not one of those types, then skip it - next if node_types.any? && node_types.none? { |type| candidate.is_a?(type) } - - # If the current node is narrower than or equal to the previous closest node, then it is more precise - closest_loc = closest.location - if loc.end_char - loc.start_char <= closest_loc.end_char - closest_loc.start_char - parent = T.let(closest, SyntaxTree::Node) - closest = candidate - end - end - - [closest, parent] - end - - sig { params(node: T.nilable(SyntaxTree::Node), range: T.nilable(T::Range[Integer])).returns(T::Boolean) } - def visible?(node, range) - return true if range.nil? - return false if node.nil? - - loc = node.location - range.cover?(loc.start_line - 1) && range.cover?(loc.end_line - 1) - end end end end diff --git a/lib/ruby_lsp/requests/support/common.rb b/lib/ruby_lsp/requests/support/common.rb new file mode 100644 index 0000000000..5b7b6976b4 --- /dev/null +++ b/lib/ruby_lsp/requests/support/common.rb @@ -0,0 +1,55 @@ +# typed: strict +# frozen_string_literal: true + +module RubyLsp + module Requests + module Support + module Common + extend T::Sig + + sig { params(node: SyntaxTree::Node).returns(Interface::Range) } + def range_from_syntax_tree_node(node) + loc = node.location + + Interface::Range.new( + start: Interface::Position.new( + line: loc.start_line - 1, + character: loc.start_column, + ), + end: Interface::Position.new(line: loc.end_line - 1, character: loc.end_column), + ) + end + + sig do + params(node: T.any(SyntaxTree::ConstPathRef, SyntaxTree::ConstRef, SyntaxTree::TopConstRef)).returns(String) + end + def full_constant_name(node) + name = +node.constant.value + constant = T.let(node, SyntaxTree::Node) + + while constant.is_a?(SyntaxTree::ConstPathRef) + constant = constant.parent + + case constant + when SyntaxTree::ConstPathRef + name.prepend("#{constant.constant.value}::") + when SyntaxTree::VarRef + name.prepend("#{constant.value.value}::") + end + end + + name + end + + sig { params(node: T.nilable(SyntaxTree::Node), range: T.nilable(T::Range[Integer])).returns(T::Boolean) } + def visible?(node, range) + return true if range.nil? + return false if node.nil? + + loc = node.location + range.cover?(loc.start_line - 1) && range.cover?(loc.end_line - 1) + end + end + end + end +end diff --git a/test/requests/base_request_test.rb b/test/requests/base_request_test.rb index bf2948ed57..8a467cb2df 100644 --- a/test/requests/base_request_test.rb +++ b/test/requests/base_request_test.rb @@ -17,32 +17,6 @@ class Post < ActiveRecord::Base @fake_request = @request_class.new(@document) end - def test_locate - # Locate the `ActiveRecord` module (19 is the position of the `R` character) - found, parent = @fake_request.locate(@document.tree, 19) - assert_instance_of(SyntaxTree::Const, found) - assert_equal("ActiveRecord", found.value) - - assert_instance_of(SyntaxTree::VarRef, parent) - assert_equal("ActiveRecord", parent.value.value) - - # Locate the `Base` class (27 is the position of the `B` character) - found, parent = @fake_request.locate(@document.tree, 27) - assert_instance_of(SyntaxTree::Const, found) - assert_equal("Base", found.value) - - assert_instance_of(SyntaxTree::ConstPathRef, parent) - assert_equal("Base", parent.constant.value) - assert_equal("ActiveRecord", parent.parent.value.value) - - # Locate the `where` invocation (94 is the position of the `w` character) - found, parent = @fake_request.locate(@document.tree, 94) - assert_instance_of(SyntaxTree::Ident, found) - assert_equal("where", found.value) - - assert_instance_of(SyntaxTree::CallNode, parent) - end - # We can remove this once we drop support for Ruby 2.7 def test_super_is_valid_on_ruby_2_7 document = RubyLsp::Document.new(source: "", version: 1, uri: "file:///foo/bar.rb") From 8a0b73af25f9f24fb272885081aa755eb3bf929a Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Mon, 3 Apr 2023 15:07:55 -0400 Subject: [PATCH 5/8] Use Listener pattern for Hover --- lib/ruby_lsp/event_emitter.rb | 14 +++++- lib/ruby_lsp/executor.rb | 15 +++++- lib/ruby_lsp/listener.rb | 5 ++ lib/ruby_lsp/requests/hover.rb | 58 ++++++++++-------------- rakelib/check_docs.rake | 1 + test/requests/hover_expectations_test.rb | 17 +++++-- 6 files changed, 70 insertions(+), 40 deletions(-) diff --git a/lib/ruby_lsp/event_emitter.rb b/lib/ruby_lsp/event_emitter.rb index bc81f11e85..d3b09308c6 100644 --- a/lib/ruby_lsp/event_emitter.rb +++ b/lib/ruby_lsp/event_emitter.rb @@ -2,6 +2,18 @@ # frozen_string_literal: true module RubyLsp + # EventEmitter is an intermediary between our requests and Syntax Tree visitors. It's used to visit the document's AST + # and emit events that the requests can listen to for providing functionality. Usages: + # - For positional requests, locate the target node and use `emit_for_target` to fire events for each listener + # - For nonpositional requests, use `visit` to go through the AST, which will fire events for each listener as nodes + # are found + # = Example + # ```ruby + # target_node = document.locate_node(position) + # listener = Requests::Hover.new + # EventEmitter.new(listener).emit_for_target(target_node) + # listener.response + # ``` class EventEmitter < SyntaxTree::Visitor extend T::Sig @@ -15,7 +27,7 @@ def initialize(*listeners) # Emit events for a specific node. This is similar to the regular `visit` method, but avoids going deeper into the # tree for performance sig { params(node: T.nilable(SyntaxTree::Node)).void } - def emit_for_position(node) + def emit_for_target(node) case node when SyntaxTree::Command @listeners.each { |listener| listener.on_command(node) } diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb index 51a291ba9d..d035c88791 100644 --- a/lib/ruby_lsp/executor.rb +++ b/lib/ruby_lsp/executor.rb @@ -149,7 +149,20 @@ def code_lens(uri) ).returns(T.nilable(Interface::Hover)) end def hover(uri, position) - RubyLsp::Requests::Hover.new(@store.get(uri), position).run + document = @store.get(uri) + document.parse + return if document.syntax_error? + + target, parent = document.locate_node(position) + + if !Requests::Hover::ALLOWED_TARGETS.include?(target.class) && + Requests::Hover::ALLOWED_TARGETS.include?(parent.class) + target = parent + end + + listener = RubyLsp::Requests::Hover.new + EventEmitter.new(listener).emit_for_target(target) + 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 e4627f8478..786931865a 100644 --- a/lib/ruby_lsp/listener.rb +++ b/lib/ruby_lsp/listener.rb @@ -2,6 +2,8 @@ # frozen_string_literal: true module RubyLsp + # Listener is an abstract class to be used by requests for listening to events emitted when visiting an AST using the + # EventEmitter. class Listener extend T::Sig extend T::Helpers @@ -9,6 +11,9 @@ class Listener abstract! + sig { abstract.returns(Object) } + def response; end + sig { overridable.params(node: SyntaxTree::Command).void } def on_command(node); end diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb index f8bb36b61d..c443bddd82 100644 --- a/lib/ruby_lsp/requests/hover.rb +++ b/lib/ruby_lsp/requests/hover.rb @@ -17,7 +17,7 @@ module Requests # before_save :do_something # when hovering on before_save, the link will be rendered # end # ``` - class Hover < BaseRequest + class Hover < Listener extend T::Sig ALLOWED_TARGETS = T.let( @@ -29,53 +29,43 @@ class Hover < BaseRequest T::Array[T.class_of(SyntaxTree::Node)], ) - sig { params(document: Document, position: Document::PositionShape).void } - def initialize(document, position) - super(document) + sig { override.returns(T.nilable(Interface::Hover)) } + attr_reader :response - @position = T.let(document.create_scanner.find_char_position(position), Integer) + sig { void } + def initialize + @response = T.let(nil, T.nilable(Interface::Hover)) + super() end - sig { override.returns(T.nilable(Interface::Hover)) } - def run - return unless @document.parsed? + sig { override.params(node: SyntaxTree::Command).void } + def on_command(node) + message = node.message + @response = generate_rails_document_link_hover(message.value, message) + end - target, parent = locate(T.must(@document.tree), @position) - target = parent if !ALLOWED_TARGETS.include?(target.class) && ALLOWED_TARGETS.include?(parent.class) + sig { override.params(node: SyntaxTree::ConstPathRef).void } + def on_const_path_ref(node) + @response = generate_rails_document_link_hover(full_constant_name(node), node) + end - case target - when SyntaxTree::Command - message = target.message - generate_rails_document_link_hover(message.value, message) - when SyntaxTree::CallNode - message = target.message - return if message.is_a?(Symbol) + sig { override.params(node: SyntaxTree::CallNode).void } + def on_call(node) + message = node.message + return if message.is_a?(Symbol) - generate_rails_document_link_hover(message.value, message) - when SyntaxTree::ConstPathRef - constant_name = full_constant_name(target) - generate_rails_document_link_hover(constant_name, target) - end + @response = generate_rails_document_link_hover(message.value, message) end private - sig do - params(name: String, node: SyntaxTree::Node).returns(T.nilable(Interface::Hover)) - end + sig { params(name: String, node: SyntaxTree::Node).returns(T.nilable(Interface::Hover)) } def generate_rails_document_link_hover(name, node) urls = Support::RailsDocumentClient.generate_rails_document_urls(name) - return if urls.empty? - contents = Interface::MarkupContent.new( - kind: "markdown", - value: urls.join("\n\n"), - ) - Interface::Hover.new( - range: range_from_syntax_tree_node(node), - contents: contents, - ) + contents = Interface::MarkupContent.new(kind: "markdown", value: urls.join("\n\n")) + Interface::Hover.new(range: range_from_syntax_tree_node(node), contents: contents) end end end diff --git a/rakelib/check_docs.rake b/rakelib/check_docs.rake index ad487240b8..34e252d448 100644 --- a/rakelib/check_docs.rake +++ b/rakelib/check_docs.rake @@ -7,6 +7,7 @@ task :check_docs do require "language_server-protocol" require "syntax_tree" require "logger" + require "ruby_lsp/internal" require "ruby_lsp/requests/base_request" require "ruby_lsp/requests/support/rubocop_diagnostics_runner" require "ruby_lsp/requests/support/rubocop_formatting_runner" diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index 6886bc61d8..95ab0d5208 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -16,10 +16,14 @@ def assert_expectations(source, expected) end def test_search_index_being_nil - document = RubyLsp::Document.new(source: "belongs_to :foo", version: 1, uri: "file:///fake.rb") + store = RubyLsp::Store.new + store.set(uri: "file:///fake.rb", source: "belongs_to :foo", version: 1) RubyLsp::Requests::Support::RailsDocumentClient.stubs(search_index: nil) - RubyLsp::Requests::Hover.new(document, { character: 0, line: 0 }).run + RubyLsp::Executor.new(store).execute({ + method: "textDocument/hover", + params: { textDocument: { uri: "file:///fake.rb" }, position: { line: 0, character: 0 } }, + }).response end class FakeHTTPResponse @@ -32,14 +36,19 @@ def initialize(code, body) end def run_expectations(source) - document = RubyLsp::Document.new(source: source, version: 1, uri: "file:///fake.rb") js_content = File.read(File.join(TEST_FIXTURES_DIR, "rails_search_index.js")) fake_response = FakeHTTPResponse.new("200", js_content) position = @__params&.first || { character: 0, line: 0 } Net::HTTP.stubs(get_response: fake_response) - RubyLsp::Requests::Hover.new(document, position).run + store = RubyLsp::Store.new + store.set(uri: "file:///fake.rb", source: source, version: 1) + + RubyLsp::Executor.new(store).execute({ + method: "textDocument/hover", + params: { textDocument: { uri: "file:///fake.rb" }, position: position }, + }).response end private From 0808db8f648be9141cd3ab48364ef4dd4f371fcf Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 4 Apr 2023 11:30:06 -0400 Subject: [PATCH 6/8] More documentation --- lib/ruby_lsp/event_emitter.rb | 3 +++ lib/ruby_lsp/listener.rb | 2 ++ 2 files changed, 5 insertions(+) diff --git a/lib/ruby_lsp/event_emitter.rb b/lib/ruby_lsp/event_emitter.rb index d3b09308c6..91285cb361 100644 --- a/lib/ruby_lsp/event_emitter.rb +++ b/lib/ruby_lsp/event_emitter.rb @@ -4,10 +4,13 @@ module RubyLsp # EventEmitter is an intermediary between our requests and Syntax Tree visitors. It's used to visit the document's AST # and emit events that the requests can listen to for providing functionality. Usages: + # # - For positional requests, locate the target node and use `emit_for_target` to fire events for each listener # - For nonpositional requests, use `visit` to go through the AST, which will fire events for each listener as nodes # are found + # # = Example + # # ```ruby # target_node = document.locate_node(position) # listener = Requests::Hover.new diff --git a/lib/ruby_lsp/listener.rb b/lib/ruby_lsp/listener.rb index 786931865a..7f93788290 100644 --- a/lib/ruby_lsp/listener.rb +++ b/lib/ruby_lsp/listener.rb @@ -11,6 +11,8 @@ class Listener abstract! + # 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(Object) } def response; end From b721a3ed51d8e3674612678af1e4136bcb8ebeae Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 4 Apr 2023 11:37:00 -0400 Subject: [PATCH 7/8] Make Listener generic --- lib/ruby_lsp/event_emitter.rb | 2 +- lib/ruby_lsp/listener.rb | 5 ++++- lib/ruby_lsp/requests/hover.rb | 7 +++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/ruby_lsp/event_emitter.rb b/lib/ruby_lsp/event_emitter.rb index 91285cb361..f35ab936c5 100644 --- a/lib/ruby_lsp/event_emitter.rb +++ b/lib/ruby_lsp/event_emitter.rb @@ -20,7 +20,7 @@ module RubyLsp class EventEmitter < SyntaxTree::Visitor extend T::Sig - sig { params(listeners: Listener).void } + sig { params(listeners: Listener[T.untyped]).void } def initialize(*listeners) @listeners = listeners diff --git a/lib/ruby_lsp/listener.rb b/lib/ruby_lsp/listener.rb index 7f93788290..8e7ad4e6aa 100644 --- a/lib/ruby_lsp/listener.rb +++ b/lib/ruby_lsp/listener.rb @@ -7,13 +7,16 @@ module RubyLsp class Listener extend T::Sig extend T::Helpers + extend T::Generic include Requests::Support::Common + ResponseType = type_member + abstract! # 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(Object) } + sig { abstract.returns(ResponseType) } def response; end sig { overridable.params(node: SyntaxTree::Command).void } diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb index c443bddd82..47a8e10668 100644 --- a/lib/ruby_lsp/requests/hover.rb +++ b/lib/ruby_lsp/requests/hover.rb @@ -19,6 +19,9 @@ module Requests # ``` class Hover < Listener extend T::Sig + extend T::Generic + + ResponseType = type_member { { fixed: T.nilable(Interface::Hover) } } ALLOWED_TARGETS = T.let( [ @@ -29,12 +32,12 @@ class Hover < Listener T::Array[T.class_of(SyntaxTree::Node)], ) - sig { override.returns(T.nilable(Interface::Hover)) } + sig { override.returns(ResponseType) } attr_reader :response sig { void } def initialize - @response = T.let(nil, T.nilable(Interface::Hover)) + @response = T.let(nil, ResponseType) super() end From 37e69eb39cc8fbfbf1f377572e806c9cd6a9dec9 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 4 Apr 2023 13:10:41 -0400 Subject: [PATCH 8/8] Avoid invoking listener methods that haven't been registered --- lib/ruby_lsp/event_emitter.rb | 14 +++++++++++--- lib/ruby_lsp/listener.rb | 26 +++++++++++++++++--------- lib/ruby_lsp/requests/hover.rb | 30 ++++++++++++++++-------------- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/lib/ruby_lsp/event_emitter.rb b/lib/ruby_lsp/event_emitter.rb index f35ab936c5..9fa7e0a9b3 100644 --- a/lib/ruby_lsp/event_emitter.rb +++ b/lib/ruby_lsp/event_emitter.rb @@ -24,6 +24,14 @@ class EventEmitter < SyntaxTree::Visitor def initialize(*listeners) @listeners = listeners + # Create a map of event name to listeners that have registered it, so that we avoid unnecessary invocations + @event_to_listener_map = T.let( + listeners.each_with_object(Hash.new { |h, k| h[k] = [] }) do |listener, hash| + listener.class.events&.each { |event| hash[event] << listener } + end, + T::Hash[Symbol, T::Array[Listener[T.untyped]]], + ) + super() end @@ -33,11 +41,11 @@ def initialize(*listeners) def emit_for_target(node) case node when SyntaxTree::Command - @listeners.each { |listener| listener.on_command(node) } + @event_to_listener_map[:on_command]&.each { |listener| T.unsafe(listener).on_command(node) } when SyntaxTree::CallNode - @listeners.each { |listener| listener.on_call(node) } + @event_to_listener_map[:on_call]&.each { |listener| T.unsafe(listener).on_call(node) } when SyntaxTree::ConstPathRef - @listeners.each { |listener| listener.on_const_path_ref(node) } + @event_to_listener_map[:on_const_path_ref]&.each { |listener| T.unsafe(listener).on_const_path_ref(node) } end end end diff --git a/lib/ruby_lsp/listener.rb b/lib/ruby_lsp/listener.rb index 8e7ad4e6aa..85f514e6ee 100644 --- a/lib/ruby_lsp/listener.rb +++ b/lib/ruby_lsp/listener.rb @@ -14,18 +14,26 @@ class Listener abstract! + class << self + extend T::Sig + + sig { returns(T.nilable(T::Array[Symbol])) } + attr_reader :events + + # 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 + sig { params(block: T.proc.void).void } + def listener_events(&block) + current_methods = instance_methods + block.call + @events = T.let(instance_methods - current_methods, T.nilable(T::Array[Symbol])) + end + 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 - - sig { overridable.params(node: SyntaxTree::Command).void } - def on_command(node); end - - sig { overridable.params(node: SyntaxTree::CallNode).void } - def on_call(node); end - - sig { overridable.params(node: SyntaxTree::ConstPathRef).void } - def on_const_path_ref(node); end end end diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb index 47a8e10668..36ca4d7448 100644 --- a/lib/ruby_lsp/requests/hover.rb +++ b/lib/ruby_lsp/requests/hover.rb @@ -41,23 +41,25 @@ def initialize super() end - sig { override.params(node: SyntaxTree::Command).void } - def on_command(node) - message = node.message - @response = generate_rails_document_link_hover(message.value, message) - end + listener_events do + sig { params(node: SyntaxTree::Command).void } + def on_command(node) + message = node.message + @response = generate_rails_document_link_hover(message.value, message) + end - sig { override.params(node: SyntaxTree::ConstPathRef).void } - def on_const_path_ref(node) - @response = generate_rails_document_link_hover(full_constant_name(node), node) - end + sig { params(node: SyntaxTree::ConstPathRef).void } + def on_const_path_ref(node) + @response = generate_rails_document_link_hover(full_constant_name(node), node) + end - sig { override.params(node: SyntaxTree::CallNode).void } - def on_call(node) - message = node.message - return if message.is_a?(Symbol) + sig { params(node: SyntaxTree::CallNode).void } + def on_call(node) + message = node.message + return if message.is_a?(Symbol) - @response = generate_rails_document_link_hover(message.value, message) + @response = generate_rails_document_link_hover(message.value, message) + end end private