Skip to content

Commit

Permalink
Change our internal RuboCop integration identifier
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Jan 15, 2025
1 parent 77337ef commit b6f67d1
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 30 deletions.
30 changes: 27 additions & 3 deletions lib/ruby_lsp/global_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,20 @@ def apply_options(options)
@workspace_uri = URI(workspace_uri) if workspace_uri

specified_formatter = options.dig(:initializationOptions, :formatter)
rubocop_has_addon = Gem::Requirement.new(">= 1.70.0").satisfied_by?(Gem::Version.new(::RuboCop::Version::STRING))

if specified_formatter
@formatter = specified_formatter

if specified_formatter != "auto"
notifications << Notification.window_log_message("Using formatter specified by user: #{@formatter}")
end

# If the user had originally configured to use `rubocop`, but their version doesn't provide the add-on yet,
# fallback to the internal integration
if specified_formatter == "rubocop" && !rubocop_has_addon
@formatter = "rubocop_internal"
end
end

if @formatter == "auto"
Expand All @@ -109,6 +116,23 @@ def apply_options(options)
end

specified_linters = options.dig(:initializationOptions, :linters)

if specified_formatter == "rubocop" || specified_linters&.include?("rubocop")
notifications << Notification.window_log_message(<<~MESSAGE, type: Constant::MessageType::WARNING)
Formatter is configured to be `rubocop`. As of RuboCop v1.70.0, this identifier activates the add-on
implemented in the rubocop gem itself instead of the internal integrations provided by the Ruby LSP.
If you wish to use the internal integration, please configure the formatter as `rubocop_internal`.
MESSAGE
end

# If the user had originally configured to use `rubocop`, but their version doesn't provide the add-on yet,
# fallback to the internal integration
if specified_linters&.include?("rubocop") && !rubocop_has_addon
specified_linters.delete("rubocop")
specified_linters << "rubocop_internal"
end

@linters = specified_linters || detect_linters(direct_dependencies, all_dependencies)

notifications << if specified_linters
Expand Down Expand Up @@ -185,13 +209,13 @@ def supports_watching_files
sig { params(direct_dependencies: T::Array[String], all_dependencies: T::Array[String]).returns(String) }
def detect_formatter(direct_dependencies, all_dependencies)
# NOTE: Intentionally no $ at end, since we want to match rubocop-shopify, etc.
return "rubocop" if direct_dependencies.any?(/^rubocop/)
return "rubocop_internal" if direct_dependencies.any?(/^rubocop/)

syntax_tree_is_direct_dependency = direct_dependencies.include?("syntax_tree")
return "syntax_tree" if syntax_tree_is_direct_dependency

rubocop_is_transitive_dependency = all_dependencies.include?("rubocop")
return "rubocop" if dot_rubocop_yml_present && rubocop_is_transitive_dependency
return "rubocop_internal" if dot_rubocop_yml_present && rubocop_is_transitive_dependency

"none"
end
Expand All @@ -203,7 +227,7 @@ def detect_linters(dependencies, all_dependencies)
linters = []

if dependencies.any?(/^rubocop/) || (all_dependencies.include?("rubocop") && dot_rubocop_yml_present)
linters << "rubocop"
linters << "rubocop_internal"
end

linters
Expand Down
11 changes: 5 additions & 6 deletions lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ def run_initialized
unless @setup_error
if defined?(Requests::Support::RuboCopFormatter)
begin
@global_state.register_formatter("rubocop", Requests::Support::RuboCopFormatter.new)
@global_state.register_formatter("rubocop_internal", Requests::Support::RuboCopFormatter.new)
rescue ::RuboCop::Error => e
# The user may have provided unknown config switches in .rubocop or
# is trying to load a non-existent config file.
Expand Down Expand Up @@ -1040,7 +1040,7 @@ def handle_rubocop_config_change(uri)
return unless defined?(Requests::Support::RuboCopFormatter)

send_log_message("Reloading RuboCop since #{uri} changed")
@global_state.register_formatter("rubocop", Requests::Support::RuboCopFormatter.new)
@global_state.register_formatter("rubocop_internal", Requests::Support::RuboCopFormatter.new)

# Clear all existing diagnostics since the config changed. This has to happen under a mutex because the `state`
# hash cannot be mutated during iteration or that will throw an error
Expand Down Expand Up @@ -1211,16 +1211,15 @@ def end_progress(id)
sig { void }
def check_formatter_is_available
return if @setup_error
# Warn of an unavailable `formatter` setting, e.g. `rubocop` on a project which doesn't have RuboCop.
# Syntax Tree will always be available via Ruby LSP so we don't need to check for it.
return unless @global_state.formatter == "rubocop"
# Warn of an unavailable `formatter` setting, e.g. `rubocop_internal` on a project which doesn't have RuboCop.
return unless @global_state.formatter == "rubocop_internal"

unless defined?(RubyLsp::Requests::Support::RuboCopRunner)
@global_state.formatter = "none"

