Remove tracking of file build settings status in SourceKitServer and BuildSystemManager

The core idea here is that the toolchain language servers always call into `BuildSystemManager` and `BuildSystemManager` will always reply with build settings. If it hasn’t computed them yet, it will reply with fallback settings.

With that assumption, we can remove the `documentToPendingQueue` from `SourceKitServer` since there are no longer any documents that are pending – everything has a build settings immediately.

Similarly, `BuildSystemManager.mainFileStatuses` also isn’t needed anymore.

And lastly, since we know that `BuildSystemManager.buildSettings` will always return a value `registerForChangeNotifications` is changed not call `fileBuildSettingsChanged` immediately. Instead, it will only cause `fileBuildSettingsChanged` to be called when the file’s build settings change after the `registerForChangeNotifications` call.

Author: ahoppen

Sources/SKCore/BuildSystemManager.swift

@@ -89,9 +89,6 @@ public actor BuildSystemManager {
   /// The set of watched files, along with their main file and language.
   var watchedFiles: [DocumentURI: (mainFile: DocumentURI, language: Language)] = [:]
 
-  /// Statuses for each main file, containing build settings from the build systems.
-  var mainFileStatuses: [DocumentURI: MainFileStatus] = [:]
-
   /// The underlying primary build system.
   let buildSystem: BuildSystem?
 
@@ -201,11 +198,8 @@ extension BuildSystemManager {
       self.watchedFiles[uri] = (mainFile, language)
     }
 
-    let newStatus = await self.cachedStatusOrRegisterForSettings(for: mainFile, language: language)
-
-    if newStatus.buildSettingsChange != nil, let delegate = self._delegate {
-      await delegate.fileBuildSettingsChanged([uri])
-    }
+    await buildSystem?.registerForChangeNotifications(for: mainFile, language: language)
+    fallbackBuildSystem?.registerForChangeNotifications(for: mainFile, language: language)
   }
 
   /// Return settings for `file` based on  the `change` settings for `mainFile`.
@@ -226,115 +220,6 @@ extension BuildSystemManager {
     }
   }
 
