Address Review Comments

Author: lokesh-tr

Sources/LanguageServerProtocol/Requests/ShowDocumentRequest.swift

@@ -10,13 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// Request from the server to the client to show a document on the client side.
-///
-/// - Parameters:
-///  - uri: The uri to show.
-///  - external: An optional boolean indicates to show the resource in an external program.
-///  - takeFocus: An optional boolean to indicate whether the editor showing the document should take focus or not.
-///  - selection: An optional selection range if the document is a text document.
+/// Request from the server to the client to show a document on the client
+/// side.
 public struct ShowDocumentRequest: RequestType {
   public static let method: String = "window/showDocument"
   public typealias Response = ShowDocumentResponse

Sources/SKSupport/FileSystem.swift

@@ -31,12 +31,20 @@ extension AbsolutePath {
   }
 }
 
-/// The directory to write generated module interfaces
+/// The default directory to write generated files
+/// `<TEMPORARY_DIRECTORY>/sourcekit-lsp/`
+public var defaultDirectoryForGeneratedFiles: AbsolutePath {
+  try! AbsolutePath(validating: NSTemporaryDirectory()).appending(component: "sourcekit-lsp")
+}
+
+/// The default directory to write generated module interfaces
+/// `<TEMPORARY_DIRECTORY>/sourcekit-lsp/GeneratedInterfaces/`
 public var defaultDirectoryForGeneratedInterfaces: AbsolutePath {
-  try! AbsolutePath(validating: NSTemporaryDirectory()).appending(component: "GeneratedInterfaces")
+  defaultDirectoryForGeneratedFiles.appending(component: "GeneratedInterfaces")
 }
 
-/// The directory to write generated macro expansions
+/// The default directory to write generated macro expansions
+/// `<TEMPORARY_DIRECTORY>/sourcekit-lsp/GeneratedMacroExpansions/`
 public var defaultDirectoryForGeneratedMacroExpansions: AbsolutePath {
-  try! AbsolutePath(validating: NSTemporaryDirectory()).appending(component: "GeneratedMacroExpansions")
+  defaultDirectoryForGeneratedFiles.appending(component: "GeneratedMacroExpansions")
 }

Sources/SourceKitLSP/SourceKitLSPServer+Options.swift

