Skip to content

Commit

Permalink
Remove Singleton from formatters
Browse files Browse the repository at this point in the history
This will make it trivial to reload the rubocop config for Shopify#1457 without doing an
awkward `Singleton.__init__`.
With `GlobalState` being a thing now, I don't see the need for a singleton
  • Loading branch information
Earlopain committed Apr 9, 2024
1 parent 729df3e commit bdb3d1d
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 47 deletions.
6 changes: 2 additions & 4 deletions ADDONS.md
Expand Up @@ -222,15 +222,13 @@ class MyFormatterRubyLspAddon < RubyLsp::Addon
def activate(global_state, message_queue)
# The first argument is an identifier users can pick to select this formatter. To use this formatter, users must
# have rubyLsp.formatter configured to "my_formatter"
# The second argument is a singleton instance that implements the `FormatterRunner` interface (see below)
global_state.register_formatter("my_formatter", MyFormatterRunner.instance)
# The second argument is a class instance that implements the `FormatterRunner` interface (see below)
global_state.register_formatter("my_formatter", MyFormatterRunner.new)
end
end

# Custom formatter
class MyFormatter
# Make it a singleton class
include Singleton
# If using Sorbet to develop the addon, then include this interface to make sure the class is properly implemented
include RubyLsp::Requests::Support::Formatter

Expand Down
3 changes: 0 additions & 3 deletions lib/ruby_lsp/requests/support/rubocop_formatter.rb
Expand Up @@ -3,15 +3,12 @@

return unless defined?(RubyLsp::Requests::Support::RuboCopRunner)

require "singleton"

module RubyLsp
module Requests
module Support
class RuboCopFormatter
extend T::Sig
include Formatter
include Singleton

sig { void }
def initialize
Expand Down
3 changes: 0 additions & 3 deletions lib/ruby_lsp/requests/support/syntax_tree_formatter.rb
Expand Up @@ -8,15 +8,12 @@
return
end

require "singleton"

module RubyLsp
module Requests
module Support
# :nodoc:
class SyntaxTreeFormatter
extend T::Sig
include Singleton
include Support::Formatter

sig { void }
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_lsp/server.rb
Expand Up @@ -248,10 +248,10 @@ def run_initialized
end

if defined?(Requests::Support::RuboCopFormatter)
@global_state.register_formatter("rubocop", Requests::Support::RuboCopFormatter.instance)
@global_state.register_formatter("rubocop", Requests::Support::RuboCopFormatter.new)
end
if defined?(Requests::Support::SyntaxTreeFormatter)
@global_state.register_formatter("syntax_tree", Requests::Support::SyntaxTreeFormatter.instance)
@global_state.register_formatter("syntax_tree", Requests::Support::SyntaxTreeFormatter.new)
end

perform_initial_indexing(indexing_config)
Expand Down
2 changes: 1 addition & 1 deletion test/requests/code_actions_formatting_test.rb
Expand Up @@ -79,7 +79,7 @@ def assert_corrects_to_expected(diagnostic_code, code_action_title, source, expe
global_state.formatter = "rubocop"
global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.instance,
RubyLsp::Requests::Support::RuboCopFormatter.new,
)

diagnostics = RubyLsp::Requests::Diagnostics.new(global_state, document).perform
Expand Down
2 changes: 1 addition & 1 deletion test/requests/diagnostics_expectations_test.rb
Expand Up @@ -14,7 +14,7 @@ def run_expectations(source)
global_state.formatter = "rubocop"
global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.instance,
RubyLsp::Requests::Support::RuboCopFormatter.new,
)

stdout, _ = capture_io do
Expand Down
5 changes: 2 additions & 3 deletions test/requests/diagnostics_test.rb
Expand Up @@ -9,7 +9,7 @@ def setup
@global_state.formatter = "rubocop"
@global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.instance,
RubyLsp::Requests::Support::RuboCopFormatter.new,
)
end

Expand Down Expand Up @@ -78,7 +78,6 @@ def foo
RUBY

formatter_class = Class.new do
include Singleton
include RubyLsp::Requests::Support::Formatter

def run_diagnostic(uri, document)
Expand All @@ -96,7 +95,7 @@ def run_diagnostic(uri, document)
end
end

@global_state.register_formatter("my-custom-formatter", T.unsafe(formatter_class).instance)
@global_state.register_formatter("my-custom-formatter", T.unsafe(formatter_class).new)
@global_state.formatter = "my-custom-formatter"

