Address Review Comments

Sources/SourceKitLSP/Swift/MacroExpansion.swift

@@ -33,7 +33,7 @@ struct MacroExpansion: RefactoringResponse {
     self.title = title
     self.uri = uri
     self.edits = refactoringEdits.compactMap { refactoringEdit in
-      if refactoringEdit.bufferName.isEmpty {
+      if refactoringEdit.bufferName == nil && !refactoringEdit.newText.isEmpty {
         logger.fault("Unable to retrieve some parts of the expansion")
         return nil
       }
@@ -46,14 +46,14 @@ struct MacroExpansion: RefactoringResponse {
 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
+  /// Makes a request to sourcekitd and wraps the result into a `MacroExpansion`
+  /// and then makes a `ShowDocumentRequest` to the client side for each
   /// expansion to be displayed.
   ///
   /// - Parameters:
   ///   - expandMacroCommand: The `ExpandMacroCommand` that triggered this request.
   ///
-  /// - Returns: An `[RefactoringEdit]` with the necessary edits and buffer name as a `LSPAny`
+  /// - Returns: A `[RefactoringEdit]` with the necessary edits and buffer name as a `LSPAny`
   func expandMacro(
     _ expandMacroCommand: ExpandMacroCommand
   ) async throws -> LSPAny {
@@ -70,46 +70,57 @@ extension SwiftLanguageService {
     let expansion = try await self.refactoring(expandMacroCommand)
 
     for macroEdit in expansion.edits {
-      // buffer name without ".swift"
-      let macroExpansionBufferDirectoryName =
-        macroEdit.bufferName.hasSuffix(".swift")
-        ? String(macroEdit.bufferName.dropLast(6))
-        : macroEdit.bufferName
-
-      let macroExpansionBufferDirectoryURL = self.generatedMacroExpansionsPath
-        .appendingPathComponent(macroExpansionBufferDirectoryName)
-      do {
-        try FileManager.default.createDirectory(at: macroExpansionBufferDirectoryURL, withIntermediateDirectories: true)
-      } catch {
-        throw ResponseError.unknown(
-          "Failed to create directory for Macro Expansion Buffer at path: \(macroExpansionBufferDirectoryURL.path)"
-        )
-      }
-
-      let macroExpansionFileName = sourceFileURL.deletingPathExtension().lastPathComponent  // name of the source file
-      let macroExpansionPositionRangeIndicator =
-        "L\(macroEdit.range.lowerBound.line)C\(macroEdit.range.lowerBound.utf16index)-L\(macroEdit.range.upperBound.line)C\(macroEdit.range.upperBound.utf16index)"  // github permalink notation for position range
-
-      let macroExpansionFilePath = macroExpansionBufferDirectoryURL.appendingPathComponent(
-        "\(macroExpansionFileName)_\(macroExpansionPositionRangeIndicator).\(sourceFileURL.pathExtension)"
-      )
+      if let bufferName = macroEdit.bufferName {
+        // buffer name without ".swift"
+        let macroExpansionBufferDirectoryName =
+          bufferName.hasSuffix(".swift")
+          ? String(bufferName.dropLast(6))
+          : bufferName
+
+        let macroExpansionBufferDirectoryURL = self.generatedMacroExpansionsPath
+          .appendingPathComponent(macroExpansionBufferDirectoryName)
+        do {
+          try FileManager.default.createDirectory(
+            at: macroExpansionBufferDirectoryURL,
+            withIntermediateDirectories: true
+          )
+        } catch {
+          throw ResponseError.unknown(
+            "Failed to create directory for macro expansion buffer at path: \(macroExpansionBufferDirectoryURL.path)"
+          )
+        }
 
-      do {
-        try macroEdit.newText.write(to: macroExpansionFilePath, atomically: true, encoding: .utf8)
-      } catch {
-        throw ResponseError.unknown("Unable to write macro expansion to file path: \"\(macroExpansionFilePath.path)\"")
-      }
+        let macroExpansionFileName = sourceFileURL.deletingPathExtension().lastPathComponent  // name of the source file
+        let macroExpansionPositionRangeIndicator =
+          "L\(macroEdit.range.lowerBound.line)C\(macroEdit.range.lowerBound.utf16index)-L\(macroEdit.range.upperBound.line)C\(macroEdit.range.upperBound.utf16index)"  // github permalink notation for position range
+
+        let macroExpansionFilePath =
+          macroExpansionBufferDirectoryURL
+          .appendingPathComponent(
+            "\(macroExpansionFileName)_\(macroExpansionPositionRangeIndicator).\(sourceFileURL.pathExtension)"
+          )
+
+        do {
+          try macroEdit.newText.write(to: macroExpansionFilePath, atomically: true, encoding: .utf8)
+        } catch {
+          throw ResponseError.unknown(
+            "Unable to write macro expansion to file path: \"\(macroExpansionFilePath.path)\""
+          )
+        }
 
-      Task {
-        let req = ShowDocumentRequest(uri: DocumentURI(macroExpansionFilePath), selection: macroEdit.range)
+        Task {
+          let req = ShowDocumentRequest(uri: DocumentURI(macroExpansionFilePath), selection: macroEdit.range)
 
-        let response = await orLog("Sending ShowDocumentRequest to Client") {
-          try await sourceKitLSPServer.sendRequestToClient(req)
-        }
+          let response = await orLog("Sending ShowDocumentRequest to Client") {
+            try await sourceKitLSPServer.sendRequestToClient(req)
+          }
 
-        if let response, !response.success {
-          logger.error("client refused to show document for \(expansion.title, privacy: .public)")
+          if let response, !response.success {
+            logger.error("client refused to show document for \(expansion.title, privacy: .public)")
+          }
         }
+      } else if !macroEdit.newText.isEmpty {
+        logger.fault("Unable to retrieve some parts of macro expansion")
       }
     }
 

Sources/SourceKitLSP/Swift/Refactoring.swift

@@ -68,7 +68,7 @@ extension RefactoringResponse {
           RefactoringEdit(
             range: startPosition..<endPosition,
             newText: textWithSnippets,
-            bufferName: edit[keys.bufferName] ?? ""
+            bufferName: edit[keys.bufferName]
           )
         )
         return true

Sources/SourceKitLSP/Swift/RefactoringEdit.swift

@@ -26,9 +26,9 @@ import SourceKitD
   /// file but to a separate buffer, a fake name for that buffer. For example
   /// for expansion of macros, this is @ followed by the mangled name of the
   /// macro expansion, followed by .swift.
-  public var bufferName: String
+  public var bufferName: String?
 
-  public init(range: Range<Position>, newText: String, bufferName: String) {
+  public init(range: Range<Position>, newText: String, bufferName: String?) {
     self._range = CustomCodable<PositionRange>(wrappedValue: range)
     self.newText = newText
     self.bufferName = bufferName
@@ -38,20 +38,33 @@ import SourceKitD
 extension RefactoringEdit: LSPAnyCodable {
   public init?(fromLSPDictionary dictionary: [String: LSPAny]) {
     guard case .dictionary(let rangeDict) = dictionary[CodingKeys.range.stringValue],
-      case .string(let newText) = dictionary[CodingKeys.newText.stringValue],
-      case .string(let bufferName) = dictionary[CodingKeys.bufferName.stringValue]
+      case .string(let newText) = dictionary[CodingKeys.newText.stringValue]
     else {
       return nil
     }
+
     guard let range = Range<Position>(fromLSPDictionary: rangeDict) else {
       return nil
     }
+
     self._range = CustomCodable<PositionRange>(wrappedValue: range)
     self.newText = newText
-    self.bufferName = bufferName
+
+    if case .string(let bufferName) = dictionary[CodingKeys.bufferName.stringValue] {
+      self.bufferName = bufferName
+    } else {
+      self.bufferName = nil
+    }
   }
 
   public func encodeToLSPAny() -> LSPAny {
+    guard let bufferName = bufferName else {
+      return .dictionary([
+        CodingKeys.range.stringValue: range.encodeToLSPAny(),
+        CodingKeys.newText.stringValue: .string(newText),
+      ])
+    }
+
     return .dictionary([
       CodingKeys.range.stringValue: range.encodeToLSPAny(),
       CodingKeys.newText.stringValue: .string(newText),

Sources/SourceKitLSP/Swift/SemanticRefactoring.swift

@@ -63,7 +63,7 @@ extension SwiftLanguageService {
 
   /// Handles the `SemanticRefactorCommand`.
   ///
-  /// Makes request to sourcekitd and wraps the result into a
+  /// Sends a request to sourcekitd and wraps the result into a
   /// `SemanticRefactoring` and then makes an `ApplyEditRequest` to the client
   /// side for the actual refactoring.
   ///

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

@@ -819,16 +819,14 @@ extension SwiftLanguageService {
 
     var canInlineMacro = false
 
-    let showMacroExpansionsIsEnabled = await self.sourceKitLSPServer?.options.experimentalFeatures.contains(
-      .showMacroExpansions
-    )
+    let showMacroExpansionsIsEnabled =
+      await self.sourceKitLSPServer?.options.experimentalFeatures
+      .contains(.showMacroExpansions) ?? false
 
     var refactorActions = cursorInfoResponse.refactorActions.compactMap {
       let lspCommand = $0.asCommand()
-      canInlineMacro = $0.actionString == "source.refactoring.kind.inline.macro"
-
-      if !(showMacroExpansionsIsEnabled != nil && showMacroExpansionsIsEnabled!) {
-        canInlineMacro = false
+      if !canInlineMacro, showMacroExpansionsIsEnabled {
+        canInlineMacro = $0.actionString == "source.refactoring.kind.inline.macro"
       }
 
       return CodeAction(title: $0.title, kind: .refactor, command: lspCommand)

Tests/SourceKitLSPTests/ExecuteCommandTests.swift

@@ -140,6 +140,9 @@ final class ExecuteCommandTests: XCTestCase {
   func testFreestandingMacroExpansion() async throws {
     try await SkipUnless.canBuildMacroUsingSwiftSyntaxFromSourceKitLSPBuild()
 
+    var serverOptions = SourceKitLSPServer.Options.testDefault
+    serverOptions.experimentalFeatures.insert(.showMacroExpansions)
+
     let project = try await SwiftPMTestProject(
       files: [
         "MyMacros/MyMacros.swift": #"""
@@ -178,7 +181,7 @@ final class ExecuteCommandTests: XCTestCase {
         """,
       ],
       manifest: SwiftPMTestProject.macroPackageManifest,
-      serverOptions: .init(swiftPublishDiagnosticsDebounceDuration: 0, experimentalFeatures: [.showMacroExpansions])
+      serverOptions: serverOptions
     )
     try await SwiftPMTestProject.build(at: project.scratchDirectory)