Address typo-kind review comments from #1037

Author: ahoppen

Sources/Diagnose/CMakeLists.txt

@@ -1,14 +1,14 @@
 add_library(Diagnose STATIC
   CommandLineArgumentsReducer.swift
   DiagnoseCommand.swift
-  FileReducer.swift
   FixItApplier.swift
   OSLogScraper.swift
   ReductionError.swift
   ReproducerBundle.swift
   RequestInfo.swift
   SourceKitDRequestExecutor.swift
-  SourcekitdRequestCommand.swift)
+  SourcekitdRequestCommand.swift
+  SourceReducer.swift)
 
 set_target_properties(Diagnose PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY})

Sources/Diagnose/CommandLineArgumentsReducer.swift

@@ -12,8 +12,19 @@
 
 import Foundation
 
+// MARK: - Entry point
+
+extension RequestInfo {
+  func reduceCommandLineArguments(using executor: SourceKitRequestExecutor) async throws -> RequestInfo {
+    let reducer = CommandLineArgumentReducer(sourcekitdExecutor: executor)
+    return try await reducer.run(initialRequestInfo: self)
+  }
+}
+
+// MARK: - FileProducer
+
 /// Reduces the compiler arguments needed to reproduce a sourcekitd crash.
-class CommandLineArgumentReducer {
+fileprivate class CommandLineArgumentReducer {
   /// The executor that is used to run a sourcekitd request and check whether it
   /// still crashes.
   private let sourcekitdExecutor: SourceKitRequestExecutor

Sources/Diagnose/DiagnoseCommand.swift

@@ -26,13 +26,13 @@ public struct DiagnoseCommand: AsyncParsableCommand {
   @Option(
     name: .customLong("request-file"),
     help:
-      "Path to a sourcekitd request. If not specified, the command will look for crashed sourcekitd requests and have been logged to OSLog"
+      "Path to a sourcekitd request. If not specified, the command will look for crashed sourcekitd requests that have been logged to OSLog"
   )
   var sourcekitdRequestPath: String?
 
   @Option(
     name: .customLong("os-log-history"),
-    help: "If now request file is passed, how many minutes of OS Log history should be scraped for a crash."
+    help: "If no request file is passed, how many minutes of OS Log history should be scraped for a crash."
   )
   var osLogScrapeDuration: Int = 60
 
@@ -88,45 +88,53 @@ public struct DiagnoseCommand: AsyncParsableCommand {
       throw ReductionError("Unable to find sourcekitd.framework")
     }
 
+    var reproducerBundle: URL?
     for (name, requestInfo) in try requestInfos() {
       print("-- Diagnosing \(name)")
       do {
-        var requestInfo = requestInfo
-        var nspredicate: NSPredicate? = nil
-        #if canImport(Darwin)
-        if let predicate {
-          nspredicate = NSPredicate(format: predicate)
-        }
-        #endif
-        let executor = SourceKitRequestExecutor(
-          sourcekitd: URL(fileURLWithPath: sourcekitd),
-          reproducerPredicate: nspredicate
-        )
-        let fileReducer = FileReducer(sourcekitdExecutor: executor)
-        requestInfo = try await fileReducer.run(initialRequestInfo: requestInfo)
-
-        let commandLineReducer = CommandLineArgumentReducer(sourcekitdExecutor: executor)
-        requestInfo = try await commandLineReducer.run(initialRequestInfo: requestInfo)
-
-        let reproducerBundle = try makeReproducerBundle(for: requestInfo)
-
-        print("----------------------------------------")
-        print(
-          "Reduced SourceKit crash and created a bundle that contains information to reproduce the issue at the following path."
-        )
-        print("Please file an issue at https://github.com/apple/sourcekit-lsp/issues/new and attach this bundle")
-        print()
-        print(reproducerBundle.path)
-
-        // We have found a reproducer. Stop. Looking further probably won't help because other crashes are likely the same cause.
-        return
+        reproducerBundle = try await reduce(requestInfo: requestInfo, sourcekitd: sourcekitd)
+        // If reduce didn't throw, we have found a reproducer. Stop.
+        // Looking further probably won't help because other crashes are likely the same cause.
+        break
       } catch {
         // Reducing this request failed. Continue reducing the next one, maybe that one succeeds.
         print(error)
       }
     }
 
-    print("No reducible crashes found")
-    throw ExitCode(1)
+    guard let reproducerBundle else {
+      print("No reducible crashes found")
+      throw ExitCode(1)
+    }
+    print(
+      """
+        ----------------------------------------
+        Reduced SourceKit issue and created a bundle that contains a reduced sourcekitd request exhibiting the issue
+        and all the files referenced from the request.
+        The information in this bundle should be sufficient to reproduce the issue.
+
+        Please file an issue at https://github.com/apple/sourcekit-lsp/issues/new and attach the bundle located at
+        \(reproducerBundle.path)
+      """
+    )
+
+  }
+
+  private func reduce(requestInfo: RequestInfo, sourcekitd: String) async throws -> URL {
+    var requestInfo = requestInfo
+    var nspredicate: NSPredicate? = nil
+    #if canImport(Darwin)
+    if let predicate {
+      nspredicate = NSPredicate(format: predicate)
+    }
+    #endif
+    let executor = SourceKitRequestExecutor(
+      sourcekitd: URL(fileURLWithPath: sourcekitd),
+      reproducerPredicate: nspredicate
+    )
+    requestInfo = try await requestInfo.reduceInputFile(using: executor)
+    requestInfo = try await requestInfo.reduceCommandLineArguments(using: executor)
+
+    return try makeReproducerBundle(for: requestInfo)
   }
 }

Sources/Diagnose/RequestInfo.swift

@@ -18,14 +18,14 @@ struct RequestInfo {
   /// The JSON request object. Contains the following dynamic placeholders:
   ///  - `$OFFSET`: To be replaced by `offset` before running the request
   ///  - `$FILE`: Will be replaced with a path to the file that contains the reduced source code.
-  ///  - `$COMPILERARGS`: Will be replaced by the compiler arguments of the request
+  ///  - `$COMPILER_ARGS`: Will be replaced by the compiler arguments of the request
   var requestTemplate: String
 
   /// The offset at which the sourcekitd request should be run. Replaces the
   /// `$OFFSET` placeholder in the request template.
   var offset: Int
 
-  /// The compiler arguments of the request. Replaces the `$COMPILERARGS`placeholder in the request template.
+  /// The compiler arguments of the request. Replaces the `$COMPILER_ARGS`placeholder in the request template.
   var compilerArgs: [String]
 
   /// The contents of the file that the sourcekitd request operates on.
@@ -42,7 +42,7 @@ struct RequestInfo {
     return
       requestTemplate
       .replacingOccurrences(of: "$OFFSET", with: String(offset))
-      .replacingOccurrences(of: "$COMPILERARGS", with: compilerArgs)
+      .replacingOccurrences(of: "$COMPILER_ARGS", with: compilerArgs)
       .replacingOccurrences(of: "$FILE", with: file.path)
 
   }
@@ -106,7 +106,7 @@ private func extractCompilerArguments(
   else {
     return (requestTemplate, [])
   }
-  let template = lines[...compilerArgsStartIndex] + ["$COMPILERARGS"] + lines[compilerArgsEndIndex...]
+  let template = lines[...compilerArgsStartIndex] + ["$COMPILER_ARGS"] + lines[compilerArgsEndIndex...]
   let compilerArgsJson = "[" + lines[(compilerArgsStartIndex + 1)..<compilerArgsEndIndex].joined(separator: "\n") + "]"
   let compilerArgs = try JSONDecoder().decode([String].self, from: compilerArgsJson)
   return (template.joined(separator: "\n"), compilerArgs)

Sources/Diagnose/SourceKitDRequestExecutor.swift

@@ -16,7 +16,7 @@ import SourceKitD
 import struct TSCBasic.AbsolutePath
 import class TSCBasic.Process
 
-/// The different states in which a sourcektid request can finish.
+/// The different states in which a sourcekitd request can finish.
 enum SourceKitDRequestResult {
   /// The request succeeded.
   case success(response: String)

Sources/Diagnose/FileReducer.swift renamed to Sources/Diagnose/SourceReducer.swift

@@ -15,8 +15,19 @@ import SourceKitD
 import SwiftParser
 import SwiftSyntax
 
+// MARK: - Entry point
+
+extension RequestInfo {
+  func reduceInputFile(using executor: SourceKitRequestExecutor) async throws -> RequestInfo {
+    let reducer = SourceReducer(sourcekitdExecutor: executor)
+    return try await reducer.run(initialRequestInfo: self)
+  }
+}
+
+// MARK: - SourceReducer
+
 /// Reduces an input source file while continuing to reproduce the crash
-class FileReducer {
+fileprivate class SourceReducer {
   /// The executor that is used to run a sourcekitd request and check whether it
   /// still crashes.
   private let sourcekitdExecutor: SourceKitRequestExecutor
@@ -32,7 +43,7 @@ class FileReducer {
   /// 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
-    try await validateRequestInfoCrashes(requestInfo: requestInfo)
+    try await validateRequestInfoReproucesIssue(requestInfo: requestInfo)
 
     requestInfo = try await fatalErrorFunctionBodies(requestInfo)
     requestInfo = try await removeMembersAndCodeBlockItemsBodies(requestInfo)
@@ -51,12 +62,12 @@ class FileReducer {
     return requestInfo
   }
 
-  // MARK: - Reduction steps
+  // MARK: Reduction steps
 
-  private func validateRequestInfoCrashes(requestInfo: RequestInfo) async throws {
+  private func validateRequestInfoReproucesIssue(requestInfo: RequestInfo) async throws {
     let initialReproducer = try await runReductionStep(requestInfo: requestInfo) { tree in [] }
     if initialReproducer == nil {
-      throw ReductionError("Initial request info did not crash")
+      throw ReductionError("Initial request info did not reproduce the issue")
     }
   }
 
@@ -110,7 +121,7 @@ class FileReducer {
     }
   }
 
-  // MARK: - Primitives to run reduction steps
+  // MARK: Primitives to run reduction steps
 
   func logSuccessfulReduction(_ requestInfo: RequestInfo) {
     print("Reduced source file to \(requestInfo.fileContents.utf8.count) bytes")
@@ -191,15 +202,15 @@ class FileReducer {
 
 // MARK: - Reduce functions
 
-/// See `FileReducer.runReductionStep`
-protocol StatefulReducer {
+/// See `SourceReducer.runReductionStep`
+fileprivate protocol StatefulReducer {
   func reduce(tree: SourceFileSyntax) -> [SourceEdit]
 }
 
 // MARK: Replace function bodies
 
 /// Tries replacing one function body by `fatalError()` at a time.
-class ReplaceFunctionBodiesByFatalError: StatefulReducer {
+fileprivate class ReplaceFunctionBodiesByFatalError: StatefulReducer {
   /// The function bodies that should not be replaced by `fatalError()`.
   ///
   /// When we tried replacing a function body by `fatalError`, it gets added to this list.
@@ -240,24 +251,23 @@ class ReplaceFunctionBodiesByFatalError: StatefulReducer {
       }
       if keepFunctionBodies.contains(node.statements.description.trimmingCharacters(in: .whitespacesAndNewlines)) {
         return .visitChildren
-      } else {
-        keepFunctionBodies.append(node.statements.description.trimmingCharacters(in: .whitespacesAndNewlines))
-        edits.append(
-          SourceEdit(
-            range: node.statements.position..<node.statements.endPosition,
-            replacement: "\(node.statements.leadingTrivia)fatalError()"
-          )
-        )
-        return .skipChildren
       }
+      keepFunctionBodies.append(node.statements.description.trimmingCharacters(in: .whitespacesAndNewlines))
+      edits.append(
+        SourceEdit(
+          range: node.statements.position..<node.statements.endPosition,
+          replacement: "\(node.statements.leadingTrivia)fatalError()"
+        )
+      )
+      return .skipChildren
     }
   }
 }
 
 // MARK: Remove members and code block items
 
 /// Tries removing `MemberBlockItemSyntax` and `CodeBlockItemSyntax` one at a time.
-class RemoveMembersAndCodeBlockItems: StatefulReducer {
+fileprivate class RemoveMembersAndCodeBlockItems: StatefulReducer {
   /// The code block items / members that shouldn't be removed.
   ///
   /// See `ReplaceFunctionBodiesByFatalError.keepFunctionBodies`.
@@ -323,7 +333,7 @@ class RemoveMembersAndCodeBlockItems: StatefulReducer {
 }
 
 /// Removes all comments from the source file.
-func removeComments(from tree: SourceFileSyntax) -> [SourceEdit] {
+fileprivate func removeComments(from tree: SourceFileSyntax) -> [SourceEdit] {
   class CommentRemover: SyntaxVisitor {
     var edits: [SourceEdit] = []
 
@@ -366,7 +376,7 @@ fileprivate extension TriviaPiece {
 
 // MARK: Inline first include
 
-class FirstImportFinder: SyntaxAnyVisitor {
+fileprivate class FirstImportFinder: SyntaxAnyVisitor {
   var firstImport: ImportDeclSyntax?
 
   override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
@@ -391,24 +401,18 @@ class FirstImportFinder: SyntaxAnyVisitor {
   }
 }
 
-private func getSwiftInterface(_ moduleName: String, executor: SourceKitRequestExecutor, compilerArgs: [String])
-  async throws -> String
-{
-  // FIXME: Use the sourcekitd specified on the command line once rdar://121676425 is fixed
-  let sourcekitdPath =
-    "/Applications/Geode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/sourcekitdInProc.framework/sourcekitdInProc"
-  let executor = SourceKitRequestExecutor(
-    sourcekitd: URL(fileURLWithPath: sourcekitdPath),
-    reproducerPredicate: nil
-  )
-
+fileprivate func getSwiftInterface(
+  _ moduleName: String,
+  executor: SourceKitRequestExecutor,
+  compilerArgs: [String]
+) async throws -> String {
   // We use `RequestInfo` and its template to add the compiler arguments to the request.
   let requestTemplate = """
     {
       key.request: source.request.editor.open.interface,
       key.name: "fake",
       key.compilerargs: [
-        $COMPILERARGS
+        $COMPILER_ARGS
       ],
       key.modulename: "\(moduleName)"
     }
@@ -448,7 +452,7 @@ private func getSwiftInterface(_ moduleName: String, executor: SourceKitRequestE
   return try JSONDecoder().decode(String.self, from: sanitizedData)
 }
 
-func inlineFirstImport(
+fileprivate func inlineFirstImport(
   in tree: SourceFileSyntax,
   executor: SourceKitRequestExecutor,
   compilerArgs: [String]

Sources/sourcekit-lsp/SourceKitLSP.swift

@@ -267,8 +267,9 @@ struct SourceKitLSP: AsyncParsableCommand {
       }
     )
 
-    // Park the main function by sleeping for a year.
-    // All request handling is done on other threads.
-    try await Task.sleep(for: .seconds(60 * 60 * 24 * 365))
+    // Park the main function.
+    // All request handling is done on other threads and sourcekit-lsp exits by calling `_Exit` when it receives a
+    // shutdown notification.
+    let _: Void = await withCheckedContinuation { _ in }
   }
 }