Merge pull request #256 from Shopify/andyw8/implement-lsp-runner

Switch to `rails runner` approach instead of Rack app

Author: andyw8

README.md

@@ -52,16 +52,15 @@ See the [documentation](https://shopify.github.io/ruby-lsp-rails) for more in-de
 
 ## How It Works
 
-This gem consists of two components that enable enhanced Rails functionality in the editor:
+When Ruby LSP Rails starts, it spawns a `rails runner` instance which runs
+`[server.rb](https://github.com/Shopify/ruby-lsp-rails/blob/main/lib/ruby_lsp/ruby_lsp_rails/server.rb)`.
+The addon communicates with this process over a pipe (i.e. `stdin` and `stdout`) to fetch runtime information about the application.
 
-1. A Rack app that automatically exposes APIs when Rails server is running
-1. A Ruby LSP addon that connects to the exposed APIs to fetch runtime information from the Rails server
-
-This is why the Rails server needs to be running for some features to work.
+When extension is stopped (e.g. by quitting the editor), the server instance is shut down.
 
 > [!NOTE]
-> There is no need to restart the Ruby LSP every time the Rails server is booted.
-> If the server is shut down, the extra features will temporarily disappear and reappear once the server is running again.
+> Prior to v0.3, `ruby-lsp-rails` used a different approach which involved mounting a Rack application within the Rails app.
+> That approach was brittle and susceptible to the application's configuration, such as routing and middleware.
 
 ## Contributing
 

lib/ruby-lsp-rails.rb

@@ -1,9 +1,6 @@
 # typed: strict
 # frozen_string_literal: true
 
-require "sorbet-runtime"
-require "pathname"
-
 require "ruby_lsp_rails/version"
 require "ruby_lsp_rails/railtie"
 

lib/ruby_lsp/ruby_lsp_rails/addon.rb

@@ -3,7 +3,7 @@
 
 require "ruby_lsp/addon"
 
-require_relative "rails_client"
+require_relative "runner_client"
 require_relative "hover"
 require_relative "code_lens"
 
@@ -12,18 +12,18 @@ module Rails
     class Addon < ::RubyLsp::Addon
       extend T::Sig
 
-      sig { returns(RailsClient) }
+      sig { returns(RunnerClient) }
       def client
-        @client ||= T.let(RailsClient.new, T.nilable(RailsClient))
+        @client ||= T.let(RunnerClient.new, T.nilable(RunnerClient))
       end
 
       sig { override.params(message_queue: Thread::Queue).void }
-      def activate(message_queue)
-        client.check_if_server_is_running!
-      end
+      def activate(message_queue); end
 
       sig { override.void }
-      def deactivate; end
+      def deactivate
+        client.shutdown
+      end
 
       # Creates a new CodeLens listener. This method is invoked on every CodeLens request
       sig do

lib/ruby_lsp/ruby_lsp_rails/hover.rb

@@ -22,7 +22,7 @@ class Hover
 
       sig do
         params(
-          client: RailsClient,
+          client: RunnerClient,
           response_builder: ResponseBuilders::Hover,
           nesting: T::Array[String],
           index: RubyIndexer::Index,

lib/ruby_lsp/ruby_lsp_rails/rails_client.rb

@@ -2,34 +2,17 @@
 # frozen_string_literal: true
 
 require "rails"
-require "ruby_lsp_rails/rack_app"
 
 module RubyLsp
   module Rails
     class Railtie < ::Rails::Railtie
       config.ruby_lsp_rails = ActiveSupport::OrderedOptions.new
-      config.ruby_lsp_rails.server = true
 
       initializer "ruby_lsp_rails.setup" do |_app|
-        config.after_initialize do |app|
-          # If we start the app with `bin/rails console` then `Rails::Server` is not defined.
-          if defined?(::Rails::Server) && config.ruby_lsp_rails.server
-            app.routes.prepend do
-              T.bind(self, ActionDispatch::Routing::Mapper)
-              mount(RackApp.new => RackApp::BASE_PATH)
-            end
-
-            ssl_enable, host, port = ::Rails::Server::Options.new.parse!(ARGV).values_at(:SSLEnable, :Host, :Port)
-            app_uri = "#{ssl_enable ? "https" : "http"}://#{host}:#{port}"
-            app_uri_path = ::Rails.root.join("tmp", "app_uri.txt")
-            app_uri_path.write(app_uri)
-
-            at_exit do
-              # The app_uri.txt file should only exist when the server is running. The addon uses its presence to
-              # report if the server is running or not. If the server is not running, some of the addon features
-              # will not be available.
-              File.delete(app_uri_path) if File.exist?(app_uri_path)
-            end
+        config.after_initialize do |_app|
+          unless config.ruby_lsp_rails.server.nil?
+            ActiveSupport::Deprecation.new.warn("The `ruby_lsp_rails.server` configuration option is no longer " \
+              "needed and will be removed in a future release.")
           end
         end
       end

lib/ruby_lsp_rails/rack_app.rb

@@ -10,19 +10,6 @@ class AddonTest < ActiveSupport::TestCase
         addon = Addon.new
         assert_equal("Ruby LSP Rails", addon.name)
       end
-
-      test "activate checks if Rails server is running" do
-        rails_client = stub("rails_client", check_if_server_is_running!: true)
-
-        RubyLsp::Rails::RailsClient.stubs(instance: rails_client)
-
-        addon = Addon.new
-        queue = Thread::Queue.new
-
-        capture_io { assert(addon.activate(queue)) }
-      ensure
-        T.must(queue).close
-      end
     end
   end
 end

lib/ruby_lsp_rails/railtie.rb

@@ -7,8 +7,6 @@ module RubyLsp
   module Rails
     class HoverTest < ActiveSupport::TestCase
       setup do
-        File.write("#{Dir.pwd}/test/dummy/tmp/app_uri.txt", "http://localhost:3000")
-        @client = RailsClient.new
         @message_queue = Thread::Queue.new
 
         # Build the Rails documents index ahead of time
@@ -23,7 +21,7 @@ class HoverTest < ActiveSupport::TestCase
 
       test "hook returns model column information" do
         expected_response = {
-          schema_file: "#{@client.root}/db/schema.rb",
+          schema_file: "#{dummy_root}/db/schema.rb",
           columns: [
             ["id", "integer"],
             ["first_name", "string"],
@@ -34,8 +32,7 @@ class HoverTest < ActiveSupport::TestCase
           ],
         }
 
-        stub_http_request("200", expected_response.to_json)
-        @client.stubs(:check_if_server_is_running!)
+        RunnerClient.any_instance.stubs(model: expected_response)
 
         response = hover_on_source(<<~RUBY, { line: 3, character: 0 })
           class User < ApplicationRecord
@@ -50,7 +47,7 @@ class User < ApplicationRecord
           ```
 
           **Definitions**: [fake.rb](file:///fake.rb#L1,1-2,4)
-          [Schema](file://#{@client.root}/db/schema.rb)
+          [Schema](file://#{dummy_root}/db/schema.rb)
 
 
           **id**: integer
@@ -69,7 +66,7 @@ class User < ApplicationRecord
 
       test "return column information for namespaced models" do
         expected_response = {
-          schema_file: "#{@client.root}/db/schema.rb",
+          schema_file: "#{dummy_root}/db/schema.rb",
           columns: [
             ["id", "integer"],
             ["first_name", "string"],
@@ -80,8 +77,7 @@ class User < ApplicationRecord
           ],
         }
 
-        stub_http_request("200", expected_response.to_json)
-        @client.stubs(:check_if_server_is_running!)
+        RunnerClient.any_instance.stubs(model: expected_response)
 
         response = hover_on_source(<<~RUBY, { line: 4, character: 6 })
           module Blog
@@ -93,7 +89,7 @@ class User < ApplicationRecord
         RUBY
 
         assert_equal(<<~CONTENT.chomp, response.contents.value)
-          [Schema](file://#{@client.root}/db/schema.rb)
+          [Schema](file://#{dummy_root}/db/schema.rb)
 
           **id**: integer
 
@@ -111,12 +107,11 @@ class User < ApplicationRecord
 
       test "handles `db/structure.sql` instead of `db/schema.rb`" do
         expected_response = {
-          schema_file: "#{@client.root}/db/structure.sql",
+          schema_file: "#{dummy_root}/db/structure.sql",
           columns: [],
         }
 
-        stub_http_request("200", expected_response.to_json)
-        @client.stubs(:check_if_server_is_running!)
+        RunnerClient.any_instance.stubs(model: expected_response)
 
         response = hover_on_source(<<~RUBY, { line: 3, character: 0 })
           class User < ApplicationRecord
@@ -127,7 +122,7 @@ class User < ApplicationRecord
 
         assert_includes(
           response.contents.value,
-          "[Schema](file://#{@client.root}/db/structure.sql)",
+          "[Schema](file://#{dummy_root}/db/structure.sql)",
         )
       end
 
@@ -137,8 +132,7 @@ class User < ApplicationRecord
           columns: [],
         }
 
-        stub_http_request("200", expected_response.to_json)
-        @client.stubs(:check_if_server_is_running!)
+        RunnerClient.any_instance.stubs(model: expected_response)
 
         response = hover_on_source(<<~RUBY, { line: 3, character: 0 })
           class User < ApplicationRecord
@@ -213,6 +207,10 @@ def hover_on_source(source, position)
         assert_nil(response.error)
         response.response
       end
+
+      def dummy_root
+        File.expand_path("#{__dir__}/../../test/dummy")
+      end
     end
   end
 end