Sink handling of file writing into the SourceKitRequestExecutor

It’s cleaner plus it opens the option to reduce compiler crashes in the future.

Author: ahoppen

Sources/Diagnose/CommandLineArgumentsReducer.swift

@@ -34,9 +34,6 @@ fileprivate class CommandLineArgumentReducer {
   /// still crashes.
   private let sourcekitdExecutor: SourceKitRequestExecutor
 
-  /// The file to which we write the reduced source code.
-  private let temporarySourceFile: URL
-
   /// A callback to be called when the reducer has made progress reducing the request
   private let progressUpdate: (_ progress: Double, _ message: String) -> Void
 
@@ -48,14 +45,9 @@ fileprivate class CommandLineArgumentReducer {
     progressUpdate: @escaping (_ progress: Double, _ message: String) -> Void
   ) {
     self.sourcekitdExecutor = sourcekitdExecutor
-    self.temporarySourceFile = FileManager.default.temporaryDirectory.appendingPathComponent("reduce-\(UUID()).swift")
     self.progressUpdate = progressUpdate
   }
 
-  deinit {
-    try? FileManager.default.removeItem(at: temporarySourceFile)
-  }
-
   func logSuccessfulReduction(_ requestInfo: RequestInfo) {
     progressUpdate(
       1 - (Double(requestInfo.compilerArgs.count) / Double(initialCommandLineCount)),
@@ -64,7 +56,6 @@ fileprivate class CommandLineArgumentReducer {
   }
 
   func run(initialRequestInfo: RequestInfo) async throws -> RequestInfo {
-    try initialRequestInfo.fileContents.write(to: temporarySourceFile, atomically: true, encoding: .utf8)
     var requestInfo = initialRequestInfo
     self.initialCommandLineCount = requestInfo.compilerArgs.count
 
@@ -123,7 +114,7 @@ fileprivate class CommandLineArgumentReducer {
     var reducedRequestInfo = requestInfo
     reducedRequestInfo.compilerArgs.removeSubrange(argumentsToRemove)
 
-    let result = try await sourcekitdExecutor.run(request: reducedRequestInfo.request(for: temporarySourceFile))
+    let result = try await sourcekitdExecutor.run(request: reducedRequestInfo)
     if case .reproducesIssue = result {
       logSuccessfulReduction(reducedRequestInfo)
       return reducedRequestInfo

Sources/Diagnose/RequestInfo.swift

@@ -34,7 +34,8 @@ public struct RequestInfo {
   @_spi(Testing)
   public var fileContents: String
 
-  func request(for file: URL) throws -> String {
+  @_spi(Testing)
+  public func request(for file: URL) throws -> String {
     let encoder = JSONEncoder()
     encoder.outputFormatting = .prettyPrinted
     guard var compilerArgs = String(data: try encoder.encode(compilerArgs), encoding: .utf8) else {

Sources/Diagnose/SourceKitDRequestExecutor.swift

@@ -45,29 +45,40 @@ fileprivate extension String {
 /// An executor that can run a sourcekitd request and indicate whether the request reprodes a specified issue.
 @_spi(Testing)
 public protocol SourceKitRequestExecutor {
-  func run(request requestString: String) async throws -> SourceKitDRequestResult
+  func run(request: RequestInfo) async throws -> SourceKitDRequestResult
 }
 
 /// Runs `sourcekit-lsp run-sourcekitd-request` to check if a sourcekit-request crashes.
-struct OutOfProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
+class OutOfProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
   /// The path to `sourcekitd.framework/sourcekitd`.
   private let sourcekitd: URL
 
-  /// The file to which we write the JSON request that we want to run.
+  /// The file to which we write the reduce source file.
   private let temporarySourceFile: URL
 
+  /// The file to which we write the YAML request that we want to run.
+  private let temporaryRequestFile: URL
+
   /// If this predicate evaluates to true on the sourcekitd response, the request is
   /// considered to reproduce the issue.
   private let reproducerPredicate: NSPredicate?
 
   init(sourcekitd: URL, reproducerPredicate: NSPredicate?) {
     self.sourcekitd = sourcekitd
     self.reproducerPredicate = reproducerPredicate
-    temporarySourceFile = FileManager.default.temporaryDirectory.appendingPathComponent("request.json")
+    temporaryRequestFile = FileManager.default.temporaryDirectory.appendingPathComponent("request-\(UUID()).yml")
+    temporarySourceFile = FileManager.default.temporaryDirectory.appendingPathComponent("recude-\(UUID()).swift")
+  }
+
+  deinit {
+    try? FileManager.default.removeItem(at: temporaryRequestFile)
+    try? FileManager.default.removeItem(at: temporarySourceFile)
   }
 
-  func run(request requestString: String) async throws -> SourceKitDRequestResult {
-    try requestString.write(to: temporarySourceFile, atomically: true, encoding: .utf8)
+  func run(request: RequestInfo) async throws -> SourceKitDRequestResult {
+    try request.fileContents.write(to: temporarySourceFile, atomically: true, encoding: .utf8)
+    let requestString = try request.request(for: temporarySourceFile)
+    try requestString.write(to: temporaryRequestFile, atomically: true, encoding: .utf8)
 
     let process = Process(
       arguments: [
@@ -76,7 +87,7 @@ struct OutOfProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
         "--sourcekitd",
         sourcekitd.path,
         "--request-file",
-        temporarySourceFile.path,
+        temporaryRequestFile.path,
       ]
     )
     try process.launch()

Sources/Diagnose/SourceReducer.swift

@@ -66,9 +66,6 @@ fileprivate class SourceReducer {
   /// still crashes.
   private let sourcekitdExecutor: SourceKitRequestExecutor
 
-  /// The file to which we write the reduced source code.
-  private let temporarySourceFile: URL
-
   /// A callback to call to report progress
   private let progressUpdate: (_ progress: Double, _ message: String) -> Void
 
@@ -83,14 +80,9 @@ fileprivate class SourceReducer {
     progressUpdate: @escaping (_ progress: Double, _ message: String) -> Void
   ) {
     self.sourcekitdExecutor = sourcekitdExecutor
-    self.temporarySourceFile = FileManager.default.temporaryDirectory.appendingPathComponent("reduce-\(UUID()).swift")
     self.progressUpdate = progressUpdate
   }
 
-  deinit {
-    try? FileManager.default.removeItem(at: temporarySourceFile)
-  }
-
   /// Reduce the file contents in `initialRequest` to a smaller file that still reproduces a crash.
   func run(initialRequestInfo: RequestInfo) async throws -> RequestInfo {
     var requestInfo = initialRequestInfo
@@ -243,9 +235,8 @@ fileprivate class SourceReducer {
       fileContents: reducedSource
     )
 
-    try reducedSource.write(to: temporarySourceFile, atomically: true, encoding: .utf8)
     logger.debug("Try reduction to the following input file:\n\(reducedSource)")
-    let result = try await sourcekitdExecutor.run(request: reducedRequestInfo.request(for: temporarySourceFile))
+    let result = try await sourcekitdExecutor.run(request: reducedRequestInfo)
     if case .reproducesIssue = result {
       logger.debug("Reduction successful")
       logSuccessfulReduction(reducedRequestInfo, tree: tree)
@@ -511,9 +502,8 @@ fileprivate func getSwiftInterface(
     compilerArgs: compilerArgs,
     fileContents: ""
   )
-  let request = try requestInfo.request(for: URL(fileURLWithPath: "/"))
 
-  guard case .success(let result) = try await executor.run(request: request) else {
+  guard case .success(let result) = try await executor.run(request: requestInfo) else {
     throw ReductionError("Failed to get Swift Interface for \(moduleName)")
   }
 

Tests/DiagnoseTests/DiagnoseTests.swift

@@ -191,30 +191,43 @@ private func assertReduce(
   }
 }
 
-private struct InProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
+/// We can't run the `OutOfProcessSourceKitRequestExecutor` in tests because that runs the sourcekit-lsp executable,
+/// which isn't built when running tests.
+private class InProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
   /// The path to `sourcekitd.framework/sourcekitd`.
   private let sourcekitd: URL
 
-  /// The file to which we write the JSON request that we want to run.
+  /// The file to which we write the reduce source file.
   private let temporarySourceFile: URL
 
+  /// The file to which we write the AYML request that we want to run.
+  private let temporaryRequestFile: URL
+
   /// If this predicate evaluates to true on the sourcekitd response, the request is
   /// considered to reproduce the issue.
   private let reproducerPredicate: NSPredicate
 
   init(sourcekitd: URL, reproducerPredicate: NSPredicate) {
     self.sourcekitd = sourcekitd
     self.reproducerPredicate = reproducerPredicate
-    temporarySourceFile = FileManager.default.temporaryDirectory.appendingPathComponent("request.json")
+    temporaryRequestFile = FileManager.default.temporaryDirectory.appendingPathComponent("request-\(UUID()).yml")
+    temporarySourceFile = FileManager.default.temporaryDirectory.appendingPathComponent("recude-\(UUID()).swift")
+  }
+
+  deinit {
+    try? FileManager.default.removeItem(at: temporaryRequestFile)
+    try? FileManager.default.removeItem(at: temporarySourceFile)
   }
 
-  func run(request requestYaml: String) async throws -> SourceKitDRequestResult {
-    logger.info("Sending request: \(requestYaml)")
+  func run(request: RequestInfo) async throws -> SourceKitDRequestResult {
+    try request.fileContents.write(to: temporarySourceFile, atomically: true, encoding: .utf8)
+    let requestString = try request.request(for: temporarySourceFile)
+    logger.info("Sending request: \(requestString)")
 
     let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
       dylibPath: try! AbsolutePath(validating: sourcekitd.path)
     )
-    let response = try await sourcekitd.run(requestYaml: requestYaml)
+    let response = try await sourcekitd.run(requestYaml: requestString)
 
     logger.info("Received response: \(response.description)")