send_message(
Notification.window_show_message(
"Ruby LSP formatter is set to `rubocop` but RuboCop was not found in the Gemfile or gemspec.",
"Ruby LSP formatter is set to `rubocop_internal` but RuboCop was not found in the Gemfile or gemspec.",
type: Constant::MessageType::ERROR,
),
)
Expand Down
65 changes: 59 additions & 6 deletions test/global_state_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ def test_apply_option_selects_formatter
def test_applying_auto_formatter_invokes_detection
state = GlobalState.new
state.apply_options({ initializationOptions: { formatter: "auto" } })
assert_equal("rubocop", state.formatter)
assert_equal("rubocop_internal", state.formatter)
end

def test_applying_auto_formatter_with_rubocop_extension
state = GlobalState.new
stub_direct_dependencies("rubocop-rails" => "1.2.3")
state.apply_options({ initializationOptions: { formatter: "auto" } })
assert_equal("rubocop", state.formatter)
assert_equal("rubocop_internal", state.formatter)
end

def test_applying_auto_formatter_with_rubocop_as_transitive_dependency
Expand All @@ -82,7 +82,7 @@ def test_applying_auto_formatter_with_rubocop_as_transitive_dependency

state.apply_options({ initializationOptions: { formatter: "auto" } })

assert_equal("rubocop", state.formatter)
assert_equal("rubocop_internal", state.formatter)
end

def test_applying_auto_formatter_with_rubocop_as_transitive_dependency_without_config
Expand Down Expand Up @@ -150,20 +150,21 @@ def test_watching_files_if_not_reported
end

def test_linter_specification
::RuboCop::Version.const_set(:STRING, "1.68.0")
state = GlobalState.new
state.apply_options({
initializationOptions: { linters: ["rubocop", "brakeman"] },
})

assert_equal(["rubocop", "brakeman"], state.instance_variable_get(:@linters))
assert_equal(["brakeman", "rubocop_internal"], state.instance_variable_get(:@linters))
end

def test_linter_auto_detection
stub_direct_dependencies("rubocop" => "1.2.3")
state = GlobalState.new
state.apply_options({})

assert_equal(["rubocop"], state.instance_variable_get(:@linters))
assert_equal(["rubocop_internal"], state.instance_variable_get(:@linters))
end

def test_specifying_empty_linters
Expand All @@ -185,7 +186,7 @@ def test_linter_auto_detection_with_rubocop_as_transitive_dependency

state.apply_options({})

assert_includes(state.instance_variable_get(:@linters), "rubocop")
assert_includes(state.instance_variable_get(:@linters), "rubocop_internal")
end

def test_type_checker_is_detected_based_on_transitive_sorbet_static
Expand Down Expand Up @@ -249,6 +250,58 @@ def test_enabled_feature_always_returns_true_if_all_are_enabled
assert(state.enabled_feature?(:whatever))
end

def test_notifies_the_user_when_using_rubocop_addon_through_linters
::RuboCop::Version.const_set(:STRING, "1.70.0")

state = GlobalState.new
notifications = state.apply_options({ initializationOptions: { linters: ["rubocop"] } })

log = notifications.find do |n|
n.method == "window/logMessage" && T.unsafe(n.params).message.include?("RuboCop v1.70.0")
end
refute_nil(log)
assert_equal(["rubocop"], state.instance_variable_get(:@linters))
end

def test_notifies_the_user_when_using_rubocop_addon_through_formatter
::RuboCop::Version.const_set(:STRING, "1.70.0")

state = GlobalState.new
notifications = state.apply_options({ initializationOptions: { formatter: "rubocop" } })

log = notifications.find do |n|
n.method == "window/logMessage" && T.unsafe(n.params).message.include?("RuboCop v1.70.0")
end
refute_nil(log)
assert_equal("rubocop", state.formatter)
end

def test_falls_back_to_internal_integration_for_linters_if_rubocop_has_no_addon
::RuboCop::Version.const_set(:STRING, "1.68.0")

state = GlobalState.new
notifications = state.apply_options({ initializationOptions: { linters: ["rubocop"] } })

log = notifications.find do |n|
n.method == "window/logMessage" && T.unsafe(n.params).message.include?("RuboCop v1.70.0")
end
refute_nil(log)
assert_equal(["rubocop_internal"], state.instance_variable_get(:@linters))
end

def test_falls_back_to_internal_integration_for_formatters_if_rubocop_has_no_addon
::RuboCop::Version.const_set(:STRING, "1.68.0")

state = GlobalState.new
notifications = state.apply_options({ initializationOptions: { formatter: "rubocop" } })

log = notifications.find do |n|
n.method == "window/logMessage" && T.unsafe(n.params).message.include?("RuboCop v1.70.0")
end
refute_nil(log)
assert_equal("rubocop_internal", state.formatter)
end

private