-  /// Handle a request for `FileBuildSettings` on `mainFile`.
-  ///
-  /// Updates and returns the new `MainFileStatus` for `mainFile`.
-  func cachedStatusOrRegisterForSettings(
-    for mainFile: DocumentURI,
-    language: Language
-  ) async -> MainFileStatus {
-    // If we already have a status for the main file, use that.
-    // Don't update any existing timeout.
-    if let status = self.mainFileStatuses[mainFile] {
-      return status
-    }
-    // This is a new `mainFile` that we need to handle. We need to fetch the
-    // build settings.
-    let newStatus: MainFileStatus
-    if let buildSystem = self.buildSystem {
-      // Register the timeout if it's applicable.
-      if let fallback = self.fallbackBuildSystem {
-        Task {
-          try await Task.sleep(nanoseconds: UInt64(self.fallbackSettingsTimeout.nanoseconds()!))
-          await self.handleFallbackTimer(for: mainFile, language: language, fallback)
-        }
-      }
-
-      // Intentionally register with the `BuildSystem` after setting the fallback to allow for
-      // testing of the fallback system triggering before the `BuildSystem` can reply (e.g. if a
-      // fallback time of 0 is specified).
-      await buildSystem.registerForChangeNotifications(for: mainFile, language: language)
-
-
-      newStatus = .waiting
-    } else if let fallback = self.fallbackBuildSystem {
-      // Only have a fallback build system. We consider it be a primary build
-      // system that functions synchronously.
-      if let settings = fallback.buildSettings(for: mainFile, language: language) {
-        newStatus = .primary(settings)
-      } else {
-        newStatus = .unsupported
-      }
-    } else {  // Don't have any build systems.
-      newStatus = .unsupported
-    }
-
-    if let status = self.mainFileStatuses[mainFile] {
-      // Since we await above, another call to `cachedStatusOrRegisterForSettings`
-      // might have set the main file status of `mainFile`. If this race happened,
-      // return the value set by the concurrently executing function. This is safe
-      // since all calls from this function are either side-effect free or
-      // idempotent.
-      return status
-    }
-
-    self.mainFileStatuses[mainFile] = newStatus
-    return newStatus
-  }
-
-  /// Update and notify our delegate for the given main file changes if they are
-  /// convertible into `FileBuildSettingsChange`.
-  func updateAndNotifyStatuses(changes: [DocumentURI: MainFileStatus]) async {
-    var changedWatchedFiles = Set<DocumentURI>()
-    for (mainFile, status) in changes {
-      let watches = self.watchedFiles.filter { $1.mainFile == mainFile }
-      guard !watches.isEmpty else {
-        // Possible notification after the file was unregistered. Ignore.
-        continue
-      }
-      let prevStatus = self.mainFileStatuses[mainFile]
-      self.mainFileStatuses[mainFile] = status
-
-      // It's possible that the command line arguments didn't change
-      // (waitingFallback --> fallback), in that case we don't need to report a change.
-      // If we were waiting though, we need to emit an initial change.
-      guard prevStatus == .waiting || status.buildSettings != prevStatus?.buildSettings else {
-        continue
-      }
-      guard status.buildSettingsChange != nil else {
-        continue
-      }
-      for watch in watches {
-        changedWatchedFiles.insert(watch.key)
-      }
-    }
-
-    if !changedWatchedFiles.isEmpty, let delegate = self._delegate {
-      await delegate.fileBuildSettingsChanged(changedWatchedFiles)
-    }
-  }
-
-  /// Handle the fallback timer firing for a given `mainFile`.
-  ///
-  /// Since this doesn't occur immediately it's possible that the `mainFile` is
-  /// no longer referenced or is referenced by multiple watched files.
-  func handleFallbackTimer(
-    for mainFile: DocumentURI,
-    language: Language,
-    _ fallback: FallbackBuildSystem
-  ) async {
-    // There won't be a current status if it's unreferenced by any watched file.
-    // Similarly, if the status isn't `waiting` then there's nothing to do.
-    guard let status = self.mainFileStatuses[mainFile], status == .waiting else {
-      return
-    }
-    if let settings = fallback.buildSettings(for: mainFile, language: language) {
-      await self.updateAndNotifyStatuses(changes: [mainFile: .waitingUsingFallback(settings)])
-    } else {
-      // Keep the status as waiting.
-    }
-  }
-
   public func unregisterForChangeNotifications(for uri: DocumentURI) async {
     guard let mainFile = self.watchedFiles[uri]?.mainFile else {
       log("Unbalanced calls for registerForChangeNotifications and unregisterForChangeNotifications", level: .warning)
@@ -350,7 +235,6 @@ extension BuildSystemManager {
     if !self.watchedFiles.values.lazy.map({ $0.mainFile }).contains(mainFile) {
       // This was the last reference to the main file. Remove it.
       await self.buildSystem?.unregisterForChangeNotifications(for: mainFile)
-      self.mainFileStatuses[mainFile] = nil
     }
   }
 
@@ -370,35 +254,22 @@ extension BuildSystemManager: BuildSystemDelegate {
     }
   }
 
-  public func fileBuildSettingsChangedImpl(_ changes: Set<DocumentURI>) async {
-    var statusChanges: [DocumentURI: MainFileStatus] = [:]
-    for mainFile in changes {
+  public func fileBuildSettingsChangedImpl(_ changedFiles: Set<DocumentURI>) async {
+    var changedWatchedFiles = Set<DocumentURI>()
+    for mainFile in changedFiles {
       let watches = self.watchedFiles.filter { $1.mainFile == mainFile }
-      guard let firstWatch = watches.first else {
+      guard !watches.isEmpty else {
         // Possible notification after the file was unregistered. Ignore.
         continue
       }
-      let newStatus: MainFileStatus
-
-      let settings = await self.buildSettings(for: mainFile, language: firstWatch.value.language)
-
-      if let settings {
-        newStatus = settings.isFallback ? .fallback(settings.buildSettings) : .primary(settings.buildSettings)
-      } else if let fallback = self.fallbackBuildSystem {
-        // FIXME: we need to stop threading the language everywhere, or we need the build system
-        // itself to pass it in here. Or alternatively cache the fallback settings/language earlier?
-        let language = firstWatch.value.language
-        if let settings = fallback.buildSettings(for: mainFile, language: language) {
-          newStatus = .fallback(settings)
-        } else {
-          newStatus = .unsupported
-        }
-      } else {
-        newStatus = .unsupported
+      for watch in watches {
+        changedWatchedFiles.insert(watch.key)
       }
-      statusChanges[mainFile] = newStatus
     }
-    await self.updateAndNotifyStatuses(changes: statusChanges)
+
+    if !changedWatchedFiles.isEmpty, let delegate = self._delegate {
+      await delegate.fileBuildSettingsChanged(changedWatchedFiles)
+    }
   }
 
   // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has ben asyncified
@@ -477,11 +348,7 @@ extension BuildSystemManager: MainFilesDelegate {
         log("main file for '\(uri)' changed old: '\(state.mainFile)' -> new: '\(newMainFile)'", level: .info)
         await self.checkUnreferencedMainFile(state.mainFile)
 
-        let newStatus = await self.cachedStatusOrRegisterForSettings(
-            for: newMainFile, language: language)
-        if newStatus.buildSettingsChange != nil {
-          buildSettingsChanges.insert(uri)
-        }
+        buildSettingsChanges.insert(uri)
       }
     }
 
@@ -497,11 +364,6 @@ extension BuildSystemManager {
   public func _cachedMainFile(for uri: DocumentURI) -> DocumentURI? {
     watchedFiles[uri]?.mainFile
   }
-
-  /// *For Testing* Returns the main file used for `uri`, if this is a registered file.
-  public func _cachedMainFileSettings(for uri: DocumentURI) -> FileBuildSettings?? {
-    mainFileStatuses[uri]?.buildSettings
-  }
 }
 
 /// Choose a new main file for the given uri, preferring to use a previous main file if still

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -93,10 +93,6 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
     self.watchedFiles[uri] = language
-
-    guard let delegate = self.delegate else { return }
-
-    await delegate.fileBuildSettingsChanged([uri])
   }
 
   /// We don't support change watching.

Sources/SKCore/FallbackBuildSystem.swift

@@ -60,11 +60,7 @@ public final class FallbackBuildSystem: BuildSystem {
   }
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
-    guard let delegate = self.delegate else { return }
-
-    Task {
-      await delegate.fileBuildSettingsChanged([uri])
-    }
+    // Fallback build systems never change.
   }
 
   /// We don't support change watching.

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

@@ -301,16 +301,7 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
     assert(self.watchedFiles[uri] == nil, "Registered twice for change notifications of the same URI")
-    guard let delegate = self.delegate else { return }
     self.watchedFiles[uri] = language
-
-    var settings: FileBuildSettings? = nil
-    do {
-      settings = try self.buildSettings(for: uri, language: language)
-    } catch {
-      log("error computing settings: \(error)")
-    }
-    await delegate.fileBuildSettingsChanged([uri])
   }
 
   /// Unregister the given file for build-system level change notifications, such as command