Move feature configuration to global state (#3613)

Author: vinistock

lib/ruby_lsp/global_state.rb

@@ -56,6 +56,13 @@ def initialize
       @enabled_feature_flags = {} #: Hash[Symbol, bool]
       @mutex = Mutex.new #: Mutex
       @telemetry_machine_id = nil #: String?
+      @feature_configuration = {
+        inlayHint: RequestConfig.new({
+          enableAll: false,
+          implicitRescue: false,
+          implicitHashValue: false,
+        }),
+      } #: Hash[Symbol, RequestConfig]
     end
 
     #: [T] { -> T } -> T
@@ -175,9 +182,19 @@ def apply_options(options)
       @enabled_feature_flags = enabled_flags if enabled_flags
 
       @telemetry_machine_id = options.dig(:initializationOptions, :telemetryMachineId)
+
+      options.dig(:initializationOptions, :featuresConfiguration)&.each do |feature_name, config|
+        @feature_configuration[feature_name]&.merge!(config)
+      end
+
       notifications
     end
 
+    #: (Symbol) -> RequestConfig?
+    def feature_configuration(feature_name)
+      @feature_configuration[feature_name]
+    end
+
     #: (Symbol flag) -> bool?
     def enabled_feature?(flag)
       @enabled_feature_flags[:all] || @enabled_feature_flags[flag]

lib/ruby_lsp/listeners/inlay_hints.rb

@@ -8,10 +8,12 @@ class InlayHints
 
       RESCUE_STRING_LENGTH = "rescue".length #: Integer
 
-      #: (ResponseBuilders::CollectionResponseBuilder[Interface::InlayHint] response_builder, RequestConfig hints_configuration, Prism::Dispatcher dispatcher) -> void
-      def initialize(response_builder, hints_configuration, dispatcher)
+      #: (GlobalState, ResponseBuilders::CollectionResponseBuilder[Interface::InlayHint], Prism::Dispatcher) -> void
+      def initialize(global_state, response_builder, dispatcher)
         @response_builder = response_builder
-        @hints_configuration = hints_configuration
+        @hints_configuration = ( # rubocop:disable Style/RedundantParentheses
+          global_state.feature_configuration(:inlayHint) #: as !nil
+        ) #: RequestConfig
 
         dispatcher.register(self, :on_rescue_node_enter, :on_implicit_node_enter)
       end

lib/ruby_lsp/requests/inlay_hints.rb

@@ -16,13 +16,13 @@ def provider
         end
       end
 
-      #: ((RubyDocument | ERBDocument) document, RequestConfig hints_configuration, Prism::Dispatcher dispatcher) -> void
-      def initialize(document, hints_configuration, dispatcher)
+      #: (GlobalState, (RubyDocument | ERBDocument), Prism::Dispatcher) -> void
+      def initialize(global_state, document, dispatcher)
         super()
 
         @response_builder = ResponseBuilders::CollectionResponseBuilder
           .new #: ResponseBuilders::CollectionResponseBuilder[Interface::InlayHint]
-        Listeners::InlayHints.new(@response_builder, hints_configuration, dispatcher)
+        Listeners::InlayHints.new(global_state, @response_builder, dispatcher)
       end
 
       # @override

lib/ruby_lsp/server.rb

@@ -209,10 +209,6 @@ def run_initialize(message)
 
       configured_features = options.dig(:initializationOptions, :enabledFeatures)
 
-      configured_hints = options.dig(:initializationOptions, :featuresConfiguration, :inlayHint)
-      @store.features_configuration.dig(:inlayHint) #: as !nil
-        .configuration.merge!(configured_hints) if configured_hints
-
       enabled_features = case configured_features
       when Array
         # If the configuration is using an array, then absent features are disabled and present ones are enabled. That's
@@ -479,8 +475,8 @@ def run_combined_requests(message)
       document_symbol = Requests::DocumentSymbol.new(uri, dispatcher)
       document_link = Requests::DocumentLink.new(uri, parse_result.comments, dispatcher)
       inlay_hint = Requests::InlayHints.new(
+        @global_state,
         document,
-        @store.features_configuration.dig(:inlayHint), #: as !nil
         dispatcher,
       )
 
@@ -827,15 +823,14 @@ def text_document_inlay_hint(message)
         return
       end
 
-      hints_configurations = @store.features_configuration.dig(:inlayHint) #: as !nil
       dispatcher = Prism::Dispatcher.new
 
       unless document.is_a?(RubyDocument) || document.is_a?(ERBDocument)
         send_empty_response(message[:id])
         return
       end
 
-      request = Requests::InlayHints.new(document, hints_configurations, dispatcher)
+      request = Requests::InlayHints.new(@global_state, document, dispatcher)
       dispatcher.visit(document.ast)
       result = request.perform
       document.cache_set("textDocument/inlayHint", result)

lib/ruby_lsp/store.rb

@@ -5,23 +5,13 @@ module RubyLsp
   class Store
     class NonExistingDocumentError < StandardError; end
 
-    #: Hash[Symbol, RequestConfig]
-    attr_accessor :features_configuration
-
     #: String
     attr_accessor :client_name
 
     #: (GlobalState global_state) -> void
     def initialize(global_state)
       @global_state = global_state
       @state = {} #: Hash[String, Document[untyped]]
-      @features_configuration = {
-        inlayHint: RequestConfig.new({
-          enableAll: false,
-          implicitRescue: false,
-          implicitHashValue: false,
-        }),
-      } #: Hash[Symbol, RequestConfig]
       @client_name = "Unknown" #: String
     end
 

lib/ruby_lsp/utils.rb

@@ -247,9 +247,6 @@ def to_hash
 
   # A request configuration, to turn on/off features
   class RequestConfig
-    #: Hash[Symbol, bool]
-    attr_accessor :configuration
-
     #: (Hash[Symbol, bool] configuration) -> void
     def initialize(configuration)
       @configuration = configuration
@@ -259,6 +256,11 @@ def initialize(configuration)
     def enabled?(feature)
       @configuration[:enableAll] || @configuration[feature]
     end
+
+    #: (Hash[Symbol, bool]) -> void
+    def merge!(hash)
+      @configuration.merge!(hash)
+    end
   end
 
   class SorbetLevel

test/global_state_test.rb

@@ -314,6 +314,67 @@ def test_saves_telemetry_machine_id
       assert_equal("test_machine_id", state.telemetry_machine_id)
     end
 
+    def test_default_feature_configuration
+      state = GlobalState.new
+
+      inlay_hint_config = state.feature_configuration(:inlayHint) #: as !nil
+      refute(inlay_hint_config.enabled?(:implicitRescue))
+      refute(inlay_hint_config.enabled?(:implicitHashValue))
+    end
+
+    def test_feature_configuration_with_provided_configuration
+      state = GlobalState.new
+      state.apply_options({
+        initializationOptions: {
+          featuresConfiguration: {
+            inlayHint: {
+              implicitRescue: true,
+              implicitHashValue: true,
+            },
+          },
+        },
+      })
+
+      inlay_hint_config = state.feature_configuration(:inlayHint) #: as !nil
+      assert(inlay_hint_config.enabled?(:implicitRescue))
+      assert(inlay_hint_config.enabled?(:implicitHashValue))
+    end
+
+    def test_feature_configuration_with_partially_provided_configuration
+      state = GlobalState.new
+      state.apply_options({
+        initializationOptions: {
+          featuresConfiguration: {
+            inlayHint: {
+              implicitHashValue: true,
+            },
+          },
+        },
+      })
+
+      inlay_hint_config = state.feature_configuration(:inlayHint) #: as !nil
+      refute(inlay_hint_config.enabled?(:implicitRescue))
+      assert(inlay_hint_config.enabled?(:implicitHashValue))
+    end
+
+    def test_initialize_features_with_enable_all_configuration
+      state = GlobalState.new
+      state.apply_options({
+        initializationOptions: {
+          featuresConfiguration: {
+            inlayHint: {
+              enableAll: true,
+
+            },
+          },
+        },
+      })
+
+      inlay_hint_config = state.feature_configuration(:inlayHint) #: as !nil
+      assert(inlay_hint_config.enabled?(:implicitRescue))
+      assert(inlay_hint_config.enabled?(:implicitHashValue))
+    end
+
     private
 
     def stub_direct_dependencies(dependencies)

test/requests/inlay_hints_expectations_test.rb

@@ -13,8 +13,14 @@ def run_expectations(source)
     document = RubyLsp::RubyDocument.new(source: source, version: 1, uri: uri, global_state: @global_state)
 
     dispatcher = Prism::Dispatcher.new
-    hints_configuration = RubyLsp::RequestConfig.new({ implicitRescue: true, implicitHashValue: true })
-    request = RubyLsp::Requests::InlayHints.new(document, hints_configuration, dispatcher)
+    @global_state.apply_options({
+      initializationOptions: {
+        featuresConfiguration: {
+          inlayHint: { implicitRescue: true, implicitHashValue: true },
+        },
+      },
+    })
+    request = RubyLsp::Requests::InlayHints.new(@global_state, document, dispatcher)
     dispatcher.dispatch(document.ast)
     range = params.first
     ruby_range = range.dig(:start, :line)..range.dig(:end, :line)
@@ -35,8 +41,14 @@ def test_skip_implicit_hash_value
     RUBY
 
     dispatcher = Prism::Dispatcher.new
-    hints_configuration = RubyLsp::RequestConfig.new({ implicitRescue: true, implicitHashValue: false })
-    request = RubyLsp::Requests::InlayHints.new(document, hints_configuration, dispatcher)
+    @global_state.apply_options({
+      initializationOptions: {
+        featuresConfiguration: {
+          inlayHint: { implicitRescue: true, implicitHashValue: false },
+        },
+      },
+    })
+    request = RubyLsp::Requests::InlayHints.new(@global_state, document, dispatcher)
     dispatcher.dispatch(document.ast)
     assert_empty(request.perform)
   end
@@ -50,8 +62,14 @@ def test_skip_implicit_rescue
     RUBY
 
     dispatcher = Prism::Dispatcher.new
-    hints_configuration = RubyLsp::RequestConfig.new({ implicitRescue: false, implicitHashValue: true })
-    request = RubyLsp::Requests::InlayHints.new(document, hints_configuration, dispatcher)
+    @global_state.apply_options({
+      initializationOptions: {
+        featuresConfiguration: {
+          inlayHint: { implicitRescue: false, implicitHashValue: true },
+        },
+      },
+    })
+    request = RubyLsp::Requests::InlayHints.new(@global_state, document, dispatcher)
     dispatcher.dispatch(document.ast)
     assert_empty(request.perform)
   end

test/server_test.rb

@@ -294,73 +294,6 @@ def test_did_close_clears_diagnostics
     )
   end
 
