Skip to content

Commit

Permalink
Fully adopt the new response builder pattern and remove the Listener …
Browse files Browse the repository at this point in the history
…class (#1337)

* Restructure Completion

* Restructure DocumentHighlight

* Restructure DocumentLink

* Restructure FoldingRanges

* Restructure InlayHints

* Restructure SignatureHelp

* Remove listener.rb

* Update check_docs task's messages

* Replace unnecessarily similar response builders with CollectionResponseBuilder
  • Loading branch information
st0012 authored Jan 30, 2024
1 parent 9e49c36 commit 06fa0e6
Show file tree
Hide file tree
Showing 31 changed files with 191 additions and 258 deletions.
12 changes: 4 additions & 8 deletions lib/rubocop/cop/ruby_lsp/use_register_with_handler_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ module RubyLsp
# # Register without handler method.
#
# # bad
# class MyListener < Listener
# class MyListener
# def initialize(dispatcher)
# super()
# dispatcher.register(
# self,
# :on_string_node_enter,
Expand All @@ -24,9 +23,8 @@ module RubyLsp
# end
#
# # good
# class MyListener < Listener
# class MyListener
# def initialize(dispatcher)
# super()
# dispatcher.register(
# self,
# :on_string_node_enter,
Expand All @@ -41,9 +39,8 @@ module RubyLsp
# # Handler method without register.
#
# # bad
# class MyListener < Listener
# class MyListener
# def initialize(dispatcher)
# super()
# dispatcher.register(
# self,
# )
Expand All @@ -54,9 +51,8 @@ module RubyLsp
# end
#
# # good
# class MyListener < Listener
# class MyListener
# def initialize(dispatcher)
# super()
# dispatcher.register(
# self,
# :on_string_node_enter,
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_lsp/addon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def name; end
# Creates a new CodeLens listener. This method is invoked on every CodeLens request
sig do
overridable.params(
response_builder: ResponseBuilders::CodeLens,
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::CodeLens],
uri: URI::Generic,
dispatcher: Prism::Dispatcher,
).void
Expand All @@ -123,7 +123,7 @@ def create_code_lens_listener(response_builder, uri, dispatcher); end
nesting: T::Array[String],
index: RubyIndexer::Index,
dispatcher: Prism::Dispatcher,
).returns(T.nilable(Listener[T.nilable(Interface::Hover)]))
).void
end
def create_hover_listener(response_builder, nesting, index, dispatcher); end

Expand All @@ -147,7 +147,7 @@ def create_semantic_highlighting_listener(response_builder, dispatcher); end
# Creates a new Definition listener. This method is invoked on every Definition request
sig do
overridable.params(
response_builder: ResponseBuilders::Definition,
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::Location],
uri: URI::Generic,
nesting: T::Array[String],
index: RubyIndexer::Index,
Expand Down
14 changes: 7 additions & 7 deletions lib/ruby_lsp/check_docs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

module RubyLsp
# This rake task checks that all requests or addons are fully documented. Add the rake task to your Rakefile and
# specify the absolute path for all files that must be required in order to discover all listeners and their related
# specify the absolute path for all files that must be required in order to discover all requests and their related
# GIFs
#
# # Rakefile
Expand All @@ -33,7 +33,7 @@ def initialize(require_files, gif_files)

sig { void }
def define_task
desc("Checks if all Ruby LSP listeners are documented")
desc("Checks if all Ruby LSP requests are documented")
task(@name) { run_task }
end

Expand All @@ -46,10 +46,10 @@ def gif_exists?(request_path)

sig { void }
def run_task
# Require all files configured to make sure all listeners are loaded
# Require all files configured to make sure all requests are loaded
@file_list.each { |f| require(f.delete_suffix(".rb")) }

# Find all classes that inherit from BaseRequest or Listener, which are the ones we want to make sure are
# Find all classes that inherit from BaseRequest, which are the ones we want to make sure are
# documented
features = ObjectSpace.each_object(Class).select do |k|
klass = T.unsafe(k)
Expand All @@ -69,7 +69,7 @@ def run_task
lines = File.readlines(file_path)
docs = []

# Extract the documentation on top of the listener constant
# Extract the documentation on top of the request constant
while (line = lines[line_number]&.strip) && line.start_with?("#")
docs.unshift(line)
line_number -= 1
Expand Down Expand Up @@ -116,15 +116,15 @@ def run_task

if missing_docs.any?
warn(<<~WARN)
The following listeners are missing documentation:
The following requests are missing documentation:
#{missing_docs.map { |k, v| "#{k}\n\n#{v.join("\n")}" }.join("\n\n")}
WARN

abort
end

puts "All listeners are documented!"
puts "All requests are documented!"
end
end
end
1 change: 0 additions & 1 deletion lib/ruby_lsp/internal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
require "ruby_lsp/executor"
require "ruby_lsp/requests"
require "ruby_lsp/response_builders"
require "ruby_lsp/listener"
require "ruby_lsp/document"
require "ruby_lsp/ruby_document"
require "ruby_lsp/store"
Expand Down
33 changes: 0 additions & 33 deletions lib/ruby_lsp/listener.rb

This file was deleted.

2 changes: 1 addition & 1 deletion lib/ruby_lsp/listeners/code_lens.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class CodeLens

sig do
params(
response_builder: ResponseBuilders::CodeLens,
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::CodeLens],
uri: URI::Generic,
lenses_configuration: RequestConfig,
dispatcher: Prism::Dispatcher,
Expand Down
23 changes: 9 additions & 14 deletions lib/ruby_lsp/listeners/completion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,21 @@

