Fix a negation issue that prevented parallel indexing

Author: ahoppen

Sources/SemanticIndex/PreparationTaskDescription.swift

@@ -78,7 +78,6 @@ public struct PreparationTaskDescription: IndexTaskDescription {
       subsystem: "org.swift.sourcekit-lsp.indexing",
       scope: "preparation-\(id % 100)"
     ) {
-      await testHooks.preparationTaskDidStart?(self)
       let targetsToPrepare = await targetsToPrepare.asyncFilter {
         await !preparationUpToDateStatus.isUpToDate($0)
       }.sorted(by: {
@@ -87,6 +86,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
       if targetsToPrepare.isEmpty {
         return
       }
+      await testHooks.preparationTaskDidStart?(self)
 
       let targetsToPrepareDescription =
         targetsToPrepare

Sources/SemanticIndex/TestHooks.swift

@@ -16,16 +16,20 @@ public struct IndexTestHooks: Sendable {
 
   public var preparationTaskDidFinish: (@Sendable (PreparationTaskDescription) async -> Void)?
 
+  public var updateIndexStoreTaskDidStart: (@Sendable (UpdateIndexStoreTaskDescription) async -> Void)?
+
   /// A callback that is called when an index task finishes.
   public var updateIndexStoreTaskDidFinish: (@Sendable (UpdateIndexStoreTaskDescription) async -> Void)?
 
   public init(
     preparationTaskDidStart: (@Sendable (PreparationTaskDescription) async -> Void)? = nil,
     preparationTaskDidFinish: (@Sendable (PreparationTaskDescription) async -> Void)? = nil,
+    updateIndexStoreTaskDidStart: (@Sendable (UpdateIndexStoreTaskDescription) async -> Void)? = nil,
     updateIndexStoreTaskDidFinish: (@Sendable (UpdateIndexStoreTaskDescription) async -> Void)? = nil
   ) {
     self.preparationTaskDidStart = preparationTaskDidStart
     self.preparationTaskDidFinish = preparationTaskDidFinish
+    self.updateIndexStoreTaskDidStart = updateIndexStoreTaskDidStart
     self.updateIndexStoreTaskDidFinish = updateIndexStoreTaskDidFinish
   }
 }

Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift

@@ -22,7 +22,7 @@ import class TSCBasic.Process
 
 private nonisolated(unsafe) var updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1)
 
-enum FileToIndex: CustomLogStringConvertible {
+public enum FileToIndex: CustomLogStringConvertible {
   /// A non-header file
   case indexableFile(DocumentURI)
 
@@ -33,7 +33,7 @@ enum FileToIndex: CustomLogStringConvertible {
   ///
   /// This file might be a header file that doesn't have build settings associated with it. For the actual compiler
   /// invocation that updates the index store, the `mainFile` should be used.
-  var sourceFile: DocumentURI {
+  public var sourceFile: DocumentURI {
     switch self {
     case .indexableFile(let uri): return uri
     case .headerFile(header: let header, mainFile: _): return header
@@ -51,7 +51,7 @@ enum FileToIndex: CustomLogStringConvertible {
     }
   }
 
-  var description: String {
+  public var description: String {
     switch self {
     case .indexableFile(let uri):
       return uri.description
@@ -60,7 +60,7 @@ enum FileToIndex: CustomLogStringConvertible {
     }
   }
 
-  var redactedDescription: String {
+  public var redactedDescription: String {
     switch self {
     case .indexableFile(let uri):
       return uri.redactedDescription
@@ -71,9 +71,9 @@ enum FileToIndex: CustomLogStringConvertible {
 }
 
 /// A file to index and the target in which the file should be indexed.
-struct FileAndTarget {
-  let file: FileToIndex
-  let target: ConfiguredTarget
+public struct FileAndTarget: Sendable {
+  public let file: FileToIndex
+  public let target: ConfiguredTarget
 }
 
 /// Describes a task to index a set of source files.
@@ -84,7 +84,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
   public let id = updateIndexStoreIDForLogging.fetchAndIncrement()
 
   /// The files that should be indexed.
-  private let filesToIndex: [FileAndTarget]
+  public let filesToIndex: [FileAndTarget]
 
   /// The build system manager that is used to get the toolchain and build settings for the files to index.
   private let buildSystemManager: BuildSystemManager
@@ -140,6 +140,8 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
     ) {
       let startDate = Date()
 
+      await testHooks.updateIndexStoreTaskDidStart?(self)
+
       let filesToIndexDescription = filesToIndex.map {
         $0.file.sourceFile.fileURL?.lastPathComponent ?? $0.file.sourceFile.stringValue
       }
@@ -166,9 +168,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
   ) -> [TaskDependencyAction<UpdateIndexStoreTaskDescription>] {
     let selfMainFiles = Set(filesToIndex.map(\.file.mainFile))
     return currentlyExecutingTasks.compactMap { (other) -> TaskDependencyAction<UpdateIndexStoreTaskDescription>? in
-      guard
-        !other.filesToIndex.lazy.map(\.file.mainFile).contains(where: { selfMainFiles.contains($0) })
-      else {
+      if !other.filesToIndex.lazy.map(\.file.mainFile).contains(where: { selfMainFiles.contains($0) }) {
         // Disjoint sets of files can be indexed concurrently.
         return nil
       }

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -22,109 +22,6 @@ fileprivate let backgroundIndexingOptions = SourceKitLSPServer.Options(
   indexOptions: IndexOptions(enableBackgroundIndexing: true)
 )
 
-fileprivate struct ExpectedPreparation {
-  let targetID: String
-  let runDestinationID: String
-
-  /// A closure that will be executed when a preparation task starts.
-  /// This allows the artificial delay of a preparation task to force two preparation task to race.
-  let didStart: (() -> Void)?
-
-  /// A closure that will be executed when a preparation task finishes.
-  /// This allows the artificial delay of a preparation task to force two preparation task to race.
-  let didFinish: (() -> Void)?
-
-  internal init(
-    targetID: String,
-    runDestinationID: String,
-    didStart: (() -> Void)? = nil,
-    didFinish: (() -> Void)? = nil
-  ) {
-    self.targetID = targetID
-    self.runDestinationID = runDestinationID
-    self.didStart = didStart
-    self.didFinish = didFinish
-  }
-
-  var configuredTarget: ConfiguredTarget {
-    return ConfiguredTarget(targetID: targetID, runDestinationID: runDestinationID)
-  }
-}
-
-fileprivate actor ExpectedPreparationTracker {
-  /// The targets we expect to be prepared. For targets within the same set, we don't care about the exact order.
-  private var expectedPreparations: [[ExpectedPreparation]]
-
-  /// Implicitly-unwrapped optional so we can reference `self` when creating `IndexTestHooks`.
-  /// `nonisolated(unsafe)` is fine because this is not modified after `testHooks` is created.
-  nonisolated(unsafe) var testHooks: IndexTestHooks!
-
-  init(expectedPreparations: [[ExpectedPreparation]]) {
-    self.expectedPreparations = expectedPreparations
-    self.testHooks = IndexTestHooks(
-      preparationTaskDidStart: { [weak self] in
-        await self?.preparationTaskDidStart(taskDescription: $0)
-      },
-      preparationTaskDidFinish: { [weak self] in
-        await self?.preparationTaskDidFinish(taskDescription: $0)
-      }
-    )
-  }
-
-  func preparationTaskDidStart(taskDescription: PreparationTaskDescription) -> Void {
-    if Task.isCancelled {
-      return
-    }
-    guard let expectedTargetsToPrepare = expectedPreparations.first else {
-      return
-    }
-    for expectedPreparation in expectedTargetsToPrepare {
-      if taskDescription.targetsToPrepare.contains(expectedPreparation.configuredTarget) {
-        expectedPreparation.didStart?()
-      }
-    }
-  }
-
-  func preparationTaskDidFinish(taskDescription: PreparationTaskDescription) -> Void {
-    if Task.isCancelled {
-      return
-    }
-    guard let expectedTargetsToPrepare = expectedPreparations.first else {
-      XCTFail("Didn't expect a preparation but received \(taskDescription.targetsToPrepare)")
-      return
-    }
-    guard Set(taskDescription.targetsToPrepare).isSubset(of: expectedTargetsToPrepare.map(\.configuredTarget)) else {
-      XCTFail("Received unexpected preparation of \(taskDescription.targetsToPrepare)")
-      return
-    }
-    var remainingExpectedTargetsToPrepare: [ExpectedPreparation] = []
-    for expectedPreparation in expectedTargetsToPrepare {
-      if taskDescription.targetsToPrepare.contains(expectedPreparation.configuredTarget) {
-        expectedPreparation.didFinish?()
-      } else {
-        remainingExpectedTargetsToPrepare.append(expectedPreparation)
-      }
-    }
-    if remainingExpectedTargetsToPrepare.isEmpty {
-      expectedPreparations.remove(at: 0)
-    } else {
-      expectedPreparations[0] = remainingExpectedTargetsToPrepare
-    }
-  }
-
-  nonisolated func keepAlive() {
-    withExtendedLifetime(self) { _ in }
-  }
-
-  deinit {
-    let expectedPreparations = self.expectedPreparations
-    XCTAssert(
-      expectedPreparations.isEmpty,
-      "ExpectedPreparationTracker destroyed with unfulfilled expected preparations: \(expectedPreparations)."
-    )
-  }
-}
-
 final class BackgroundIndexingTests: XCTestCase {
   func testBackgroundIndexingOfSingleFile() async throws {
     let project = try await SwiftPMTestProject(
@@ -614,7 +511,7 @@ final class BackgroundIndexingTests: XCTestCase {
   func testPrepareTargetAfterEditToDependency() async throws {
     try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     var serverOptions = backgroundIndexingOptions
-    let expectedPreparationTracker = ExpectedPreparationTracker(expectedPreparations: [
+    let expectedPreparationTracker = ExpectedIndexTaskTracker(expectedPreparations: [
       [
         ExpectedPreparation(targetID: "LibA", runDestinationID: "dummy"),
         ExpectedPreparation(targetID: "LibB", runDestinationID: "dummy"),
@@ -706,7 +603,7 @@ final class BackgroundIndexingTests: XCTestCase {
 
     try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     var serverOptions = backgroundIndexingOptions
-    let expectedPreparationTracker = ExpectedPreparationTracker(expectedPreparations: [
+    let expectedPreparationTracker = ExpectedIndexTaskTracker(expectedPreparations: [
       // Preparation of targets during the initial of the target
       [
         ExpectedPreparation(targetID: "LibA", runDestinationID: "dummy"),
@@ -799,4 +696,47 @@ final class BackgroundIndexingTests: XCTestCase {
       "\(indexFileNotification.message) does not have the expected prefix"
     )
   }
+
+  func testPreparationHappensInParallel() async throws {
+    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
+
+    let fileAIndexingStarted = self.expectation(description: "FileA indexing started")
+    let fileBIndexingStarted = self.expectation(description: "FileB indexing started")
+
+    var serverOptions = backgroundIndexingOptions
+    let expectedIndexTaskTracker = ExpectedIndexTaskTracker(
+      expectedIndexStoreUpdates: [
+        [
+          ExpectedIndexStoreUpdate(
+            sourceFileName: "FileA.swift",
+            didStart: {
+              fileAIndexingStarted.fulfill()
+            },
+            didFinish: {
+              self.wait(for: [fileBIndexingStarted], timeout: defaultTimeout)
+            }
+          ),
+          ExpectedIndexStoreUpdate(
+            sourceFileName: "FileB.swift",
+            didStart: {
+              fileBIndexingStarted.fulfill()
+            },
+            didFinish: {
+              self.wait(for: [fileAIndexingStarted], timeout: defaultTimeout)
+            }
+          ),
+        ]
+      ]
+    )
+    serverOptions.indexTestHooks = expectedIndexTaskTracker.testHooks
+
+    _ = try await SwiftPMTestProject(
+      files: [
+        "FileA.swift": "",
+        "FileB.swift": "",
+      ],
+      serverOptions: serverOptions,
+      cleanUp: { expectedIndexTaskTracker.keepAlive() }
+    )
+  }
 }