Parse documents in the worker thread after queued edits are applied

lib/ruby_lsp/base_server.rb

@@ -39,46 +39,13 @@ def start
       @reader.each_message do |message|
         method = message[:method]
 
-        # We must parse the document under a mutex lock or else we might switch threads and accept text edits in the
-        # source. Altering the source reference during parsing will put the parser in an invalid internal state, since
-        # it started parsing with one source but then it changed in the middle. We don't want to do this for text
-        # synchronization notifications
-        @global_state.synchronize do
-          uri = message.dig(:params, :textDocument, :uri)
-
-          if uri
-            begin
-              parsed_uri = URI(uri)
-              message[:params][:textDocument][:uri] = parsed_uri
-
-              # We don't want to try to parse documents on text synchronization notifications
-              unless method.start_with?("textDocument/did")
-                document = @store.get(parsed_uri)
-
-                # If the client supports request delegation and we're working with an ERB document and there was
-                # something to parse, then we have to maintain the client updated about the virtual state of the host
-                # language source
-                if document.parse! && @global_state.client_capabilities.supports_request_delegation &&
-                    document.is_a?(ERBDocument)
-
-                  send_message(
-                    Notification.new(
-                      method: "delegate/textDocument/virtualState",
-                      params: {
-                        textDocument: {
-                          uri: uri,
-                          text: document.host_language_source,
-                        },
-                      },
-                    ),
-                  )
-                end
-              end
-            rescue URI::Error
-              # A client can send a document URI with a scheme Ruby's URI parser rejects, so we don't want to fail
-            rescue Store::NonExistingDocumentError
-              # If we receive a request for a file that no longer exists, we don't want to fail
-            end
+        uri = message.dig(:params, :textDocument, :uri)
+
+        if uri
+          begin
+            message[:params][:textDocument][:uri] = URI(uri)
+          rescue URI::Error
+            # A client can send a document URI with a scheme Ruby's URI parser rejects, so we don't want to fail
           end
         end
 
@@ -178,11 +145,48 @@ def handle_incoming_message(message)
         end
       end
 
+      parse_document(message)
       process_message(message)
       # Ensure we remove the request if it got cancelled while it was being processed
       @cancelled_requests.delete(id)
     end
 
+    # Parse the document targeted by a request right before processing it. This must happen in the worker thread, after
+    # any text synchronization notifications that were queued ahead of the request have been applied. Parsing when the
+    # message is read would use the source from before those edits, and the request would then compute (and cache) its
+    # response from an outdated parse result
+    #: (Hash[Symbol, untyped] message) -> void
+    def parse_document(message)
+      method = message[:method]
+      uri = message.dig(:params, :textDocument, :uri)
+      return if method.nil? || method.start_with?("textDocument/did") || !uri.is_a?(URI::Generic)
+
+      @global_state.synchronize do
+        document = @store.get(uri)
+
+        # If the client supports request delegation and we're working with an ERB document and there was
+        # something to parse, then we have to maintain the client updated about the virtual state of the host
+        # language source
+        if document.parse! && @global_state.client_capabilities.supports_request_delegation &&
+            document.is_a?(ERBDocument)
+
+          send_message(
+            Notification.new(
+              method: "delegate/textDocument/virtualState",
+              params: {
+                textDocument: {
+                  uri: uri.to_s,
+                  text: document.host_language_source,
+                },
+              },
+            ),
+          )
+        end
+      rescue Store::NonExistingDocumentError
+        # If we receive a request for a file that no longer exists, we don't want to fail
+      end
+    end
+
     #: ((Result | Error | Notification | Request) message) -> void
     def send_message(message)
       # When we're shutting down the server, there's a small race condition between closing the thread queues and

test/server_test.rb

@@ -1779,6 +1779,43 @@ def test_does_not_crash_when_a_document_uri_cannot_be_parsed
     server&.run_shutdown
   end
 
+  def test_requests_parse_the_document_after_queued_edits_are_applied
+    # Text synchronization notifications are processed in the worker thread. A request read right after an edit must
+    # not be computed (and cached) from a parse result that predates that edit
+    uri = URI::Generic.from_path(path: File.join(Dir.pwd, "fake.rb")).to_s
+    messages = [
+      {
+        method: "textDocument/didOpen",
+        params: { textDocument: { uri: uri, text: "def foo", version: 1, languageId: "ruby" } },
+      },
+      {
+        method: "textDocument/didChange",
+        params: {
+          textDocument: { uri: uri, version: 2 },
+          contentChanges: [
+            { range: { start: { line: 0, character: 7 }, end: { line: 0, character: 7 } }, text: "\nend" },
+          ],
+        },
+      },
+      { id: 1, method: "textDocument/diagnostic", params: { textDocument: { uri: uri } } },
+    ]
+    input = messages.map do |message|
+      body = message.to_json
+      "Content-Length: #{body.bytesize}\r\n\r\n#{body}"
+    end.join
+
+    @server.run_shutdown
+    @server = RubyLsp::Server.new(test_mode: true, reader: StringIO.new(input), writer: StringIO.new)
+    @server.global_state.stubs(:active_linters).returns([])
+
+    capture_subprocess_io { @server.start }
+
+    result = find_message(RubyLsp::Result, id: 1)
+    items = result.response #: as RubyLsp::Interface::FullDocumentDiagnosticReport
+      .items
+    assert_empty(items.map(&:message))
+  end
+
   private
 
   def run_initialize_server_with_setup_error(error)