Extract RunnerClient interface

amomchilov · amomchilov · commit e3df69ff01dc · 2024-07-31T12:26:49.000-04:00
diff --git a/lib/ruby_lsp/ruby_lsp_rails/addon.rb b/lib/ruby_lsp/ruby_lsp_rails/addon.rb
@@ -25,15 +25,15 @@ def initialize
 
         # We first initialize the client as a NullClient, so that we can start the server in a background thread. Until
         # the real client is initialized, features that depend on it will not be blocked by using the NullClient
-        @client = T.let(NullClient.new, RunnerClient)
+        @client = T.let(NullClient.new, Client)
       end
 
       sig { override.params(global_state: GlobalState, message_queue: Thread::Queue).void }
       def activate(global_state, message_queue)
         @global_state = T.let(global_state, T.nilable(RubyLsp::GlobalState))
         $stderr.puts("Activating Ruby LSP Rails addon v#{VERSION}")
         # Start booting the real client in a background thread. Until this completes, the client will be a NullClient
-        Thread.new { @client = RunnerClient.create_client }
+        Thread.new { @client = Client.create_client }
         register_additional_file_watchers(global_state: global_state, message_queue: message_queue)
       end
 
diff --git a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb
@@ -78,7 +78,7 @@ class CodeLens
 
       sig do
         params(
-          client: RunnerClient,
+          client: Client,
           global_state: GlobalState,
           response_builder:  ResponseBuilders::CollectionResponseBuilder[Interface::CodeLens],
           uri: URI::Generic,
diff --git a/lib/ruby_lsp/ruby_lsp_rails/definition.rb b/lib/ruby_lsp/ruby_lsp_rails/definition.rb
@@ -33,7 +33,7 @@ class Definition
 
       sig do
         params(
-          client: RunnerClient,
+          client: Client,
           response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::Location],
           node_context: NodeContext,
           index: RubyIndexer::Index,
diff --git a/lib/ruby_lsp/ruby_lsp_rails/hover.rb b/lib/ruby_lsp/ruby_lsp_rails/hover.rb
@@ -22,7 +22,7 @@ class Hover
 
       sig do
         params(
-          client: RunnerClient,
+          client: Client,
           response_builder: ResponseBuilders::Hover,
           node_context: NodeContext,
           global_state: GlobalState,
diff --git a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb b/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb
@@ -6,14 +6,19 @@
 
 module RubyLsp
   module Rails
-    class RunnerClient
+    module Client
+      extend T::Sig
+      extend T::Helpers
+
+      abstract!
+
       class << self
         extend T::Sig
 
-        sig { returns(RunnerClient) }
+        sig { returns(Client) }
         def create_client
           if File.exist?("bin/rails")
-            new
+            RunnerClient.new
           else
             $stderr.puts(<<~MSG)
               Ruby LSP Rails failed to locate bin/rails in the current directory: #{Dir.pwd}"
@@ -32,6 +37,94 @@ class InitializationError < StandardError; end
       class IncompleteMessageError < StandardError; end
       class EmptyMessageError < StandardError; end
 
+      sig { overridable.params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
+      def model(name)
+        make_request("model", name: name)
+      rescue IncompleteMessageError
+        $stderr.puts("Ruby LSP Rails failed to get model information: #{read_error}")
+        nil
+      end
+
+      sig { overridable.params(controller: String, action: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
+      def route(controller:, action:)
+        make_request("route_info", controller: controller, action: action)
+      rescue IncompleteMessageError
+        $stderr.puts("Ruby LSP Rails failed to get route information: #{read_error}")
+        nil
+      end
+
+      sig { overridable.void }
+      def trigger_reload
+        $stderr.puts("Reloading Rails application")
+        send_notification("reload")
+      rescue IncompleteMessageError
+        $stderr.puts("Ruby LSP Rails failed to trigger reload")
+        nil
+      end
+
+      sig do
+        params(
+          model_name: String,
+          association_name: String,
+          ).returns(T.nilable(T::Hash[Symbol, T.untyped]))
+      end
+      def association_target_location(model_name:, association_name:)
+        make_request(
+          "association_target_location",
+          model_name: model_name,
+          association_name: association_name,
+          )
+      rescue => e
+        $stderr.puts("Ruby LSP Rails failed with #{e.message}: #{read_error}")
+      end
+
+      sig { overridable.params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
+      def route_location(name)
+        make_request("route_location", name: name)
+      rescue IncompleteMessageError
+        $stderr.puts("Ruby LSP Rails failed to get route location: #{read_error}")
+        nil
+      end
+
+      sig { abstract.void }
+      def shutdown; end
+
+      sig { abstract.returns(T::Boolean) }
+      def stopped?; end
+
+      sig { abstract.returns(String) }
+      def rails_root; end
+
+      private
+
+      sig do
+        overridable.params(
+          request: String,
+          params: T.nilable(T::Hash[Symbol, T.untyped]),
+        ).returns(T.nilable(T::Hash[Symbol, T.untyped]))
+      end
+      def make_request(request, params = nil)
+        send_message(request, params)
+        read_response
+      end
+
+      sig { abstract.params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
+      def send_message(request, params = nil); end
+
+      # Notifications are like messages, but one-way, with no response sent back.
+      sig { params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
+      def send_notification(request, params = nil) = send_message(request, params)
+
+      sig { abstract.returns(T.nilable(T::Hash[Symbol, T.untyped])) }
+      def read_response; end
+
+      sig { abstract.returns(String) }
+      def read_error; end
+    end
+
+    class RunnerClient
+      include Client
+
       MAX_RETRIES = 5
 
       extend T::Sig
@@ -89,59 +182,10 @@ def initialize
           end
         end
       rescue Errno::EPIPE, IncompleteMessageError
-        raise InitializationError, @stderr.read
+        raise InitializationError, read_error
       end
 
-      sig { params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
-      def model(name)
-        make_request("model", name: name)
-      rescue IncompleteMessageError
-        $stderr.puts("Ruby LSP Rails failed to get model information: #{@stderr.read}")
-        nil
-      end
-
-      sig do
-        params(
-          model_name: String,
-          association_name: String,
-        ).returns(T.nilable(T::Hash[Symbol, T.untyped]))
-      end
-      def association_target_location(model_name:, association_name:)
-        make_request(
-          "association_target_location",
-          model_name: model_name,
-          association_name: association_name,
-        )
-      rescue => e
-        $stderr.puts("Ruby LSP Rails failed with #{e.message}: #{@stderr.read}")
-      end
-
-      sig { params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
-      def route_location(name)
-        make_request("route_location", name: name)
-      rescue IncompleteMessageError
-        $stderr.puts("Ruby LSP Rails failed to get route location: #{@stderr.read}")
-        nil
-      end
-
-      sig { params(controller: String, action: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
-      def route(controller:, action:)
-        make_request("route_info", controller: controller, action: action)
-      rescue IncompleteMessageError
-        $stderr.puts("Ruby LSP Rails failed to get route information: #{@stderr.read}")
-        nil
-      end
-
-      sig { void }
-      def trigger_reload
-        $stderr.puts("Reloading Rails application")
-        send_notification("reload")
-      rescue IncompleteMessageError
-        $stderr.puts("Ruby LSP Rails failed to trigger reload")
-        nil
-      end
-
-      sig { void }
+      sig { override.void }
       def shutdown
         $stderr.puts("Ruby LSP Rails shutting down server")
         send_message("shutdown")
@@ -152,25 +196,14 @@ def shutdown
         force_kill
       end
 
-      sig { returns(T::Boolean) }
+      sig { override.returns(T::Boolean) }
       def stopped?
         [@stdin, @stdout, @stderr].all?(&:closed?) && !@wait_thread.alive?
       end
 
       private
 
-      sig do
-        params(
-          request: String,
-          params: T.nilable(T::Hash[Symbol, T.untyped]),
-        ).returns(T.nilable(T::Hash[Symbol, T.untyped]))
-      end
-      def make_request(request, params = nil)
-        send_message(request, params)
-        read_response
-      end
-
-      sig { overridable.params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
+      sig { override.params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
       def send_message(request, params = nil)
         message = { method: request }
         message[:params] = params if params
@@ -181,11 +214,7 @@ def send_message(request, params = nil)
         # The server connection died
       end
 
-      # Notifications are like messages, but one-way, with no response sent back.
-      sig { params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
-      def send_notification(request, params = nil) = send_message(request, params)
-
-      sig { overridable.returns(T.nilable(T::Hash[Symbol, T.untyped])) }
+      sig { override.returns(T.nilable(T::Hash[Symbol, T.untyped])) }
       def read_response
         headers = @stdout.gets("\r\n\r\n")
         raise IncompleteMessageError unless headers
@@ -207,19 +236,20 @@ def read_response
         nil
       end
 
+      sig { override.returns(String) }
+      def read_error = @stderr.read
+
       sig { void }
       def force_kill
         # Windows does not support the `TERM` signal, so we're forced to use `KILL` here
         Process.kill(T.must(Signal.list["KILL"]), @wait_thread.pid)
       end
     end
 
-    class NullClient < RunnerClient
+    class NullClient
       extend T::Sig
 
-      sig { void }
-      def initialize # rubocop:disable Lint/MissingSuper
-      end
+      include Client
 
       sig { override.void }
       def shutdown
@@ -247,6 +277,9 @@ def send_message(request, params = nil)
       def read_response
         # no-op
       end
+
+      sig { override.returns(String) }
+      def read_error = "<no error information available>"
     end
   end
 end
diff --git a/test/ruby_lsp_rails/runner_client_test.rb b/test/ruby_lsp_rails/runner_client_test.rb
@@ -45,7 +45,7 @@ class RunnerClientTest < ActiveSupport::TestCase
         FileUtils.mv("bin/rails", "bin/rails_backup")
 
         assert_output("", %r{Ruby LSP Rails failed to locate bin/rails in the current directory}) do
-          client = RunnerClient.create_client
+          client = Client.create_client
 
           assert_instance_of(NullClient, client)
           assert_nil(client.model("User"))
@@ -61,7 +61,7 @@ class RunnerClientTest < ActiveSupport::TestCase
           "",
           /Ruby LSP Rails failed to initialize server/,
         ) do
-          client = RunnerClient.create_client
+          client = Client.create_client
 
           assert_instance_of(NullClient, client)
           assert_nil(client.model("User"))
@@ -78,7 +78,7 @@ class RunnerClientTest < ActiveSupport::TestCase
         File.write("test/dummy/config/application.rb", content + junk)
 
         capture_subprocess_io do
-          client = RunnerClient.create_client
+          client = Client.create_client
 
           response = T.must(client.model("User"))
           assert(response.key?(:columns))
