Set server pipes to binmode (#366)

lib/ruby_lsp/ruby_lsp_rails/definition.rb

@@ -88,7 +88,11 @@ def handle_route(node)
         result = @client.route_location(T.must(node.message))
         return unless result
 
-        file_path, line = result.fetch(:location).split(":")
+        *file_parts, line = result.fetch(:location).split(":")
+
+        # On Windows, file paths will look something like `C:/path/to/file.rb:123`. Only the last colon is the line
+        # number and all other parts compose the file path
+        file_path = file_parts.join(":")
 
         @response_builder << Interface::Location.new(
           uri: URI::Generic.from_path(path: file_path).to_s,

lib/ruby_lsp/ruby_lsp_rails/runner_client.rb

@@ -80,7 +80,9 @@ def initialize
             if @wait_thread.alive?
               $stderr.puts("Ruby LSP Rails is force killing the server")
               sleep(0.5) # give the server a bit of time if we already issued a shutdown notification
-              Process.kill(T.must(Signal.list["TERM"]), @wait_thread.pid)
+
+              # 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
         end

lib/ruby_lsp/ruby_lsp_rails/server.rb

@@ -14,6 +14,8 @@ class Server
       def initialize
         $stdin.sync = true
         $stdout.sync = true
+        $stdin.binmode
+        $stdout.binmode
         @running = true
       end
 

test/ruby_lsp_rails/definition_test.rb

@@ -87,7 +87,10 @@ def baz; end
 
         assert_equal(1, response.size)
         dummy_root = File.expand_path("../dummy", __dir__)
-        assert_equal("file://#{dummy_root}/config/routes.rb", response[0].uri)
+        assert_equal(
+          URI::Generic.from_path(path: File.join(dummy_root, "config", "routes.rb")).to_s,
+          response[0].uri,
+        )
         assert_equal(3, response[0].range.start.line)
         assert_equal(3, response[0].range.end.line)
       end
@@ -107,7 +110,10 @@ def baz; end
 
         assert_equal(1, response.size)
         dummy_root = File.expand_path("../dummy", __dir__)
-        assert_equal("file://#{dummy_root}/config/routes.rb", response[0].uri)
+        assert_equal(
+          URI::Generic.from_path(path: File.join(dummy_root, "config", "routes.rb")).to_s,
+          response[0].uri,
+        )
         assert_equal(4, response[0].range.start.line)
         assert_equal(4, response[0].range.end.line)
       end
@@ -132,7 +138,9 @@ def generate_definitions_for_source(source, position)
             params: { textDocument: { uri: uri }, position: position },
           )
 
-          server.pop_response.response
+          result = server.pop_response
+          assert_instance_of(RubyLsp::Result, result)
+          result.response
         end
       end
     end

test/ruby_lsp_rails/runner_client_test.rb

@@ -4,21 +4,19 @@
 require "test_helper"
 require "ruby_lsp/ruby_lsp_rails/runner_client"
 
-# tests are hanging in CI. https://github.com/Shopify/ruby-lsp-rails/issues/348
-return if Gem.win_platform?
-
 module RubyLsp
   module Rails
     class RunnerClientTest < ActiveSupport::TestCase
       setup do
-        capture_subprocess_io do
-          @client = T.let(RunnerClient.new, RunnerClient)
-        end
+        @client = T.let(RunnerClient.new, RunnerClient)
       end
 
       teardown do
-        capture_subprocess_io { @client.shutdown }
-        assert_predicate @client, :stopped?
+        @client.shutdown
+
+        # On Windows, the server process sometimes takes a lot longer to shutdown and may end up getting force killed,
+        # which makes this assertion flaky
+        assert_predicate(@client, :stopped?) unless Gem.win_platform?
       end
 
       # These are integration tests which start the server. For the more fine-grained tests, see `server_test.rb`.