diagnostics = T.must(RubyLsp::Requests::Diagnostics.new(@global_state, document).perform)
Expand Down
2 changes: 1 addition & 1 deletion test/requests/formatting_expectations_test.rb
Expand Up @@ -12,7 +12,7 @@ def run_expectations(source)
global_state.formatter = "rubocop"
global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.instance,
RubyLsp::Requests::Support::RuboCopFormatter.new,
)
document = RubyLsp::RubyDocument.new(source: source, version: 1, uri: URI::Generic.from_path(path: __FILE__))
RubyLsp::Requests::Formatting.new(global_state, document).perform&.first&.new_text
Expand Down
11 changes: 4 additions & 7 deletions test/requests/formatting_test.rb
Expand Up @@ -9,11 +9,11 @@ def setup
@global_state.formatter = "rubocop"
@global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.instance,
RubyLsp::Requests::Support::RuboCopFormatter.new,
)
@global_state.register_formatter(
"syntax_tree",
RubyLsp::Requests::Support::SyntaxTreeFormatter.instance,
RubyLsp::Requests::Support::SyntaxTreeFormatter.new,
)
@document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: URI::Generic.from_path(path: __FILE__))
class Foo
Expand Down Expand Up @@ -141,17 +141,15 @@ def test_returns_nil_when_formatter_is_invalid
end

def test_using_a_custom_formatter
require "singleton"
formatter_class = Class.new do
include Singleton
include RubyLsp::Requests::Support::Formatter

def run_formatting(uri, document)
"#{document.source}\n# formatter by my-custom-formatter"
end
end

@global_state.register_formatter("my-custom-formatter", T.unsafe(formatter_class).instance)
@global_state.register_formatter("my-custom-formatter", T.unsafe(formatter_class).new)
assert_includes(formatted_document("my-custom-formatter"), "# formatter by my-custom-formatter")
end

Expand Down Expand Up @@ -181,10 +179,9 @@ def with_syntax_tree_config_file(contents)
def clear_syntax_tree_runner_singleton_instance
return unless defined?(RubyLsp::Requests::Support::SyntaxTreeFormatter)

Singleton.__init__(RubyLsp::Requests::Support::SyntaxTreeFormatter)
@global_state.register_formatter(
"syntax_tree",
RubyLsp::Requests::Support::SyntaxTreeFormatter.instance,
RubyLsp::Requests::Support::SyntaxTreeFormatter.new,
)
end
end
49 changes: 27 additions & 22 deletions test/server_test.rb
Expand Up @@ -410,31 +410,31 @@ def test_sets_formatter_to_none_if_neither_rubocop_or_syntax_tree_are_present
end

def test_shows_error_if_formatter_set_to_rubocop_but_rubocop_not_available
capture_subprocess_io do
@server.process_message(id: 1, method: "initialize", params: {
initializationOptions: { formatter: "rubocop" },
})
# capture_subprocess_io do
@server.process_message(id: 1, method: "initialize", params: {
initializationOptions: { formatter: "rubocop" },
})

@server.global_state.register_formatter("rubocop", RubyLsp::Requests::Support::RuboCopFormatter.instance)
with_uninstalled_rubocop do
@server.process_message({ method: "initialized" })
end
@server.global_state.register_formatter("rubocop", RubyLsp::Requests::Support::RuboCopFormatter.new)
with_uninstalled_rubocop do
@server.process_message({ method: "initialized" })
end

assert_equal("none", @server.global_state.formatter)
assert_equal("none", @server.global_state.formatter)

# Remove the initialization notifications
@server.pop_response
@server.pop_response
@server.pop_response
# Remove the initialization notifications
@server.pop_response
@server.pop_response
@server.pop_response

notification = @server.pop_response
notification = @server.pop_response

assert_equal("window/showMessage", notification.method)
assert_equal(
"Ruby LSP formatter is set to `rubocop` but RuboCop was not found in the Gemfile or gemspec.",
T.cast(notification.params, RubyLsp::Interface::ShowMessageParams).message,
)
end
assert_equal("window/showMessage", notification.method)
assert_equal(
"Ruby LSP formatter is set to `rubocop` but RuboCop was not found in the Gemfile or gemspec.",
T.cast(notification.params, RubyLsp::Interface::ShowMessageParams).message,
)
# end
end

def test_initialize_sets_client_name
Expand Down Expand Up @@ -480,20 +480,25 @@ def with_uninstalled_rubocop(&block)
rubocop_paths = $LOAD_PATH.select { |path| path.include?("gems/rubocop") }
rubocop_paths.each { |path| $LOAD_PATH.delete(path) }

$LOADED_FEATURES.delete_if { |path| path.include?("ruby_lsp/requests/support/rubocop_runner") }
$LOADED_FEATURES.delete_if do |path|
path.include?("ruby_lsp/requests/support/rubocop_runner") ||
path.include?("ruby_lsp/requests/support/rubocop_formatter")
end

unload_rubocop_runner
block.call
ensure
$LOAD_PATH.unshift(*rubocop_paths)
unload_rubocop_runner
require "ruby_lsp/requests/support/rubocop_runner"
require "ruby_lsp/requests/support/rubocop_formatter"
end

def unload_rubocop_runner
RubyLsp::Requests::Support.send(:remove_const, :RuboCopRunner)
RubyLsp::Requests::Support.send(:remove_const, :RuboCopFormatter)
rescue NameError
# Depending on which tests have run prior to this one, `RuboCopRunner` may or may not be defined
# Depending on which tests have run prior to this one, the classes may or may not be defined
end

def stub_dependencies(rubocop:, syntax_tree:)
Expand Down

0 comments on commit bdb3d1d

Please sign in to comment.