Change the Request type to be a fileprivate RequestAndReply in SourceKitServer

Since asyncifying all the request handling, `Request` only had a few lingering uses. Remove most of them and shrink it down to a type that only contains the request’s parameters and the reply block of the corresponding type.

And while we’re doing this, also move `NotificationType.forLogging` out of the `LanguageServerProtocol` module to remove the dependency from `LanguageServerProtocol` on `LSPLogging`.

Resolves #881
Resolves #936
rdar://116705662
rdar://117562587

Author: ahoppen

Package.swift

@@ -102,7 +102,6 @@ let package = Package(
     .target(
       name: "LanguageServerProtocol",
       dependencies: [
-        "LSPLogging",
         "SKSupport",
         .product(name: "SwiftToolsSupport-auto", package: "swift-tools-support-core"),
       ],

Sources/LSPTestSupport/TestJSONRPCConnection.swift

@@ -87,7 +87,6 @@ public final class TestMessageHandler: MessageHandler {
   }
 
   var oneShotNotificationHandlers: [((Any) -> Void)] = []
-  var oneShotRequestHandlers: [((Any) -> Void)] = []
 
   public var allowUnexpectedNotification: Bool = true
 
@@ -100,15 +99,6 @@ public final class TestMessageHandler: MessageHandler {
     })
   }
 