-  def test_initialize_features_with_default_configuration
-    capture_subprocess_io do
-      @server.process_message({ method: "initialized" })
-    end
-    store = @server.instance_variable_get(:@store)
-
-    refute(store.features_configuration.dig(:inlayHint).enabled?(:implicitRescue))
-    refute(store.features_configuration.dig(:inlayHint).enabled?(:implicitHashValue))
-  end
-
-  def test_initialize_features_with_provided_configuration
-    capture_subprocess_io do
-      @server.process_message(id: 1, method: "initialize", params: {
-        initializationOptions: {
-          featuresConfiguration: {
-            inlayHint: {
-              implicitRescue: true,
-              implicitHashValue: true,
-            },
-          },
-        },
-      })
-    end
-
-    store = @server.instance_variable_get(:@store)
-    assert(store.features_configuration.dig(:inlayHint).enabled?(:implicitRescue))
-    assert(store.features_configuration.dig(:inlayHint).enabled?(:implicitHashValue))
-  end
-
-  def test_initialize_features_with_partially_provided_configuration
-    capture_subprocess_io do
-      @server.process_message(id: 1, method: "initialize", params: {
-        initializationOptions: {
-          featuresConfiguration: {
-            inlayHint: {
-              implicitHashValue: true,
-            },
-          },
-        },
-      })
-    end
-
-    store = @server.instance_variable_get(:@store)
-
-    refute(store.features_configuration.dig(:inlayHint).enabled?(:implicitRescue))
-    assert(store.features_configuration.dig(:inlayHint).enabled?(:implicitHashValue))
-  end
-
-  def test_initialize_features_with_enable_all_configuration
-    capture_subprocess_io do
-      @server.process_message(id: 1, method: "initialize", params: {
-        initializationOptions: {
-          featuresConfiguration: {
-            inlayHint: {
-              enableAll: true,
-            },
-          },
-        },
-      })
-    end
-
-    store = @server.instance_variable_get(:@store)
-
-    assert(store.features_configuration.dig(:inlayHint).enabled?(:implicitRescue))
-    assert(store.features_configuration.dig(:inlayHint).enabled?(:implicitHashValue))
-  end
-
   def test_handles_invalid_configuration
     File.write(".index.yml", "} invalid yaml")