module RubyLsp
module Listeners
class Completion < Listener
class Completion
extend T::Sig
extend T::Generic

ResponseType = type_member { { fixed: T::Array[Interface::CompletionItem] } }

sig { override.returns(ResponseType) }
attr_reader :_response
include Requests::Support::Common

sig do
params(
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::CompletionItem],
index: RubyIndexer::Index,
nesting: T::Array[String],
typechecker_enabled: T::Boolean,
dispatcher: Prism::Dispatcher,
).void
end
def initialize(index, nesting, typechecker_enabled, dispatcher)
super(dispatcher)
@_response = T.let([], ResponseType)
def initialize(response_builder, index, nesting, typechecker_enabled, dispatcher)
@response_builder = response_builder
@index = index
@nesting = nesting
@typechecker_enabled = typechecker_enabled
Expand All @@ -39,7 +34,7 @@ def initialize(index, nesting, typechecker_enabled, dispatcher)
sig { params(node: Prism::StringNode).void }
def on_string_node_enter(node)
@index.search_require_paths(node.content).map!(&:require_path).sort!.each do |path|
@_response << build_completion(T.must(path), node)
@response_builder << build_completion(T.must(path), node)
end
end

Expand All @@ -52,7 +47,7 @@ def on_constant_read_node_enter(node)
candidates = @index.prefix_search(name, @nesting)
candidates.each do |entries|
complete_name = T.must(entries.first).name
@_response << build_entry_completion(
@response_builder << build_entry_completion(
complete_name,
name,
node,
Expand Down Expand Up @@ -96,7 +91,7 @@ def on_constant_path_node_enter(node)

full_name = aliased_namespace.empty? ? constant_name : "#{aliased_namespace}::#{constant_name}"

@_response << build_entry_completion(
@response_builder << build_entry_completion(
full_name,
name,
node,
Expand All @@ -123,7 +118,7 @@ def on_call_node_enter(node)
entry = entries.find { |e| e.is_a?(RubyIndexer::Entry::Member) && e.owner&.name == receiver.name }
next unless entry

@_response << build_method_completion(T.cast(entry, RubyIndexer::Entry::Member), node)
@response_builder << build_method_completion(T.cast(entry, RubyIndexer::Entry::Member), node)
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Definition

sig do
params(
response_builder: ResponseBuilders::Definition,
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::Location],
uri: URI::Generic,
nesting: T::Array[String],
index: RubyIndexer::Index,
Expand Down
17 changes: 6 additions & 11 deletions lib/ruby_lsp/listeners/document_highlight.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@

module RubyLsp
module Listeners
class DocumentHighlight < Listener
class DocumentHighlight
extend T::Sig

ResponseType = type_member { { fixed: T::Array[Interface::DocumentHighlight] } }
include Requests::Support::Common

GLOBAL_VARIABLE_NODES = T.let(
[
Expand Down Expand Up @@ -87,20 +86,16 @@ class DocumentHighlight < Listener
T::Array[T.class_of(Prism::Node)],
)

sig { override.returns(ResponseType) }
attr_reader :_response

sig do
params(
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::DocumentHighlight],
target: T.nilable(Prism::Node),
parent: T.nilable(Prism::Node),
dispatcher: Prism::Dispatcher,
).void
end
def initialize(target, parent, dispatcher)
super(dispatcher)

@_response = T.let([], T::Array[Interface::DocumentHighlight])
def initialize(response_builder, target, parent, dispatcher)
@response_builder = response_builder

return unless target && parent

Expand Down Expand Up @@ -521,7 +516,7 @@ def matches?(node, classes)

sig { params(kind: Integer, location: Prism::Location).void }
def add_highlight(kind, location)
@_response << Interface::DocumentHighlight.new(range: range_from_location(location), kind: kind)
@response_builder << Interface::DocumentHighlight.new(range: range_from_location(location), kind: kind)
end

sig { params(node: T.nilable(Prism::Node)).returns(T.nilable(String)) }
Expand Down
18 changes: 6 additions & 12 deletions lib/ruby_lsp/listeners/document_link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@

module RubyLsp
module Listeners
class DocumentLink < Listener
class DocumentLink
extend T::Sig
extend T::Generic

ResponseType = type_member { { fixed: T::Array[Interface::DocumentLink] } }
include Requests::Support::Common

GEM_TO_VERSION_MAP = T.let(
[*::Gem::Specification.default_stubs, *::Gem::Specification.stubs].map! do |s|
Expand Down Expand Up @@ -59,25 +57,21 @@ def gem_paths
end
end

sig { override.returns(ResponseType) }
attr_reader :_response

sig do
params(
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::DocumentLink],
uri: URI::Generic,
comments: T::Array[Prism::Comment],
dispatcher: Prism::Dispatcher,
).void
end
def initialize(uri, comments, dispatcher)
super(dispatcher)

def initialize(response_builder, uri, comments, dispatcher)
# Match the version based on the version in the RBI file name. Notice that the `@` symbol is sanitized to `%40`
# in the URI
@response_builder = response_builder
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])
@lines_to_comments = T.let(
comments.to_h do |comment|
[comment.location.end_line, comment]
Expand Down Expand Up @@ -137,7 +131,7 @@ def extract_document_link(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_builder << Interface::DocumentLink.new(
range: range_from_location(comment.location),
target: "file://#{file_path}##{uri.line_number}",
tooltip: "Jump to #{file_path}##{uri.line_number}",
Expand Down
Loading

0 comments on commit 06fa0e6

Please sign in to comment.