def stub_direct_dependencies(dependencies)
Expand Down
4 changes: 2 additions & 2 deletions test/requests/code_actions_formatting_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ def assert_fixtures_match(name, diagnostic_code, code_action_title)
def assert_corrects_to_expected(diagnostic_code, code_action_title, source, expected)
global_state = RubyLsp::GlobalState.new
global_state.apply_options({
initializationOptions: { linters: ["rubocop"] },
initializationOptions: { linters: ["rubocop_internal"] },
})
global_state.register_formatter(
"rubocop",
"rubocop_internal",
RubyLsp::Requests::Support::RuboCopFormatter.new,
)

Expand Down
4 changes: 2 additions & 2 deletions test/requests/diagnostics_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ class DiagnosticsExpectationsTest < ExpectationsTestRunner
def run_expectations(source)
result = T.let(nil, T.nilable(T::Array[RubyLsp::Interface::Diagnostic]))
@global_state.apply_options({
initializationOptions: { linters: ["rubocop"] },
initializationOptions: { linters: ["rubocop_internal"] },
})
@global_state.register_formatter(
"rubocop",
"rubocop_internal",
RubyLsp::Requests::Support::RuboCopFormatter.new,
)

Expand Down
6 changes: 3 additions & 3 deletions test/requests/diagnostics_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ def setup
@uri = URI("file:///fake/file.rb")
@global_state = RubyLsp::GlobalState.new
@global_state.apply_options({
initializationOptions: { linters: ["rubocop"] },
initializationOptions: { linters: ["rubocop_internal"] },
})
@global_state.register_formatter(
"rubocop",
"rubocop_internal",
RubyLsp::Requests::Support::RuboCopFormatter.new,
)
end
Expand Down Expand Up @@ -52,7 +52,7 @@ def foo
klass = RubyLsp::Requests::Support::RuboCopFormatter
RubyLsp::Requests::Support.send(:remove_const, :RuboCopFormatter)

@global_state.instance_variable_get(:@supported_formatters).delete("rubocop")
@global_state.instance_variable_get(:@supported_formatters).delete("rubocop_internal")

diagnostics = T.must(RubyLsp::Requests::Diagnostics.new(@global_state, document).perform)

Expand Down
4 changes: 2 additions & 2 deletions test/requests/formatting_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ class FormattingExpectationsTest < ExpectationsTestRunner
expectations_tests RubyLsp::Requests::Formatting, "formatting"

def run_expectations(source)
@global_state.formatter = "rubocop"
@global_state.formatter = "rubocop_internal"
@global_state.register_formatter(
"rubocop",
"rubocop_internal",
RubyLsp::Requests::Support::RuboCopFormatter.new,
)
document = RubyLsp::RubyDocument.new(
Expand Down
10 changes: 5 additions & 5 deletions test/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def test_server_info_includes_version
end

def test_server_info_includes_formatter
@server.global_state.expects(:formatter).twice.returns("rubocop")
@server.global_state.expects(:formatter).twice.returns("rubocop_internal")
capture_subprocess_io do
@server.process_message({
id: 1,
Expand All @@ -185,7 +185,7 @@ def test_server_info_includes_formatter

result = find_message(RubyLsp::Result, id: 1)
hash = JSON.parse(result.response.to_json)
assert_equal("rubocop", hash.dig("formatter"))
assert_equal("rubocop_internal", hash.dig("formatter"))
end

def test_initialized_recovers_from_indexing_failures
Expand Down Expand Up @@ -375,10 +375,10 @@ def test_handles_invalid_configuration
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" },
initializationOptions: { formatter: "rubocop_internal" },
})

@server.global_state.register_formatter("rubocop", RubyLsp::Requests::Support::RuboCopFormatter.new)
@server.global_state.register_formatter("rubocop_internal", RubyLsp::Requests::Support::RuboCopFormatter.new)
with_uninstalled_rubocop do
@server.process_message({ method: "initialized" })
end
Expand All @@ -388,7 +388,7 @@ def test_shows_error_if_formatter_set_to_rubocop_but_rubocop_not_available
notification = find_message(RubyLsp::Notification, "window/showMessage")

assert_equal(
"Ruby LSP formatter is set to `rubocop` but RuboCop was not found in the Gemfile or gemspec.",
"Ruby LSP formatter is set to `rubocop_internal` but RuboCop was not found in the Gemfile or gemspec.",
T.cast(notification.params, RubyLsp::Interface::ShowMessageParams).message,
)
end
Expand Down
4 changes: 3 additions & 1 deletion vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@
"enum": [
"auto",
"rubocop",
"rubocop_internal",
"syntax_tree",
"standard",
"rubyfmt",
Expand All @@ -381,6 +382,7 @@
"enumDescriptions": [
"Automatically detect formatter",
"RuboCop",
"Ruby LSP RuboCop integration",
"Syntax Tree",
"Standard (supported by community addon)",
"Rubyfmt (supported by community addon)",
Expand All @@ -393,7 +395,7 @@
"type": "array",
"examples": [
[
"rubocop"
"rubocop_internal"
]
],
"default": null
Expand Down

0 comments on commit b6f67d1

Please sign in to comment.