Change our internal RuboCop integration identifier (#3067)

### Motivation

This PR addresses point 1 and 2 from #3046

Now that the RuboCop add-on is using the `rubocop` identifier, we need to change ours to prevent conflicts, while still ensuring that users who are on older versions of RuboCop are getting the right behaviour.

### Implementation

Essentially, we are changing our identifier to `rubocop_internal`.

The only extra logic is falling back when users have explicitly configured their formatter or linter as `rubocop`. Since that is now owned by RuboCop, but only present on versions higher than v1.70, we need to check the version and ensure that it's actually available or fallback to our internal integration.

### Automated Tests

Added tests.

lib/ruby_lsp/global_state.rb

@@ -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"
@@ -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 integration 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,
+      # fall back 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

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,
           ),
         )

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,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
@@ -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)

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,
     )
 

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,
     )
 

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)
 

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(

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

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