Skip to content

Commit

Permalink
Remove Singleton from formatters (#1904)
Browse files Browse the repository at this point in the history
This will make it trivial to reload the rubocop config for #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 10, 2024
1 parent 9e91813 commit 3850b82
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 28 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 @@ -245,10 +245,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
11 changes: 8 additions & 3 deletions test/server_test.rb
Expand Up @@ -415,7 +415,7 @@ def test_shows_error_if_formatter_set_to_rubocop_but_rubocop_not_available
initializationOptions: { formatter: "rubocop" },
})

@server.global_state.register_formatter("rubocop", RubyLsp::Requests::Support::RuboCopFormatter.instance)
@server.global_state.register_formatter("rubocop", RubyLsp::Requests::Support::RuboCopFormatter.new)
with_uninstalled_rubocop do
@server.process_message({ method: "initialized" })
end
Expand Down Expand Up @@ -501,20 +501,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 3850b82

Please sign in to comment.