address review comments

Author: matthewbastien

Contributor Documentation/LSP Extensions.md

@@ -171,27 +171,39 @@ interface SKCompletionOptions {
 
 ## `textDocument/doccDocumentation`
 
-New request that returns a RenderNode for a symbol at a given location that can then be
-rendered in an editor by `swiftlang/swift-docc-render`.
+New request that generates documentation for a symbol at a given cursor location.
 
-This request uses Swift DocC's convert service to convert documentation for a Swift symbol
-or Markup/Tutorial file. It responds with a string containing a JSON encoded `RenderNode`
-or an error if the documentation could not be converted. This error message can be displayed
-to the user in the live preview editor.
+Primarily designed to support live preview of Swift documentation in editors.
 
-At the moment this request is only available on macOS and Linux. If SourceKit-LSP supports
-this request it will add `textDocument/doccDocumentation` to its experimental server
-capabilities.
+This request looks up the nearest documentable symbol (if any) at a given cursor location within
+a text document and returns a `DoccDocumentationResponse`. The response contains a string
+representing single JSON encoded DocC RenderNode. This RenderNode can then be rendered in an
+editor via https://github.com/swiftlang/swift-docc-render.
+
+The position may be ommitted for documentation within DocC markdown and tutorial files as they
+represent a single documentation page. It is only required for generating documentation within
+Swift files as they usually contain multiple documentable symbols.
+
+Documentation can fail to be generated for a number of reasons. The most common of which being
+that no documentable symbol could be found. In such cases the request will fail with a request
+failed LSP error code (-32803) that contains a human-readable error message. This error message can
+be displayed within the live preview editor to indicate that something has gone wrong.
+
+At the moment this request is only available on macOS and Linux. SourceKit-LSP will advertise
+`textDocument/doccDocumentation` in its experimental server capabilities if it supports it.
+
+- params: `DoccDocumentationParams`
+- result: `DoccDocumentationResponse`
 
 ```ts
 export interface DoccDocumentationParams {
   /**
-   * The document to render documentation for.
+   * The document to generate documentation for.
    */
   textDocument: TextDocumentIdentifier;
 
   /**
-   * The document location at which to lookup symbol information.
+   * The cursor position within the document. (optional)
    * 
    * This parameter is only used in Swift files to determine which symbol to render.
    * The position is ignored for markdown and tutorial documents.

Sources/LanguageServerProtocol/Requests/DoccDocumentationRequest.swift

@@ -10,17 +10,32 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// Request for generating documentation for the symbol at a given location **(LSP Extension)**.
+/// Request that generates documentation for a symbol at a given cursor location **(LSP Extension)**.
 ///
-/// This request looks up the symbol (if any) at a given text document location and returns a
-/// ``DoccDocumentationResponse`` for that location. This request is primarily designed for editors
-/// to support live preview of Swift documentation.
+/// Primarily designed to support live preview of Swift documentation in editors.
+///
+/// This request looks up the nearest documentable symbol (if any) at a given cursor location within
+/// a text document and returns a `DoccDocumentationResponse`. The response contains a string
+/// representing single JSON encoded DocC RenderNode. This RenderNode can then be rendered in an
+/// editor via https://github.com/swiftlang/swift-docc-render.
+///
+/// The position may be ommitted for documentation within DocC markdown and tutorial files as they
+/// represent a single documentation page. It is only required for generating documentation within
+/// Swift files as they usually contain multiple documentable symbols.
+///
+/// Documentation can fail to be generated for a number of reasons. The most common of which being
+/// that no documentable symbol could be found. In such cases the request will fail with a request
+/// failed LSP error code (-32803) that contains a human-readable error message. This error message can
+/// be displayed within the live preview editor to indicate that something has gone wrong.
+///
+/// At the moment this request is only available on macOS and Linux. SourceKit-LSP will advertise
+/// `textDocument/doccDocumentation` in its experimental server capabilities if it supports it.
 ///
 /// - Parameters:
-///   - textDocument: The document to render documentation for.
-///   - position: The document location at which to lookup symbol information. (optional)
+///   - textDocument: The document to generate documentation for.
+///   - position: The cursor position within the document. (optional)
 ///
-/// - Returns: A ``DoccDocumentationResponse`` for the given location, which may contain an error
+/// - Returns: A `DoccDocumentationResponse` for the given location, which may contain an error
 ///   message if documentation could not be converted. This error message can be displayed to the user
 ///   in the live preview editor.
 ///

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -237,17 +237,14 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
 
   // MARK: - Sending messages
 
-  package func sendWithRawResponse<R: RequestType>(_ request: R) async -> Result<R.Response, ResponseError> {
-    await withCheckedContinuation { continuation in
+  /// Send the request to `server` and return the request result.
+  package func send<R: RequestType>(_ request: R) async throws(ResponseError) -> R.Response {
+    let response = await withCheckedContinuation { continuation in
       self.send(request) { result in
         continuation.resume(returning: result)
       }
     }
-  }
-
-  /// Send the request to `server` and return the request result.
-  package func send<R: RequestType>(_ request: R) async throws(ResponseError) -> R.Response {
-    return try await sendWithRawResponse(request).get()
+    return try response.get()
   }
 
   /// Variant of `send` above that allows the response to be discarded if it is a `VoidResponse`.

Sources/SourceKitLSP/Documentation/DocCServer.swift

@@ -82,7 +82,7 @@ package struct DocCServer {
       messageType: ConvertService.convertMessageType,
       messageIdentifier: convertRequestIdentifier,
       request: request
-    );
+    )
     guard let responsePayload = response.payload else {
       throw .unexpectedlyNilPayload(response.type.rawValue)
     }
@@ -136,11 +136,12 @@ package struct DocCServer {
       }
       // Send the request to the server and decode the response
       server.process(encodedMessage) { response in
-        let decodeMessageResult: Result<DocumentationServer.Message, DocCServerError> = Result {
-          try JSONDecoder().decode(DocumentationServer.Message.self, from: response)
+        do {
+          let decodedMessage = try JSONDecoder().decode(DocumentationServer.Message.self, from: response)
+          continuation.resume(returning: .success(decodedMessage))
+        } catch {
+          continuation.resume(returning: .failure(.decodingFailure(error)))
         }
-        .flatMapError { .failure(.decodingFailure($0)) }
-        continuation.resume(returning: decodeMessageResult)
       }
     }
     return try result.get()

Sources/SourceKitLSP/Documentation/DocumentationManager.swift

@@ -45,7 +45,7 @@ package final actor DocumentationManager {
     let snapshot = try sourceKitLSPServer.documentManager.latestSnapshot(documentURI)
     let targetId = await workspace.buildSystemManager.canonicalTarget(for: documentURI)
     var moduleName: String? = nil
-    if let targetId = targetId {
+    if let targetId {
       moduleName = await workspace.buildSystemManager.moduleName(for: documentURI, in: targetId)
     }
 
@@ -70,7 +70,7 @@ package final actor DocumentationManager {
           at: snapshot.absolutePosition(of: position)
         )
       else {
-        throw ResponseError.requestFailed(.noDocumentation)
+        throw ResponseError.requestFailed(convertError: .noDocumentation)
       }
       // Retrieve the symbol graph as well as information about the symbol
       let symbolPosition = await swiftLanguageService.adjustPositionToStartOfIdentifier(
@@ -96,7 +96,7 @@ package final actor DocumentationManager {
       symbolGraphs.append(rawSymbolGraph)
       overridingDocumentationComments[symbolUSR] = nearestDocumentableSymbol.documentationComments
     default:
-      throw ResponseError.requestFailed(.noDocumentation)
+      throw ResponseError.requestFailed(convertError: .noDocumentation)
     }
     // Send the convert request to SwiftDocC and wait for the response
     let convertResponse = try await doccServer.convert(
@@ -124,25 +124,19 @@ package final actor DocumentationManager {
 }
 
 package enum ConvertDocumentationError {
-  case indexNotAvailable
   case noDocumentation
-  case symbolNotFound(String)
 
   public var message: String {
     switch self {
-    case .indexNotAvailable:
-      return "The index is not availble to complete the request"
     case .noDocumentation:
       return "No documentation could be rendered for the position in this document"
-    case .symbolNotFound(let symbolName):
-      return "Could not find symbol \(symbolName) in the project"
     }
   }
 }
 
 fileprivate extension ResponseError {
-  static func requestFailed(_ error: ConvertDocumentationError) -> ResponseError {
-    return ResponseError(code: .requestFailed, message: error.message)
+  static func requestFailed(convertError: ConvertDocumentationError) -> ResponseError {
+    return ResponseError.requestFailed(convertError.message)
   }
 }
 

Tests/SourceKitLSPTests/DoccDocumentationTests.swift

@@ -507,14 +507,13 @@ fileprivate func renderDocumentation(
       XCTFail("No expected response was given for marker \(marker)", file: file, line: line)
       return
     }
-    let actualResponse = await testClient.sendWithRawResponse(
-      DoccDocumentationRequest(
-        textDocument: TextDocumentIdentifier(uri),
-        position: positions[marker]
+    do {
+      let response = try await testClient.send(
+        DoccDocumentationRequest(
+          textDocument: TextDocumentIdentifier(uri),
+          position: positions[marker]
+        )
       )
-    )
-    switch actualResponse {
-    case .success(let response):
       let renderNodeString = response.renderNode
       guard let renderNodeData = renderNodeString.data(using: .utf8),
         let renderNode = try? JSONDecoder().decode(RenderNode.self, from: renderNodeData)
@@ -555,7 +554,7 @@ fileprivate func renderDocumentation(
           line: line
         )
       }
-    case .failure(let error):
+    } catch {
       switch expectedResponse {
       case .renderNode:
         XCTFail(