Merge pull request #949 from ahoppen/ahoppen/fix-unbalanced-register-unregister-crash

Remove assertion that we can't register for change notifications twice in `SwiftPMWorkspace`

Author: ahoppen

Sources/SKCore/BuildServerBuildSystem.swift

@@ -256,7 +256,7 @@ extension BuildServerBuildSystem: BuildSystem {
     return buildSettings[document]
   }
 
-  public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
+  public func registerForChangeNotifications(for uri: DocumentURI) {
     let request = RegisterForChanges(uri: uri, action: .register)
     _ = self.buildServer?.send(request) { result in
       if let error = result.failure {

Sources/SKCore/BuildSystem.swift

@@ -70,7 +70,7 @@ public protocol BuildSystem: AnyObject {
   /// IMPORTANT: When first receiving a register request, the `BuildSystem` MUST asynchronously
   /// inform its delegate of any initial settings for the given file via the
   /// `fileBuildSettingsChanged` method, even if unavailable.
-  func registerForChangeNotifications(for: DocumentURI, language: Language) async
+  func registerForChangeNotifications(for: DocumentURI) async
 
   /// Unregister the given file for build-system level change notifications,
   /// such as command line flag changes, dependency changes, etc.

Sources/SKCore/BuildSystemManager.swift

@@ -154,7 +154,7 @@ extension BuildSystemManager {
     // system. That way, iff the main file changes, we will also notify the
     // delegate about build setting changes of all header files that are based
     // on that main file.
-    await buildSystem?.registerForChangeNotifications(for: mainFile, language: language)
+    await buildSystem?.registerForChangeNotifications(for: mainFile)
   }
 
   public func unregisterForChangeNotifications(for uri: DocumentURI) async {

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -51,7 +51,7 @@ public actor CompilationDatabaseBuildSystem {
 
   /// The URIs for which the delegate has registered for change notifications,
   /// mapped to the language the delegate specified when registering for change notifications.
-  var watchedFiles: [DocumentURI: Language] = [:]
+  var watchedFiles: Set<DocumentURI> = []
 
   private var _indexStorePath: AbsolutePath?
   public var indexStorePath: AbsolutePath? {
@@ -109,13 +109,13 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     )
   }
 
-  public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
-    self.watchedFiles[uri] = language
+  public func registerForChangeNotifications(for uri: DocumentURI) async {
+    self.watchedFiles.insert(uri)
   }
 
   /// We don't support change watching.
   public func unregisterForChangeNotifications(for uri: DocumentURI) {
-    self.watchedFiles[uri] = nil
+    self.watchedFiles.remove(uri)
   }
 
   private func database(for url: URL) -> CompilationDatabase? {
@@ -165,11 +165,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     )
 
     if let delegate = self.delegate {
-      var changedFiles = Set<DocumentURI>()
-      for (uri, _) in self.watchedFiles {
-        changedFiles.insert(uri)
-      }
-      await delegate.fileBuildSettingsChanged(changedFiles)
+      await delegate.fileBuildSettingsChanged(self.watchedFiles)
     }
   }
 

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

@@ -80,7 +80,7 @@ public actor SwiftPMWorkspace {
 
   /// The URIs for which the delegate has registered for change notifications,
   /// mapped to the language the delegate specified when registering for change notifications.
-  var watchedFiles: [DocumentURI: Language] = [:]
+  var watchedFiles: Set<DocumentURI> = []
 
   /// This callback is informed when `reloadPackage` starts and ends executing.
   var reloadPackageStatusCallback: (ReloadPackageStatus) async -> Void
@@ -245,8 +245,7 @@ extension SwiftPMWorkspace {
       await reloadPackageStatusCallback(.end)
       return
     }
-    let changedFiles = Set<DocumentURI>(self.watchedFiles.keys)
-    await delegate.fileBuildSettingsChanged(changedFiles)
+    await delegate.fileBuildSettingsChanged(self.watchedFiles)
     await delegate.fileHandlingCapabilityChanged()
     await reloadPackageStatusCallback(.end)
   }
@@ -300,15 +299,14 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
     return nil
   }
 
-  public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
-    assert(self.watchedFiles[uri] == nil, "Registered twice for change notifications of the same URI")
-    self.watchedFiles[uri] = language
+  public func registerForChangeNotifications(for uri: DocumentURI) async {
+    self.watchedFiles.insert(uri)
   }
 
   /// Unregister the given file for build-system level change notifications, such as command
   /// line flag changes, dependency changes, etc.
   public func unregisterForChangeNotifications(for uri: DocumentURI) {
-    self.watchedFiles[uri] = nil
+    self.watchedFiles.remove(uri)
   }
 
   /// Returns the resolved target description for the given file, if one is known.

Tests/SKCoreTests/BuildServerBuildSystemTests.swift

@@ -53,7 +53,7 @@ final class BuildServerBuildSystemTests: XCTestCase {
       _fixLifetime(buildSystemDelegate)
     }
     await buildSystem.setDelegate(buildSystemDelegate)
-    await buildSystem.registerForChangeNotifications(for: DocumentURI(fileUrl), language: .swift)
+    await buildSystem.registerForChangeNotifications(for: DocumentURI(fileUrl))
 
     XCTAssertEqual(XCTWaiter.wait(for: [expectation], timeout: defaultTimeout), .completed)
   }
@@ -76,7 +76,7 @@ final class BuildServerBuildSystemTests: XCTestCase {
       _fixLifetime(buildSystemDelegate)
     }
     await buildSystem.setDelegate(buildSystemDelegate)
-    await buildSystem.registerForChangeNotifications(for: DocumentURI(fileUrl), language: .swift)
+    await buildSystem.registerForChangeNotifications(for: DocumentURI(fileUrl))
 
     try await fulfillmentOfOrThrow([expectation])
   }

Tests/SKCoreTests/BuildSystemManagerTests.swift

@@ -437,7 +437,7 @@ class ManualBuildSystem: BuildSystem {
     return map[uri]
   }
 
-  func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
+  func registerForChangeNotifications(for uri: DocumentURI) async {
   }
 
   func unregisterForChangeNotifications(for: DocumentURI) {

Tests/SourceKitLSPTests/BuildSystemTests.swift

@@ -48,7 +48,7 @@ final class TestBuildSystem: BuildSystem {
     return buildSettingsByFile[document]
   }
 
-  func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
+  func registerForChangeNotifications(for uri: DocumentURI) async {
     watchedFiles.insert(uri)
   }