@@ -38,11 +38,20 @@ extension SourceKitLSPServer {
     /// Options for code-completion.
     public var completionOptions: SKCompletionOptions
 
-    /// Override the default directory where generated interfaces will be stored
-    public var generatedInterfacesPath: AbsolutePath
+    /// Override the default directory where generated files will be stored
+    public var generatedFilesPath: AbsolutePath
 
-    /// Override the default directory where generated macro expansions will be stored
-    public var generatedMacroExpansionsPath: AbsolutePath
+    /// Path to the generated interfaces
+    /// `<generatedFilesPath>/GeneratedInterfaces/`
+    public var generatedInterfacesPath: AbsolutePath {
+      generatedFilesPath.appending(component: "GeneratedInterfaces")
+    }
+
+    /// Path to the generated macro expansions
+    /// `<generatedFilesPath>`/GeneratedMacroExpansions/
+    public var generatedMacroExpansionsPath: AbsolutePath {
+      generatedFilesPath.appending(component: "GeneratedMacroExpansions")
+    }
 
     /// The time that `SwiftLanguageService` should wait after an edit before starting to compute diagnostics and
     /// sending a `PublishDiagnosticsNotification`.
@@ -62,8 +71,7 @@ extension SourceKitLSPServer {
       compilationDatabaseSearchPaths: [RelativePath] = [],
       indexOptions: IndexOptions = .init(),
       completionOptions: SKCompletionOptions = .init(),
-      generatedInterfacesPath: AbsolutePath = defaultDirectoryForGeneratedInterfaces,
-      generatedMacroExpansionsPath: AbsolutePath = defaultDirectoryForGeneratedMacroExpansions,
+      generatedFilesPath: AbsolutePath = defaultDirectoryForGeneratedFiles,
       swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */
       experimentalFeatures: Set<ExperimentalFeature> = [],
       indexTestHooks: IndexTestHooks = IndexTestHooks()
@@ -73,8 +81,7 @@ extension SourceKitLSPServer {
       self.compilationDatabaseSearchPaths = compilationDatabaseSearchPaths
       self.indexOptions = indexOptions
       self.completionOptions = completionOptions
-      self.generatedInterfacesPath = generatedInterfacesPath
-      self.generatedMacroExpansionsPath = generatedMacroExpansionsPath
+      self.generatedFilesPath = generatedFilesPath
       self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration
       self.experimentalFeatures = experimentalFeatures
       self.indexTestHooks = indexTestHooks

Sources/SourceKitLSP/Swift/ExpandMacroCommand.swift

@@ -14,6 +14,8 @@ import LanguageServerProtocol
 import SourceKitD
 
 public struct ExpandMacroCommand: RefactorCommand {
+  typealias Response = MacroExpansion
+
   public static let identifier: String = "expand.macro.command"
 
   /// The name of this refactoring action.
@@ -55,8 +57,12 @@ public struct ExpandMacroCommand: RefactorCommand {
     )
   }
 
-  public init(title: String, actionString: String, positionRange: Range<Position>, textDocument: TextDocumentIdentifier)
-  {
+  public init(
+    title: String,
+    actionString: String,
+    positionRange: Range<Position>,
+    textDocument: TextDocumentIdentifier
+  ) {
     self.title = title
     self.actionString = actionString
     self.positionRange = positionRange

Sources/SourceKitLSP/Swift/MacroExpansion.swift

@@ -16,9 +16,9 @@ import SourceKitD
 
 /// Detailed information about the result of a macro expansion operation.
 ///
-/// Wraps the information returned by sourcekitd's `semantic_refactoring` request, such as the necessary macro expansion edits.
-struct MacroExpansion: Refactoring {
-
+/// Wraps the information returned by sourcekitd's `semantic_refactoring`
+/// request, such as the necessary macro expansion edits.
+struct MacroExpansion: RefactoringResponse {
   /// The title of the refactoring action.
   var title: String
 
@@ -31,61 +31,65 @@ struct MacroExpansion: Refactoring {
   init(title: String, uri: DocumentURI, refactoringEdits: [RefactoringEdit]) {
     self.title = title
     self.uri = uri
-    self.edits = refactoringEdits.map { refactoringEdit in
-      MacroExpansionEdit(
+    self.edits = refactoringEdits.compactMap { refactoringEdit in
+
+      guard let bufferName = refactoringEdit.bufferName else {
+        logger.error("Unable to retrieve some parts of the expansion")
+        return nil
+      }
+
+      return MacroExpansionEdit(
         range: refactoringEdit.startPosition..<refactoringEdit.endPosition,
         newText: refactoringEdit.newText,
-        bufferName: refactoringEdit.bufferName ?? ""  // TODO: handle bufferName if nil
+        bufferName: bufferName
       )
     }
   }
-
 }
 
 extension SwiftLanguageService {
   /// Handles the `ExpandMacroCommand`.
   ///
-  /// Makes request to sourcekitd and wraps the result into a `MacroExpansion` and then makes `ShowDocumentRequest` to the client side for each expansion to be displayed.
+  /// Makes request to sourcekitd and wraps the result into a `MacroExpansion`
+  /// and then makes `ShowDocumentRequest` to the client side for each
+  /// expansion to be displayed.
   ///
   /// - Parameters:
-  ///   - refactorCommand: The expand macro `SwiftCommand` that triggered this request.
+  ///   - expandMacroCommand: The `ExpandMacroCommand` that triggered this
+  /// request.
   ///
   /// - Returns:
-  ///   - an `[MacroExpansionEdit]` with the necessary edits and buffer name as a `LSPAny`
+  ///   - an `[MacroExpansionEdit]` with the necessary edits and buffer name as
+  /// a `LSPAny`
   func expandMacro(
-    _ refactorCommand: any SwiftCommand
+    _ expandMacroCommand: ExpandMacroCommand
   ) async throws -> LSPAny {
     guard let sourceKitLSPServer else {
-      // `SourceKitLSPServer` has been destructed. We are tearing down the language
-      // server. Nothing left to do.
+      // `SourceKitLSPServer` has been destructed. We are tearing down the
+      // language server. Nothing left to do.
       throw ResponseError.unknown("Connection to the editor closed")
     }
 
-    guard let refactorCommand = refactorCommand as? ExpandMacroCommand else {
-      throw ResponseError.unknown("refactorCommand is not a ExpandMacroCommand")
-    }
-
-    let expansion = try await self.refactoring(refactorCommand, MacroExpansion.self)
+    let expansion = try await self.refactoring(expandMacroCommand)
 
     for macroEdit in expansion.edits {
       let macroExpansionFilePath = self.generatedMacroExpansionsPath.appendingPathComponent(
         macroEdit.bufferName
       )
       let macroExpansionDocURI = DocumentURI(macroExpansionFilePath)
-      if let _ = try? self.documentManager.latestSnapshot(macroExpansionDocURI) {
-        continue
-      }
 
       do {
         try macroEdit.newText.write(to: macroExpansionFilePath, atomically: true, encoding: String.Encoding.utf8)
       } catch {
         throw ResponseError.unknown("Unable to write macro expansion to file path: \"\(macroExpansionFilePath.path)\"")
       }
 
-      let req = ShowDocumentRequest(uri: macroExpansionDocURI, selection: macroEdit.range)
-      let response = try await sourceKitLSPServer.sendRequestToClient(req)
-      if !response.success {
-        logger.error("client refused to show document for \(expansion.title, privacy: .public)!")
+      Task {
+        let req = ShowDocumentRequest(uri: macroExpansionDocURI, selection: macroEdit.range)
+        let response = try await sourceKitLSPServer.sendRequestToClient(req)
+        if !response.success {
+          logger.error("client refused to show document for \(expansion.title, privacy: .public)!")
+        }
       }
     }
 

Sources/SourceKitLSP/Swift/MacroExpansionEdit.swift

@@ -13,8 +13,9 @@
 import LanguageServerProtocol
 import SourceKitD
 
-/// Represents a macro expansion as an edit. Notionally, a subclass of `TextEdit`
-public struct MacroExpansionEdit: ResponseType, Hashable, Sendable {
+/// Represents a macro expansion as an edit. Notionally, a subclass of
+/// `TextEdit`
+public struct MacroExpansionEdit: Hashable, Sendable, Codable {
   /// The range of text to be replaced.
   @CustomCodable<PositionRange>
   public var range: Range<Position>

Sources/SourceKitLSP/Swift/RefactorCommand.swift

@@ -13,9 +13,13 @@
 import LanguageServerProtocol
 import SourceKitD
 
-/// A protocol to be utilised by all commands where underlies sourcekitd calls to perform semantic refactoring.
+/// A protocol to be utilised by all commands where underlies sourcekitd calls
+/// to perform semantic refactoring.
 protocol RefactorCommand: SwiftCommand {
 
+  /// The response type of the refactor command
+  associatedtype Response: RefactoringResponse
+
   /// The sourcekitd identifier of the refactoring action.
   var actionString: String { get set }
 

Sources/SourceKitLSP/Swift/Refactoring.swift

@@ -13,22 +13,31 @@
 import LanguageServerProtocol
 import SourceKitD
 
-protocol Refactoring {
+protocol RefactoringResponse {
+
   init(title: String, uri: DocumentURI, refactoringEdits: [RefactoringEdit])
 
-  /// Create an instance of `Refactoring` from a sourcekitd semantic refactoring response dictionary, if possible. Passes the response to `init(title: String, uri: DocumentURI, refactoringEdits: [RefactoringEdit])` in a neat manner
+  /// Create an instance of `RefactoringResponse` from a sourcekitd semantic
+  /// refactoring response dictionary, if possible. Passes the response to
+  /// `init(title: String, uri: DocumentURI, refactoringEdits:
+  /// [RefactoringEdit])` in a neat manner
   init?(_ title: String, _ dict: SKDResponseDictionary, _ snapshot: DocumentSnapshot, _ keys: sourcekitd_api_keys)
 }
 
 typealias RefactoringEdit = (startPosition: Position, endPosition: Position, newText: String, bufferName: String?)
 
-extension Refactoring {
-  /// Create an instance of `Refactoring` from a sourcekitd semantic refactoring response dictionary, if possible. Passes the response to `init(title: String, uri: DocumentURI, refactoringEdits: [RefactoringEdit])` in a neat manner
+extension RefactoringResponse {
+
+  /// Create an instance of `RefactoringResponse` from a sourcekitd semantic
+  /// refactoring response dictionary, if possible. Passes the response to
+  /// `init(title: String, uri: DocumentURI, refactoringEdits:
+  /// [RefactoringEdit])` in a neat manner
   ///
   /// - Parameters:
   ///   - title: The title of the refactoring action.
   ///   - dict: Response dictionary to extract information from.
-  ///   - url: The client URL that triggered the `semantic_refactoring` request.
+  ///   - snapshot: The snapshot that triggered the `semantic_refactoring`
+  /// request.
   ///   - keys: The sourcekitd key set to use for looking up into `dict`.
   init?(_ title: String, _ dict: SKDResponseDictionary, _ snapshot: DocumentSnapshot, _ keys: sourcekitd_api_keys) {
     guard let categorizedEdits: SKDResponseArray = dict[keys.categorizedEdits] else {
@@ -61,7 +70,8 @@ extension Refactoring {
           utf8Column: endColumn - 1
         )
         // Snippets are only supported in code completion.
-        // Remove SourceKit placeholders in refactoring actions because they can't be represented in the editor properly.
+        // Remove SourceKit placeholders in refactoring actions because they
+        // can't be represented in the editor properly.
         let textWithSnippets = rewriteSourceKitPlaceholders(in: text, clientSupportsSnippets: false)
         refactoringEdits.append(
           (
@@ -83,16 +93,20 @@ extension Refactoring {
 }
 
 extension SwiftLanguageService {
-  /// Provides detailed information about the result of a specific refactoring operation.
+  /// Provides detailed information about the result of a specific refactoring
+  /// operation.
   ///
-  /// Wraps the information returned by sourcekitd's `semantic_refactoring` request, such as the necessary edits and placeholder locations.
+  /// Wraps the information returned by sourcekitd's `semantic_refactoring`
+  /// request, such as the necessary edits and placeholder locations.
   ///
   /// - Parameters:
   ///   - command: The semantic `RefactorCommand` that triggered this request.
   ///   - responseType: The response type `T.Type` of the particular command
   /// - Returns:
-  ///   - The `Refactoring` as `T`
-  func refactoring<T: Refactoring>(_ refactorCommand: any RefactorCommand, _ responseType: T.Type) async throws -> T {
+  ///   - The `RefactoringResponse` as `T`
+  func refactoring<T: RefactorCommand>(_ refactorCommand: T) async throws
+    -> T.Response
+  {
     let keys = self.keys
 
     let uri = refactorCommand.textDocument.uri
@@ -116,7 +130,7 @@ extension SwiftLanguageService {
     ])
 
     let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text)
-    guard let refactor = T(refactorCommand.title, dict, snapshot, self.keys) else {
+    guard let refactor = T.Response(refactorCommand.title, dict, snapshot, self.keys) else {
       throw SemanticRefactoringError.noEditsNeeded(uri)
     }
     return refactor

Sources/SourceKitLSP/Swift/SemanticRefactorCommand.swift

@@ -9,10 +9,12 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import LanguageServerProtocol
 import SourceKitD
 
 public struct SemanticRefactorCommand: RefactorCommand {
+  typealias Response = SemanticRefactoring
 
   public static let identifier: String = "semantic.refactor.command"
 

Sources/SourceKitLSP/Swift/SemanticRefactoring.swift

@@ -1,4 +1,3 @@
-import LSPLogging
 //===----------------------------------------------------------------------===//
 //
 // This source file is part of the Swift.org open source project
@@ -10,13 +9,16 @@ import LSPLogging
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
+import LSPLogging
 import LanguageServerProtocol
 import SourceKitD
 
 /// Detailed information about the result of a specific refactoring operation.
 ///
-/// Wraps the information returned by sourcekitd's `semantic_refactoring` request, such as the necessary edits and placeholder locations.
-struct SemanticRefactoring: Refactoring {
+/// Wraps the information returned by sourcekitd's `semantic_refactoring`
+/// request, such as the necessary edits and placeholder locations.
+struct SemanticRefactoring: RefactoringResponse {
 
   /// The title of the refactoring action.
   var title: String
@@ -61,27 +63,26 @@ extension SwiftLanguageService {
 
   /// Handles the `SemanticRefactorCommand`.
   ///
-  /// Makes request to sourcekitd and wraps the result into a `SemanticRefactoring` and then makes an `ApplyEditRequest` to the client side for the actual refactoring.
+  /// Makes request to sourcekitd and wraps the result into a
+  /// `SemanticRefactoring` and then makes an `ApplyEditRequest` to the client
+  /// side for the actual refactoring.
   ///
   /// - Parameters:
-  ///   - refactorCommand: The semantic refactor `SwiftCommand` that triggered this request.
+  ///   - semanticRefactorCommand: The `SemanticRefactorCommand` that triggered
+  /// this request.
   ///
   /// - Returns:
   ///   - a `WorkspaceEdit` with the necessary refactors as a `LSPAny`
   func semanticRefactoring(
-    _ refactorCommand: any SwiftCommand
+    _ semanticRefactorCommand: SemanticRefactorCommand
   ) async throws -> LSPAny {
     guard let sourceKitLSPServer else {
-      // `SourceKitLSPServer` has been destructed. We are tearing down the language
-      // server. Nothing left to do.
+      // `SourceKitLSPServer` has been destructed. We are tearing down the
+      // language server. Nothing left to do.
       throw ResponseError.unknown("Connection to the editor closed")
     }
 
-    guard let refactorCommand = refactorCommand as? SemanticRefactorCommand else {
-      throw ResponseError.unknown("refactorCommand is not a SemanticRefactorCommand")
-    }
-
-    let semanticRefactor = try await self.refactoring(refactorCommand, SemanticRefactoring.self)
+    let semanticRefactor = try await self.refactoring(semanticRefactorCommand)
 
     let edit = semanticRefactor.edit
     let req = ApplyEditRequest(label: semanticRefactor.title, edit: edit)