Merge pull request #1349 from ahoppen/address-review-comments-1322-1329

Address review comments from #1322 and #1329

Author: ahoppen

Sources/SemanticIndex/CMakeLists.txt

@@ -2,12 +2,12 @@
 add_library(SemanticIndex STATIC
   CheckedIndex.swift
   CompilerCommandLineOption.swift
-  IndexStatusManager.swift
   IndexTaskDescription.swift
   PreparationTaskDescription.swift
   SemanticIndexManager.swift
   TestHooks.swift
   UpdateIndexStoreTaskDescription.swift
+  UpToDateTracker.swift
 )
 set_target_properties(SemanticIndex PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY})

Sources/SemanticIndex/PreparationTaskDescription.swift

@@ -35,7 +35,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
   /// The build system manager that is used to get the toolchain and build settings for the files to index.
   private let buildSystemManager: BuildSystemManager
 
-  private let preparationUpToDateStatus: IndexUpToDateStatusManager<ConfiguredTarget>
+  private let preparationUpToDateTracker: UpToDateTracker<ConfiguredTarget>
 
   /// See `SemanticIndexManager.indexProcessDidProduceResult`
   private let indexProcessDidProduceResult: @Sendable (IndexProcessResult) -> Void
@@ -59,13 +59,13 @@ public struct PreparationTaskDescription: IndexTaskDescription {
   init(
     targetsToPrepare: [ConfiguredTarget],
     buildSystemManager: BuildSystemManager,
-    preparationUpToDateStatus: IndexUpToDateStatusManager<ConfiguredTarget>,
+    preparationUpToDateTracker: UpToDateTracker<ConfiguredTarget>,
     indexProcessDidProduceResult: @escaping @Sendable (IndexProcessResult) -> Void,
     testHooks: IndexTestHooks
   ) {
     self.targetsToPrepare = targetsToPrepare
     self.buildSystemManager = buildSystemManager
-    self.preparationUpToDateStatus = preparationUpToDateStatus
+    self.preparationUpToDateTracker = preparationUpToDateTracker
     self.indexProcessDidProduceResult = indexProcessDidProduceResult
     self.testHooks = testHooks
   }
@@ -76,7 +76,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
     // The last 2 digits should be sufficient to differentiate between multiple concurrently running preparation operations
     await withLoggingSubsystemAndScope(subsystem: indexLoggingSubsystem, scope: "preparation-\(id % 100)") {
       let targetsToPrepare = await targetsToPrepare.asyncFilter {
-        await !preparationUpToDateStatus.isUpToDate($0)
+        await !preparationUpToDateTracker.isUpToDate($0)
       }.sorted(by: {
         ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID)
       })
@@ -114,7 +114,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
       }
       await testHooks.preparationTaskDidFinish?(self)
       if !Task.isCancelled {
-        await preparationUpToDateStatus.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate)
+        await preparationUpToDateTracker.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate)
       }
     }
   }

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -111,9 +111,9 @@ public final actor SemanticIndexManager {
   /// ...). `nil` if no build graph is currently being generated.
   private var generateBuildGraphTask: Task<Void, Never>?
 
-  private let preparationUpToDateStatus = IndexUpToDateStatusManager<ConfiguredTarget>()
+  private let preparationUpToDateTracker = UpToDateTracker<ConfiguredTarget>()
 
-  private let indexStoreUpToDateStatus = IndexUpToDateStatusManager<DocumentURI>()
+  private let indexStoreUpToDateTracker = UpToDateTracker<DocumentURI>()
 
   /// The preparation tasks that have been started and are either scheduled in the task scheduler or currently
   /// executing.
@@ -305,23 +305,23 @@ public final actor SemanticIndexManager {
     // We only re-index the files that were changed and don't re-index any of their dependencies. See the
     // `Documentation/Files_To_Reindex.md` file.
     let changedFiles = events.map(\.uri)
-    await indexStoreUpToDateStatus.markOutOfDate(changedFiles)
+    await indexStoreUpToDateTracker.markOutOfDate(changedFiles)
 
     // Note that configured targets are the right abstraction layer here (instead of a non-configured target) because a
     // build system might have targets that include different source files. Hence a source file might be in target T
     // configured for macOS but not in target T configured for iOS.
     let targets = await changedFiles.asyncMap { await buildSystemManager.configuredTargets(for: $0) }.flatMap { $0 }
     if let dependentTargets = await buildSystemManager.targets(dependingOn: targets) {
-      await preparationUpToDateStatus.markOutOfDate(dependentTargets)
+      await preparationUpToDateTracker.markOutOfDate(dependentTargets)
     } else {
-      await preparationUpToDateStatus.markAllOutOfDate()
+      await preparationUpToDateTracker.markAllKnownOutOfDate()
       // `markAllOutOfDate` only marks targets out-of-date that have been indexed before. Also mark all targets with
       // in-progress preparation out of date. So we don't get into the following situation, which would result in an
       // incorrect up-to-date status of a target
       //  - Target preparation starts for the first time
       //  - Files changed
       //  - Target preparation finishes.
-      await preparationUpToDateStatus.markOutOfDate(inProgressPreparationTasks.keys)
+      await preparationUpToDateTracker.markOutOfDate(inProgressPreparationTasks.keys)
     }
 
     await scheduleBackgroundIndex(files: changedFiles)
@@ -406,7 +406,7 @@ public final actor SemanticIndexManager {
     // schedule two preparations of the same target in quick succession, only the first one actually performs a prepare
     // and the second one will be a no-op once it runs.
     let targetsToPrepare = await targets.asyncFilter {
-      await !preparationUpToDateStatus.isUpToDate($0)
+      await !preparationUpToDateTracker.isUpToDate($0)
     }
 
     guard !targetsToPrepare.isEmpty else {
@@ -417,7 +417,7 @@ public final actor SemanticIndexManager {
       PreparationTaskDescription(
         targetsToPrepare: targetsToPrepare,
         buildSystemManager: self.buildSystemManager,
-        preparationUpToDateStatus: preparationUpToDateStatus,
+        preparationUpToDateTracker: preparationUpToDateTracker,
         indexProcessDidProduceResult: indexProcessDidProduceResult,
         testHooks: testHooks
       )
@@ -464,7 +464,7 @@ public final actor SemanticIndexManager {
         filesToIndex: filesAndTargets,
         buildSystemManager: self.buildSystemManager,
         index: index,
-        indexStoreUpToDateStatus: indexStoreUpToDateStatus,
+        indexStoreUpToDateTracker: indexStoreUpToDateTracker,
         indexProcessDidProduceResult: indexProcessDidProduceResult,
         testHooks: testHooks
       )
@@ -509,7 +509,7 @@ public final actor SemanticIndexManager {
     // schedule two indexing jobs for the same file in quick succession, only the first one actually updates the index
     // store and the second one will be a no-op once it runs.
     let outOfDateFiles = await filesToIndex(toCover: files).asyncFilter {
-      if await indexStoreUpToDateStatus.isUpToDate($0.sourceFile) {
+      if await indexStoreUpToDateTracker.isUpToDate($0.sourceFile) {
         return false
       }
       guard let language = await buildSystemManager.defaultLanguage(for: $0.mainFile),

Sources/SemanticIndex/IndexStatusManager.swift renamed to Sources/SemanticIndex/UpToDateTracker.swift

@@ -14,7 +14,7 @@ import Foundation
 import SKCore
 
 /// Keeps track of whether an item (a target or file to index) is up-to-date.
-actor IndexUpToDateStatusManager<Item: Hashable> {
+actor UpToDateTracker<Item: Hashable> {
   private enum Status {
     /// The item is up-to-date.
     case upToDate
@@ -57,7 +57,7 @@ actor IndexUpToDateStatusManager<Item: Hashable> {
     }
   }
 
-  func markAllOutOfDate() {
+  func markAllKnownOutOfDate() {
     markOutOfDate(status.keys)
   }
 

Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift

@@ -105,7 +105,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
   /// The build system manager that is used to get the toolchain and build settings for the files to index.
   private let buildSystemManager: BuildSystemManager
 
-  private let indexStoreUpToDateStatus: IndexUpToDateStatusManager<DocumentURI>
+  private let indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>
 
   /// A reference to the underlying index store. Used to check if the index is already up-to-date for a file, in which
   /// case we don't need to index it again.
@@ -138,14 +138,14 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
     filesToIndex: [FileAndTarget],
     buildSystemManager: BuildSystemManager,
     index: UncheckedIndex,
-    indexStoreUpToDateStatus: IndexUpToDateStatusManager<DocumentURI>,
+    indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>,
     indexProcessDidProduceResult: @escaping @Sendable (IndexProcessResult) -> Void,
     testHooks: IndexTestHooks
   ) {
     self.filesToIndex = filesToIndex
     self.buildSystemManager = buildSystemManager
     self.index = index
-    self.indexStoreUpToDateStatus = indexStoreUpToDateStatus
+    self.indexStoreUpToDateTracker = indexStoreUpToDateTracker
     self.indexProcessDidProduceResult = indexProcessDidProduceResult
     self.testHooks = testHooks
   }
@@ -202,7 +202,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
   }
 
   private func updateIndexStore(forSingleFile file: FileToIndex, in target: ConfiguredTarget) async {
-    guard await !indexStoreUpToDateStatus.isUpToDate(file.sourceFile) else {
+    guard await !indexStoreUpToDateTracker.isUpToDate(file.sourceFile) else {
       // If we know that the file is up-to-date without having ot hit the index, do that because it's fastest.
       return
     }
@@ -264,7 +264,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
         "Not updating index store for \(file) because it is a language that is not supported by background indexing"
       )
     }
-    await indexStoreUpToDateStatus.markUpToDate([file.sourceFile], updateOperationStartDate: startDate)
+    await indexStoreUpToDateTracker.markUpToDate([file.sourceFile], updateOperationStartDate: startDate)
   }
 
   private func updateIndexStore(

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -834,7 +834,7 @@ extension SourceKitLSPServer {
   nonisolated func indexTaskDidProduceResult(_ result: IndexProcessResult) {
     self.sendNotificationToClient(
       LogMessageNotification(
-        type: result.failed ? .info : .warning,
+        type: result.failed ? .warning : .info,
         message: """
           \(result.taskDescription) finished in \(result.duration)
           \(result.command)

Sources/sourcekit-lsp/SourceKitLSP.swift

@@ -208,8 +208,8 @@ struct SourceKitLSP: AsyncParsableCommand {
 
   @Flag(
     help: """
-      Show which index tasks are currently running in the indexing work done progress. \
-      This produces a multi-line work done progress, which might render incorrectly depending in the editor.
+      When reporting index progress, show the currently running index tasks in addition to the task's count. \
+      This produces a multi-line work done progress, which might render incorrectly, depending on the editor.
       """
   )
   var experimentalShowActivePreparationTasksInProgress = false