From b6f67d11726adb0f6dbbf62b01a4b136b8db79b0 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 15 Jan 2025 14:35:02 -0500 Subject: [PATCH] Change our internal RuboCop integration identifier --- lib/ruby_lsp/global_state.rb | 30 ++++++++- lib/ruby_lsp/server.rb | 11 ++-- test/global_state_test.rb | 65 +++++++++++++++++-- test/requests/code_actions_formatting_test.rb | 4 +- .../requests/diagnostics_expectations_test.rb | 4 +- test/requests/diagnostics_test.rb | 6 +- test/requests/formatting_expectations_test.rb | 4 +- test/server_test.rb | 10 +-- vscode/package.json | 4 +- 9 files changed, 108 insertions(+), 30 deletions(-) diff --git a/lib/ruby_lsp/global_state.rb b/lib/ruby_lsp/global_state.rb index dfe2b0114..64c475af0 100644 --- a/lib/ruby_lsp/global_state.rb +++ b/lib/ruby_lsp/global_state.rb @@ -94,6 +94,7 @@ 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 @@ -101,6 +102,12 @@ def apply_options(options) 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" @@ -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 @@ -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 @@ -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 diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index 458dbd565..92aac17ee 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -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. @@ -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 @@ -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, ), ) diff --git a/test/global_state_test.rb b/test/global_state_test.rb index b655dcbfe..f47c8586d 100644 --- a/test/global_state_test.rb +++ b/test/global_state_test.rb @@ -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 @@ -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 @@ -150,12 +150,13 @@ 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 @@ -163,7 +164,7 @@ def test_linter_auto_detection 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 @@ -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 @@ -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) diff --git a/test/requests/code_actions_formatting_test.rb b/test/requests/code_actions_formatting_test.rb index 3e3d54e1a..03e776934 100644 --- a/test/requests/code_actions_formatting_test.rb +++ b/test/requests/code_actions_formatting_test.rb @@ -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, ) diff --git a/test/requests/diagnostics_expectations_test.rb b/test/requests/diagnostics_expectations_test.rb index 3a43c12c8..0c80b0708 100644 --- a/test/requests/diagnostics_expectations_test.rb +++ b/test/requests/diagnostics_expectations_test.rb @@ -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, ) diff --git a/test/requests/diagnostics_test.rb b/test/requests/diagnostics_test.rb index 86dcef442..a49ed52d0 100644 --- a/test/requests/diagnostics_test.rb +++ b/test/requests/diagnostics_test.rb @@ -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 @@ -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) diff --git a/test/requests/formatting_expectations_test.rb b/test/requests/formatting_expectations_test.rb index 441e414a8..6f17bfeaf 100644 --- a/test/requests/formatting_expectations_test.rb +++ b/test/requests/formatting_expectations_test.rb @@ -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( diff --git a/test/server_test.rb b/test/server_test.rb index 5151faf09..201f886d2 100644 --- a/test/server_test.rb +++ b/test/server_test.rb @@ -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, @@ -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 @@ -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 @@ -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 diff --git a/vscode/package.json b/vscode/package.json index 78a74e386..1d44fd9ff 100644 --- a/vscode/package.json +++ b/vscode/package.json @@ -373,6 +373,7 @@ "enum": [ "auto", "rubocop", + "rubocop_internal", "syntax_tree", "standard", "rubyfmt", @@ -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)", @@ -393,7 +395,7 @@ "type": "array", "examples": [ [ - "rubocop" + "rubocop_internal" ] ], "default": null