Naming improvements, added comments and typo fixes in connection related code

This should make the code easier to understand. No functionality change.

Author: ahoppen

Sources/LSPTestSupport/TestJSONRPCConnection.swift

@@ -20,55 +20,68 @@ import class Foundation.Pipe
 public final class TestJSONRPCConnection {
   public let clientToServer: Pipe = Pipe()
   public let serverToClient: Pipe = Pipe()
-  public let client: TestMessageHandler
-  public let clientConnection: JSONRPCConnection
+
+  /// Mocks a client (aka. editor) that can send requests to the LSP server.
+  public let client: TestClient
+
+  /// The connection with which the client can send requests and notifications to the LSP server and using which it
+  /// receives replies to the requests.
+  public let clientToServerConnection: JSONRPCConnection
+
+  /// Mocks an LSP server that handles requests from the client.
   public let server: TestServer
-  public let serverConnection: JSONRPCConnection
+
+  /// The connection with which the server can send requests and notifications to the client and using which it
+  /// receives replies to the requests.
+  public let serverToClientConnection: JSONRPCConnection
 
   public init(allowUnexpectedNotification: Bool = true) {
-    clientConnection = JSONRPCConnection(
+    clientToServerConnection = JSONRPCConnection(
       name: "client",
       protocol: testMessageRegistry,
       inFD: serverToClient.fileHandleForReading,
       outFD: clientToServer.fileHandleForWriting
     )
 
-    serverConnection = JSONRPCConnection(
+    serverToClientConnection = JSONRPCConnection(
       name: "server",
       protocol: testMessageRegistry,
       inFD: clientToServer.fileHandleForReading,
       outFD: serverToClient.fileHandleForWriting
     )
 
-    client = TestMessageHandler(server: clientConnection, allowUnexpectedNotification: allowUnexpectedNotification)
-    server = TestServer(client: serverConnection)
+    client = TestClient(
+      connectionToServer: clientToServerConnection,
+      allowUnexpectedNotification: allowUnexpectedNotification
+    )
+    server = TestServer(client: serverToClientConnection)
 
-    clientConnection.start(receiveHandler: client) {
+    clientToServerConnection.start(receiveHandler: client) {
       // FIXME: keep the pipes alive until we close the connection. This
       // should be fixed systemically.
       withExtendedLifetime(self) {}
     }
-    serverConnection.start(receiveHandler: server) {
+    serverToClientConnection.start(receiveHandler: server) {
       // FIXME: keep the pipes alive until we close the connection. This
       // should be fixed systemically.
       withExtendedLifetime(self) {}
     }
   }
 
   public func close() {
-    clientConnection.close()
-    serverConnection.close()
+    clientToServerConnection.close()
+    serverToClientConnection.close()
   }
 }
 
 public struct TestLocalConnection {
-  public let client: TestMessageHandler
-  public let clientConnection: LocalConnection = .init()
+  public let client: TestClient
+  public let clientConnection: LocalConnection = LocalConnection()
   public let server: TestServer
-  public let serverConnection: LocalConnection = .init()
+  public let serverConnection: LocalConnection = LocalConnection()
 
   public init(allowUnexpectedNotification: Bool = true) {
-    client = TestMessageHandler(server: serverConnection, allowUnexpectedNotification: allowUnexpectedNotification)
+    client = TestClient(connectionToServer: serverConnection, allowUnexpectedNotification: allowUnexpectedNotification)
     server = TestServer(client: clientConnection)
 
     clientConnection.start(handler: client)
@@ -81,18 +94,18 @@ public struct TestLocalConnection {
   }
 }
 
-public actor TestMessageHandler: MessageHandler {
-  /// The connection to the language client.
-  public let server: Connection
+public actor TestClient: MessageHandler {
+  /// The connection to the LSP server.
+  public let connectionToServer: Connection
 
   private let messageHandlingQueue = AsyncQueue<Serial>()
 
   private var oneShotNotificationHandlers: [((Any) -> Void)] = []
 
   private let allowUnexpectedNotification: Bool
 
-  public init(server: Connection, allowUnexpectedNotification: Bool = true) {
-    self.server = server
+  public init(connectionToServer: Connection, allowUnexpectedNotification: Bool = true) {
+    self.connectionToServer = connectionToServer
     self.allowUnexpectedNotification = allowUnexpectedNotification
   }
 
@@ -129,20 +142,18 @@ public actor TestMessageHandler: MessageHandler {
   ) {
     reply(.failure(.methodNotFound(Request.method)))
   }
-}
 
-extension TestMessageHandler: Connection {
   /// Send a notification to the LSP server.
   public nonisolated func send(_ notification: some NotificationType) {
-    server.send(notification)
+    connectionToServer.send(notification)
   }
 
   /// Send a request to the LSP server and (asynchronously) receive a reply.
   public nonisolated func send<Request: RequestType>(
     _ request: Request,
     reply: @escaping (LSPResult<Request.Response>) -> Void
   ) -> RequestID {
-    return server.send(request, reply: reply)
+    return connectionToServer.send(request, reply: reply)
   }
 }
 
@@ -153,34 +164,36 @@ public final class TestServer: MessageHandler {
     self.client = client
   }
 
-  public func handle(_ params: some NotificationType) {
-    if params is EchoNotification {
-      self.client.send(params)
+  /// The client sent a notification to the server. Handle it.
+  public func handle(_ notification: some NotificationType) {
+    if notification is EchoNotification {
+      self.client.send(notification)
     } else {
       fatalError("Unhandled notification")
     }
   }
 
-  public func handle<R: RequestType>(
-    _ params: R,
+  /// The client sent a request to the server. Handle it.
+  public func handle<Request: RequestType>(
+    _ request: Request,
     id: RequestID,
-    reply: @escaping (LSPResult<R.Response>) -> Void
+    reply: @escaping (LSPResult<Request.Response>) -> Void
   ) {
-    if let params = params as? EchoRequest {
-      reply(.success(params.string as! R.Response))
-    } else if let params = params as? EchoError {
+    if let params = request as? EchoRequest {
+      reply(.success(params.string as! Request.Response))
+    } else if let params = request as? EchoError {
       if let code = params.code {
         reply(.failure(ResponseError(code: code, message: params.message!)))
       } else {
-        reply(.success(VoidResponse() as! R.Response))
+        reply(.success(VoidResponse() as! Request.Response))
       }
     } else {
       fatalError("Unhandled request")
     }
   }
 }
 
-// MARK: Test requests.
+// MARK: Test requests
 
 private let testMessageRegistry = MessageRegistry(
   requests: [EchoRequest.self, EchoError.self],

Sources/LanguageServerProtocol/Connection.swift

@@ -20,7 +20,7 @@ public protocol Connection: AnyObject, Sendable {
 
   /// Send a request and (asynchronously) receive a reply.
   func send<Request: RequestType>(
-    _: Request,
+    _ request: Request,
     reply: @escaping (LSPResult<Request.Response>) -> Void
   ) -> RequestID
 }
@@ -33,7 +33,7 @@ public protocol MessageHandler: AnyObject {
   /// The method should return as soon as the notification has been sufficiently
   /// handled to avoid out-of-order requests, e.g. once the notification has
   /// been forwarded to clangd.
-  func handle(_ params: some NotificationType)
+  func handle(_ notification: some NotificationType)
 
   /// Handle a request and (asynchronously) receive a reply.
   ///
@@ -62,7 +62,7 @@ public protocol MessageHandler: AnyObject {
 /// ```
 ///
 /// - Note: Unchecked sendable conformance because shared state is guarded by `queue`.
-public final class LocalConnection: @unchecked Sendable {
+public final class LocalConnection: Connection, @unchecked Sendable {
 
   enum State {
     case ready, started, closed
@@ -103,9 +103,7 @@ public final class LocalConnection: @unchecked Sendable {
       return .number(_nextRequestID)
     }
   }
-}
 
-extension LocalConnection: Connection {
   public func send<Notification>(_ notification: Notification) where Notification: NotificationType {
     self.handler?.handle(notification)
   }

Sources/LanguageServerProtocolJSONRPC/JSONRPCConnection.swift

@@ -21,7 +21,7 @@ import struct CDispatch.dispatch_fd_t
 
 /// A connection between a message handler (e.g. language server) in the same process as the connection object and a remote message handler (e.g. language client) that may run in another process using JSON RPC messages sent over a pair of in/out file descriptors.
 ///
-/// For example, inside a language server, the `JSONRPCConnection` takes the language service implemenation as its `receiveHandler` and itself provides the client connection for sending notifications and callbacks.
+/// For example, inside a language server, the `JSONRPCConnection` takes the language service implementation as its `receiveHandler` and itself provides the client connection for sending notifications and callbacks.
 public final class JSONRPCConnection {
 
   /// A name of the endpoint for this connection, used for logging, e.g. `clangd`.
@@ -64,7 +64,7 @@ public final class JSONRPCConnection {
   /// The set of currently outstanding outgoing requests along with information about how to decode and handle their responses.
   var outstandingRequests: [RequestID: OutstandingRequest] = [:]
 
-  /// A handler that will be called asyncronously when the connection is being
+  /// A handler that will be called asynchronously when the connection is being
   /// closed.
   var closeHandler: (() async -> Void)? = nil
 

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -112,14 +112,14 @@ public final class TestSourceKitLSPClient: MessageHandler {
     }
     self.notificationYielder = notificationYielder
 
-    let clientConnection = LocalConnection()
-    self.serverToClientConnection = clientConnection
+    let serverToClientConnection = LocalConnection()
+    self.serverToClientConnection = serverToClientConnection
     server = SourceKitServer(
-      client: clientConnection,
+      client: serverToClientConnection,
       toolchainRegistry: ToolchainRegistry.forTesting,
       options: serverOptions,
       onExit: {
-        clientConnection.close()
+        serverToClientConnection.close()
       }
     )
 

Sources/SourceKitLSP/Clang/ClangLanguageServer.swift

@@ -55,9 +55,9 @@ fileprivate class ClangdStderrLogForwarder {
 /// A thin wrapper over a connection to a clangd server providing build setting handling.
 ///
 /// In addition, it also intercepts notifications and replies from clangd in order to do things
-/// like witholding diagnostics when fallback build settings are being used.
+/// like withholding diagnostics when fallback build settings are being used.
 ///
-/// ``ClangLangaugeServerShim`` conforms to ``MessageHandler`` to receive
+/// ``ClangLanguageServerShim`` conforms to ``MessageHandler`` to receive
 /// requests and notifications **from** clangd, not from the editor, and it will
 /// forward these requests and notifications to the editor.
 actor ClangLanguageServerShim: ToolchainLanguageServer, MessageHandler {

Tests/LanguageServerProtocolJSONRPCTests/ConnectionTests.swift

@@ -53,7 +53,7 @@ class ConnectionTests: XCTestCase {
 
   func testMessageBuffer() async throws {
     let client = connection.client
-    let clientConnection = connection.clientConnection
+    let clientConnection = connection.clientToServerConnection
     let expectation = self.expectation(description: "note received")
 
     await client.appendOneShotNotificationHandler { (note: EchoNotification) in
@@ -91,8 +91,8 @@ class ConnectionTests: XCTestCase {
     try await fulfillmentOfOrThrow([expectation2])
 
     // Close the connection before accessing _requestBuffer, which ensures we don't race.
-    connection.serverConnection.close()
-    XCTAssertEqual(connection.serverConnection._requestBuffer, [])
+    connection.serverToClientConnection.close()
+    XCTAssertEqual(connection.serverToClientConnection._requestBuffer, [])
   }
 
   func testEchoError() {
@@ -175,7 +175,7 @@ class ConnectionTests: XCTestCase {
     let expectation = self.expectation(description: "response received")
 
     // response to unknown request
-    connection.clientConnection.sendReply(.success(VoidResponse()), id: .string("unknown"))
+    connection.clientToServerConnection.sendReply(.success(VoidResponse()), id: .string("unknown"))
 
     // Nothing bad should happen; check that the next request works.
 
@@ -193,18 +193,18 @@ class ConnectionTests: XCTestCase {
     let client = connection.client
     let expectation = self.expectation(description: "note received")
 
-    connection.clientConnection.close()
+    connection.clientToServerConnection.close()
 
     client.send(EchoNotification(string: "hi"))
     _ = client.send(EchoRequest(string: "yo")) { result in
       XCTAssertEqual(result, .failure(ResponseError.serverCancelled))
       expectation.fulfill()
     }
 
-    connection.clientConnection.sendReply(.success(VoidResponse()), id: .number(1))
+    connection.clientToServerConnection.sendReply(.success(VoidResponse()), id: .number(1))
 
-    connection.clientConnection.close()
-    connection.clientConnection.close()
+    connection.clientToServerConnection.close()
+    connection.clientToServerConnection.close()
 
     waitForExpectations(timeout: defaultTimeout)
   }
@@ -219,7 +219,7 @@ class ConnectionTests: XCTestCase {
     }
 
     server.client.send(EchoNotification(string: "about to close!"))
-    connection.serverConnection.close()
+    connection.serverToClientConnection.close()
 
     try await fulfillmentOfOrThrow([expectation])
   }
@@ -232,8 +232,8 @@ class ConnectionTests: XCTestCase {
       expectation.fulfill()
     }
     let notification = EchoNotification(string: "about to close!")
-    connection.serverConnection._send(.notification(notification), async: false)
-    connection.serverConnection.close()
+    connection.serverToClientConnection._send(.notification(notification), async: false)
+    connection.serverToClientConnection.close()
 
     try await fulfillmentOfOrThrow([expectation])
   }