Merge pull request #1435 from ahoppen/no-sourcekitd-test-hooks

Remove sourcekitd test hooks

Author: ahoppen

Sources/Diagnose/RunSourcekitdRequestCommand.swift

@@ -55,8 +55,7 @@ public struct RunSourceKitdRequestCommand: AsyncParsableCommand {
         throw ExitCode(1)
       }
     let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
-      dylibPath: try! AbsolutePath(validating: sourcekitdPath),
-      testHooks: SourceKitDTestHooks()
+      dylibPath: try! AbsolutePath(validating: sourcekitdPath)
     )
 
     if let lineColumn = position?.split(separator: ":", maxSplits: 2).map(Int.init),

Sources/SourceKitD/DynamicallyLoadedSourceKitD.swift

@@ -26,27 +26,17 @@ extension sourcekitd_api_keys: @unchecked Sendable {}
 extension sourcekitd_api_requests: @unchecked Sendable {}
 extension sourcekitd_api_values: @unchecked Sendable {}
 
-public struct SourceKitDTestHooks: Sendable {
-  public var sourcekitdRequestDidStart: (@Sendable (SKDRequestDictionary) -> Void)?
-
-  public init(sourcekitdRequestDidStart: (@Sendable (SKDRequestDictionary) -> Void)? = nil) {
-    self.sourcekitdRequestDidStart = sourcekitdRequestDidStart
-  }
-}
-
 /// Wrapper for sourcekitd, taking care of initialization, shutdown, and notification handler
 /// multiplexing.
 ///
 /// Users of this class should not call the api functions `initialize`, `shutdown`, or
 /// `set_notification_handler`, which are global state managed internally by this class.
 public actor DynamicallyLoadedSourceKitD: SourceKitD {
   /// The path to the sourcekitd dylib.
-  private let path: AbsolutePath
+  public let path: AbsolutePath
 
   /// The handle to the dylib.
-  private let dylib: DLHandle
-
-  public let testHooks: SourceKitDTestHooks
+  let dylib: DLHandle
 
   /// The sourcekitd API functions.
   public let api: sourcekitd_api_functions_t
@@ -65,23 +55,18 @@ public actor DynamicallyLoadedSourceKitD: SourceKitD {
   /// List of notification handlers that will be called for each notification.
   private var notificationHandlers: [WeakSKDNotificationHandler] = []
 
-  /// If there is already a `sourcekitd` instance from the given return it, otherwise create a new one.
-  ///
-  /// `testHooks` are only considered when an instance is being created. If a sourcekitd instance at the given path
-  /// already exists, its test hooks will be used.
-  public static func getOrCreate(dylibPath: AbsolutePath, testHooks: SourceKitDTestHooks) async throws -> SourceKitD {
+  public static func getOrCreate(dylibPath: AbsolutePath) async throws -> SourceKitD {
     try await SourceKitDRegistry.shared
-      .getOrAdd(dylibPath, create: { try DynamicallyLoadedSourceKitD(dylib: dylibPath, testHooks: testHooks) })
+      .getOrAdd(dylibPath, create: { try DynamicallyLoadedSourceKitD(dylib: dylibPath) })
   }
 
-  init(dylib path: AbsolutePath, testHooks: SourceKitDTestHooks) throws {
+  init(dylib path: AbsolutePath) throws {
     self.path = path
     #if os(Windows)
     self.dylib = try dlopen(path.pathString, mode: [])
     #else
     self.dylib = try dlopen(path.pathString, mode: [.lazy, .local, .first])
     #endif
-    self.testHooks = testHooks
     self.api = try sourcekitd_api_functions_t(self.dylib)
     self.keys = sourcekitd_api_keys(api: self.api)
     self.requests = sourcekitd_api_requests(api: self.api)

Sources/SourceKitD/SourceKitD.swift

@@ -31,8 +31,6 @@ extension sourcekitd_api_request_handle_t: @unchecked Sendable {}
 /// *Implementors* are expected to handle initialization and shutdown, e.g. during `init` and
 /// `deinit` or by wrapping an existing sourcekitd session that outlives this object.
 public protocol SourceKitD: AnyObject, Sendable {
-  var testHooks: SourceKitDTestHooks { get }
-
   /// The sourcekitd API functions.
   var api: sourcekitd_api_functions_t { get }
 
@@ -101,8 +99,6 @@ extension SourceKitD {
   public func send(_ request: SKDRequestDictionary, fileContents: String?) async throws -> SKDResponseDictionary {
     log(request: request)
 
-    testHooks.sourcekitdRequestDidStart?(request)
-
     let sourcekitdResponse: SKDResponse = try await withCancellableCheckedThrowingContinuation { continuation in
       var handle: sourcekitd_api_request_handle_t? = nil
       api.send_request(request.dict, &handle) { response in

Sources/SourceKitLSP/SourceKitLSPServer+Options.swift

@@ -15,7 +15,6 @@ import LanguageServerProtocol
 import SKCore
 import SKSupport
 import SemanticIndex
-import SourceKitD
 
 import struct TSCBasic.AbsolutePath
 import struct TSCBasic.RelativePath
@@ -52,8 +51,6 @@ extension SourceKitLSPServer {
     /// Experimental features that are enabled.
     public var experimentalFeatures: Set<ExperimentalFeature>
 
-    public var sourcekitdTestHooks: SourceKitDTestHooks
-
     public var indexTestHooks: IndexTestHooks
 
     public init(
@@ -65,7 +62,6 @@ extension SourceKitLSPServer {
       generatedInterfacesPath: AbsolutePath = defaultDirectoryForGeneratedInterfaces,
       swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */
       experimentalFeatures: Set<ExperimentalFeature> = [],
-      sourcekitdTestHooks: SourceKitDTestHooks = SourceKitDTestHooks(),
       indexTestHooks: IndexTestHooks = IndexTestHooks()
     ) {
       self.buildSetup = buildSetup
@@ -76,7 +72,6 @@ extension SourceKitLSPServer {
       self.generatedInterfacesPath = generatedInterfacesPath
       self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration
       self.experimentalFeatures = experimentalFeatures
-      self.sourcekitdTestHooks = sourcekitdTestHooks
       self.indexTestHooks = indexTestHooks
     }
   }

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

@@ -195,10 +195,7 @@ public actor SwiftLanguageService: LanguageService, Sendable {
     guard let sourcekitd = toolchain.sourcekitd else { return nil }
     self.sourceKitLSPServer = sourceKitLSPServer
     self.swiftFormat = toolchain.swiftFormat
-    self.sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
-      dylibPath: sourcekitd,
-      testHooks: options.sourcekitdTestHooks
-    )
+    self.sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(dylibPath: sourcekitd)
     self.capabilityRegistry = workspace.capabilityRegistry
     self.semanticIndexManager = workspace.semanticIndexManager
     self.serverOptions = options

Tests/DiagnoseTests/DiagnoseTests.swift

@@ -318,8 +318,7 @@ private class InProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
     logger.info("Sending request: \(requestString)")
 
     let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
-      dylibPath: try! AbsolutePath(validating: sourcekitd.path),
-      testHooks: SourceKitDTestHooks()
+      dylibPath: try! AbsolutePath(validating: sourcekitd.path)
     )
     let response = try await sourcekitd.run(requestYaml: requestString)
 

Tests/SourceKitDTests/SourceKitDRegistryTests.swift

@@ -63,7 +63,6 @@ private nonisolated(unsafe) var nextToken = AtomicUInt32(initialValue: 0)
 
 final class FakeSourceKitD: SourceKitD {
   let token: UInt32
-  var testHooks: SourceKitDTestHooks { SourceKitDTestHooks() }
   var api: sourcekitd_api_functions_t { fatalError() }
   var keys: sourcekitd_api_keys { fatalError() }
   var requests: sourcekitd_api_requests { fatalError() }

Tests/SourceKitDTests/SourceKitDTests.swift

@@ -28,10 +28,7 @@ import class TSCBasic.Process
 final class SourceKitDTests: XCTestCase {
   func testMultipleNotificationHandlers() async throws {
     let sourcekitdPath = await ToolchainRegistry.forTesting.default!.sourcekitd!
-    let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
-      dylibPath: sourcekitdPath,
-      testHooks: SourceKitDTestHooks()
-    )
+    let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(dylibPath: sourcekitdPath)
     let keys = sourcekitd.keys
     let path = DocumentURI(for: .swift).pseudoPath
 

Tests/SourceKitLSPTests/PullDiagnosticsTests.swift

@@ -310,15 +310,10 @@ final class PullDiagnosticsTests: XCTestCase {
   }
 
   func testDontReturnEmptyDiagnosticsIfDiagnosticRequestIsCancelled() async throws {
-    let diagnosticSourcekitdRequestDidStart = self.expectation(description: "diagnostic sourcekitd request did start")
     let diagnosticRequestCancelled = self.expectation(description: "diagnostic request cancelled")
     var serverOptions = SourceKitLSPServer.Options.testDefault
-    serverOptions.sourcekitdTestHooks.sourcekitdRequestDidStart = { request in
-      guard request.description.contains("source.request.diagnostics") else {
-        return
-      }
-      diagnosticSourcekitdRequestDidStart.fulfill()
-      self.wait(for: [diagnosticRequestCancelled], timeout: defaultTimeout)
+    serverOptions.indexTestHooks.preparationTaskDidStart = { _ in
+      await self.fulfillment(of: [diagnosticRequestCancelled], timeout: defaultTimeout)
       // Poll until the `CancelRequestNotification` has been propagated to the request handling.
       for _ in 0..<Int(defaultTimeout * 100) {
         if Task.isCancelled {
@@ -331,7 +326,9 @@ final class PullDiagnosticsTests: XCTestCase {
       files: [
         "Lib.swift": "let x: String = 1"
       ],
-      serverOptions: serverOptions
+      serverOptions: serverOptions,
+      enableBackgroundIndexing: true,
+      pollIndex: false
     )
     let (uri, _) = try project.openDocument("Lib.swift")
 
@@ -342,7 +339,6 @@ final class PullDiagnosticsTests: XCTestCase {
       XCTAssertEqual(result, .failure(ResponseError.cancelled))
       diagnosticResponseReceived.fulfill()
     }
-    try await fulfillmentOfOrThrow([diagnosticSourcekitdRequestDidStart])
     project.testClient.send(CancelRequestNotification(id: requestID))
     diagnosticRequestCancelled.fulfill()
     try await fulfillmentOfOrThrow([diagnosticResponseReceived])