Don't increment in-progress task counter for files that cannot be indexed

We had a bug where we would add an entry to `inProgressIndexTasks` for files that we couldn’t actually index, so we never actually scheduled an index task for them, which means that the files were never removed from `inProgressIndexTasks` and thus caused the indexing progress to never finish.

Author: ahoppen

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -711,18 +711,13 @@ package final actor SemanticIndexManager {
       // sort files to get deterministic indexing order
       .sorted(by: { $0.file.sourceFile.stringValue < $1.file.sourceFile.stringValue })
 
-    // The number of index tasks that don't currently have an in-progress task associated with it.
-    // The denominator in the index progress should get incremented by this amount.
-    // We don't want to increment the denominator for tasks that already have an index in progress.
-    var newIndexTasks = 0
     var alreadyScheduledTasks: Set<FileToIndex> = []
     for file in filesToIndex {
       let inProgress = inProgressIndexTasks[file.file]
 
       let shouldScheduleIndexing: Bool
       switch inProgress?.state {
       case nil:
-        newIndexTasks += 1
         shouldScheduleIndexing = true
       case .creatingIndexTask, .waitingForPreparation:
         // We already have a task that indexes the file but hasn't started preparation yet. Indexing the file again
@@ -739,18 +734,11 @@ package final actor SemanticIndexManager {
         }
       }
       if shouldScheduleIndexing {
-        inProgressIndexTasks[file.file] = InProgressIndexStore(
-          state: .creatingIndexTask,
-          fileModificationDate: file.fileModificationDate
-        )
+
       } else {
         alreadyScheduledTasks.insert(file.file)
-
       }
     }
-    if newIndexTasks > 0 {
-      indexTasksWereScheduled(newIndexTasks)
-    }
     filesToIndex = filesToIndex.filter { !alreadyScheduledTasks.contains($0.file) }
 
     if filesToIndex.isEmpty {
@@ -764,7 +752,12 @@ package final actor SemanticIndexManager {
     // to index the low-level targets ASAP.
     var filesByTarget: [BuildTargetIdentifier: [(FileToIndex)]] = [:]
 
-    for fileToIndex in filesToIndex.map(\.file) {
+    // The number of index tasks that don't currently have an in-progress task associated with it.
+    // The denominator in the index progress should get incremented by this amount.
+    // We don't want to increment the denominator for tasks that already have an index in progress.
+    var newIndexTasks = 0
+
+    for (fileToIndex, fileModificationDate) in filesToIndex {
       guard let target = await buildSystemManager.canonicalTarget(for: fileToIndex.mainFile) else {
         logger.error(
           "Not indexing \(fileToIndex.forLogging) because the target could not be determined"
@@ -777,8 +770,21 @@ package final actor SemanticIndexManager {
         continue
       }
 
+      if inProgressIndexTasks[fileToIndex] == nil {
+        // If `inProgressIndexTasks[fileToIndex]` is not `nil`, this new index task is replacing another index task.
+        // We are thus not indexing a new file and thus shouldn't increment the denominator of the indexing status.
+        newIndexTasks += 1
+      }
+      inProgressIndexTasks[fileToIndex] = InProgressIndexStore(
+        state: .creatingIndexTask,
+        fileModificationDate: fileModificationDate
+      )
+
       filesByTarget[target, default: []].append(fileToIndex)
     }
+    if newIndexTasks > 0 {
+      indexTasksWereScheduled(newIndexTasks)
+    }
 
     // The targets sorted in reverse topological order, low-level targets before high-level targets. If topological
     // sorting fails, sorted in another deterministic way where the actual order doesn't matter.

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -2176,6 +2176,55 @@ final class BackgroundIndexingTests: XCTestCase {
       ]
     )
   }
+
+  func testIndexingProgressIfNonIndexableFileIsInPackage() async throws {
+    let receivedReportProgressNotification = AtomicBool(initialValue: false)
+
+    let project = try await SwiftPMTestProject(
+      files: [
+        "MyLibrary/include/Test.h": "",
+        "MyLibrary/Test.c": "",
+        "MyLibrary/Assembly.S": "",
+      ],
+      capabilities: ClientCapabilities(window: WindowClientCapabilities(workDoneProgress: true)),
+      hooks: Hooks(
+        indexHooks: IndexHooks(updateIndexStoreTaskDidFinish: { _ in
+          while !receivedReportProgressNotification.value {
+            try? await Task.sleep(for: .milliseconds(10))
+          }
+        })
+      ),
+      enableBackgroundIndexing: true,
+      pollIndex: false,
+      preInitialization: { testClient in
+        testClient.handleMultipleRequests { (request: CreateWorkDoneProgressRequest) in
+          return VoidResponse()
+        }
+      }
+    )
+
+    let beginNotification = try await project.testClient.nextNotification(
+      ofType: WorkDoneProgress.self,
+      satisfying: { notification in
+        guard case .begin(let data) = notification.value else {
+          return false
+        }
+        return data.title == "Indexing"
+      }
+    )
+    receivedReportProgressNotification.value = true
+
+    // Check that we receive an `end` notification
+    _ = try await project.testClient.nextNotification(
+      ofType: WorkDoneProgress.self,
+      satisfying: { notification in
+        if notification.token == beginNotification.token, case .end = notification.value {
+          return true
+        }
+        return false
+      }
+    )
+  }
 }
 
 extension HoverResponseContents {