-  public func appendOneShotRequestHandler<R>(_ handler: @escaping (Request<R>) -> Void) {
-    oneShotRequestHandlers.append({ anyRequest in
-      guard let request = anyRequest as? Request<R> else {
-        fatalError("received request of the wrong type \(anyRequest); expected \(R.self)")
-      }
-      handler(request)
-    })
-  }
-
   public func handle(_ notification: some NotificationType, from clientID: ObjectIdentifier) {
     guard !oneShotNotificationHandlers.isEmpty else {
       if allowUnexpectedNotification { return }
@@ -124,13 +114,7 @@ public final class TestMessageHandler: MessageHandler {
     from clientID: ObjectIdentifier,
     reply: @escaping (LSPResult<R.Response>) -> Void
   ) {
-    let request = Request(params, id: id, clientID: clientID, reply: reply)
-
-    guard !oneShotRequestHandlers.isEmpty else {
-      fatalError("unexpected request \(request)")
-    }
-    let handler = oneShotRequestHandlers.removeFirst()
-    handler(request)
+    reply(.failure(.methodNotFound(R.method)))
   }
 }
 
@@ -172,28 +156,12 @@ public final class TestServer: MessageHandler {
     reply: @escaping (LSPResult<R.Response>) -> Void
   ) {
     if let params = params as? EchoRequest {
-      let req = Request(
-        params,
-        id: id,
-        clientID: clientID,
-        reply: { result in
-          reply(result.map({ $0 as! R.Response }))
-        }
-      )
-      req.reply(req.params.string)
+      reply(.success(params.string as! R.Response))
     } else if let params = params as? EchoError {
-      let req = Request(
-        params,
-        id: id,
-        clientID: clientID,
-        reply: { result in
-          reply(result.map({ $0 as! R.Response }))
-        }
-      )
-      if let code = req.params.code {
-        req.reply(.failure(ResponseError(code: code, message: req.params.message!)))
+      if let code = params.code {
+        reply(.failure(ResponseError(code: code, message: params.message!)))
       } else {
-        req.reply(VoidResponse())
+        reply(.success(VoidResponse() as! R.Response))
       }
     } else {
       fatalError("Unhandled request")

Sources/LanguageServerProtocol/CMakeLists.txt

@@ -7,7 +7,6 @@ add_library(LanguageServerProtocol STATIC
   MessageRegistry.swift
   Messages.swift
   PositionRange.swift
-  Request.swift
   RequestID.swift
 
   Notifications/CancelRequestNotification.swift

Sources/SourceKitLSP/CMakeLists.txt

@@ -3,6 +3,7 @@ add_library(SourceKitLSP STATIC
   CapabilityRegistry.swift
   DocumentManager.swift
   IndexStoreDB+MainFilesProvider.swift
+  LoggableMessageTypes.swift
   ResponseError+Init.swift
   Sequence+AsyncMap.swift
   SourceKitIndexDelegate.swift

Sources/SourceKitLSP/Clang/ClangLanguageServer.swift

@@ -294,26 +294,18 @@ actor ClangLanguageServerShim: ToolchainLanguageServer, MessageHandler {
     reply: @escaping (LSPResult<R.Response>) -> Void
   ) {
     clangdMessageHandlingQueue.async {
-      let request = Request(
-        params,
-        id: id,
-        clientID: clientID,
-        reply: { result in
-          reply(result)
-        }
-      )
       guard let sourceKitServer = await self.sourceKitServer else {
         // `SourceKitServer` has been destructed. We are tearing down the language
         // server. Nothing left to do.
-        request.reply(.failure(.unknown("Connection to the editor closed")))
+        reply(.failure(.unknown("Connection to the editor closed")))
         return
       }
 
       do {
-        let result = try await sourceKitServer.sendRequestToClient(request.params)
-        request.reply(.success(result))
+        let result = try await sourceKitServer.sendRequestToClient(params)
+        reply(.success(result))
       } catch {
-        request.reply(.failure(ResponseError(error)))
+        reply(.failure(ResponseError(error)))
       }
     }
   }

Sources/LanguageServerProtocol/Request.swift renamed to Sources/SourceKitLSP/LoggableMessageTypes.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -12,66 +12,11 @@
 
 import Foundation
 import LSPLogging
-
-/// A request object, wrapping the parameters of a `RequestType` and tracking its state.
-public final class Request<R: RequestType> {
-
-  public typealias Params = R
-
-  public typealias Response = R.Response
-
-  /// The request id from the client.
-  public let id: RequestID
-
-  /// The client of the request.
-  public let clientID: ObjectIdentifier
-
-  /// The request parameters.
-  public let params: Params
-
-  private var replyBlock: (LSPResult<Response>) -> Void
-
-  /// Whether a reply has been made. Every request must reply exactly once.
-  private var replied: Bool = false {
-    willSet {
-      precondition(replied == false, "replied to \(id) more than once")
-    }
-  }
-
-  public init(
-    _ request: Params,
-    id: RequestID,
-    clientID: ObjectIdentifier,
-    reply: @escaping (LSPResult<Response>) -> Void
-  ) {
-    self.id = id
-    self.clientID = clientID
-    self.params = request
-    self.replyBlock = reply
-  }
-
-  deinit {
-    precondition(replied, "request \(id) never received a reply")
-  }
-
-  /// Reply to the request with `result`.
-  ///
-  /// This must be called exactly once for each request.
-  public func reply(_ result: LSPResult<Response>) {
-    replied = true
-    replyBlock(result)
-  }
-
-  /// Reply to the request with `.success(result)`.
-  public func reply(_ result: Response) {
-    reply(.success(result))
-  }
-}
+import LanguageServerProtocol
 
 fileprivate extension Encodable {
   var prettyPrintJSON: String {
     let encoder = JSONEncoder()
-    encoder.outputFormatting.insert(.sortedKeys)
     encoder.outputFormatting.insert(.prettyPrinted)
     guard let data = try? encoder.encode(self) else {
       return "\(self)"
@@ -85,17 +30,24 @@ fileprivate extension Encodable {
   }
 }
 
-extension Request: CustomStringConvertible, CustomLogStringConvertible {
+fileprivate struct AnyRequestType: CustomLogStringConvertible {
+  let request: any RequestType
+
   public var description: String {
     return """
-      \(R.method)
-      \(params.prettyPrintJSON)
+      \(type(of: request).method)
+      \(request.prettyPrintJSON)
       """
   }
 
   public var redactedDescription: String {
-    // FIXME: (logging) Log the non-critical parts of the request
-    return "Request<\(R.method)>"
+    return "\(type(of: request).method)"
+  }
+}
+
+extension RequestType {
+  public var forLogging: CustomLogStringConvertibleWrapper {
+    return AnyRequestType(request: self).forLogging
   }
 }
 
@@ -104,14 +56,13 @@ fileprivate struct AnyNotificationType: CustomLogStringConvertible {
 
   public var description: String {
     return """
-      \(type(of: notification))
+      \(type(of: notification).method)
       \(notification.prettyPrintJSON)
       """
   }
 
   public var redactedDescription: String {
-    // FIXME: (logging) Log the non-critical parts of the notification
-    return "\(type(of: notification))"
+    return "\(type(of: notification).method)"
   }
 }