Remove a couple of async wrapper functions

These are no longer needed as all callers are async now.

Author: ahoppen

Sources/SKCore/BuildSystemManager.swift

@@ -254,14 +254,7 @@ extension BuildSystemManager {
 }
 
 extension BuildSystemManager: BuildSystemDelegate {
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has ben asyncified
-  public nonisolated func fileBuildSettingsChanged(_ changes: Set<DocumentURI>) {
-    Task {
-      await fileBuildSettingsChangedImpl(changes)
-    }
-  }
-
-  public func fileBuildSettingsChangedImpl(_ changedFiles: Set<DocumentURI>) async {
+  public func fileBuildSettingsChanged(_ changedFiles: Set<DocumentURI>) async {
     let changedWatchedFiles = changedFiles.flatMap({ mainFile in
       self.watchedFiles.filter { $1.mainFile == mainFile }.keys
     })
@@ -271,14 +264,7 @@ extension BuildSystemManager: BuildSystemDelegate {
     }
   }
 
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has ben asyncified
-  public nonisolated func filesDependenciesUpdated(_ changedFiles: Set<DocumentURI>) {
-    Task {
-      await filesDependenciesUpdatedImpl(changedFiles)
-    }
-  }
-
-  public func filesDependenciesUpdatedImpl(_ changedFiles: Set<DocumentURI>) async {
+  public func filesDependenciesUpdated(_ changedFiles: Set<DocumentURI>) async {
     // Empty changes --> assume everything has changed.
     guard !changedFiles.isEmpty else {
       if let delegate = self._delegate {
@@ -295,27 +281,13 @@ extension BuildSystemManager: BuildSystemDelegate {
     }
   }
 
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has ben asyncified
-  public nonisolated func buildTargetsChanged(_ changes: [BuildTargetEvent]) {
-    Task {
-      await buildTargetsChangedImpl(changes)
-    }
-  }
-
-  public func buildTargetsChangedImpl(_ changes: [BuildTargetEvent]) async {
+  public func buildTargetsChanged(_ changes: [BuildTargetEvent]) async {
     if let delegate = self._delegate {
       await delegate.buildTargetsChanged(changes)
     }
   }
 
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has ben asyncified
-  public nonisolated func fileHandlingCapabilityChanged() {
-    Task {
-      await fileHandlingCapabilityChangedImpl()
-    }
-  }
-
-  public func fileHandlingCapabilityChangedImpl() async {
+  public func fileHandlingCapabilityChanged() async {
     if let delegate = self._delegate {
       await delegate.fileHandlingCapabilityChanged()
     }

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -87,8 +87,16 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
 
   public var indexPrefixMappings: [PathPrefixMapping] { return [] }
 
-  public func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? {
-    return self.settings(for: document)
+  public func buildSettings(for document: DocumentURI, language: Language) async -> FileBuildSettings? {
+    guard let url = document.fileURL else {
+      // We can't determine build settings for non-file URIs.
+      return nil
+    }
+    guard let db = database(for: url),
+          let cmd = db[url].first else { return nil }
+    return FileBuildSettings(
+      compilerArguments: Array(cmd.commandLine.dropFirst()),
+      workingDirectory: cmd.directory)
   }
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
@@ -168,22 +176,3 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     }
   }
 }
-
-extension CompilationDatabaseBuildSystem {
-  private func settings(for uri: DocumentURI) -> FileBuildSettings? {
-    guard let url = uri.fileURL else {
-      // We can't determine build settings for non-file URIs.
-      return nil
-    }
-    guard let db = database(for: url),
-          let cmd = db[url].first else { return nil }
-    return FileBuildSettings(
-      compilerArguments: Array(cmd.commandLine.dropFirst()),
-      workingDirectory: cmd.directory)
-  }
-
-  /// Exposed for *testing*.
-  public func _settings(for uri: DocumentURI) -> FileBuildSettings? {
-    return self.settings(for: uri)
-  }
-}

Sources/SourceKitLSP/SourceKitServer.swift

@@ -606,8 +606,7 @@ extension SourceKitServer: MessageHandler {
 // MARK: - Build System Delegate
 
 extension SourceKitServer: BuildSystemDelegate {
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has been asyncified
-  public nonisolated func buildTargetsChanged(_ changes: [BuildTargetEvent]) {
+  public func buildTargetsChanged(_ changes: [BuildTargetEvent]) {
     // TODO: do something with these changes once build target support is in place
   }
 
@@ -622,19 +621,11 @@ extension SourceKitServer: BuildSystemDelegate {
     return documentManager.openDocuments.intersection(changes)
   }
 
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has been asyncified
-  /// Non-async variant that executes `fileBuildSettingsChangedImpl` in a new task.
-  public nonisolated func fileBuildSettingsChanged(_ changedFiles: Set<DocumentURI>) {
-    Task {
-      await self.fileBuildSettingsChangedImpl(changedFiles)
-    }
-  }
-
   /// Handle a build settings change notification from the `BuildSystem`.
   /// This has two primary cases:
   /// - Initial settings reported for a given file, now we can fully open it
   /// - Changed settings for an already open file
-  public func fileBuildSettingsChangedImpl(_ changedFiles: Set<DocumentURI>) async {
+  public func fileBuildSettingsChanged(_ changedFiles: Set<DocumentURI>) async {
     for uri in changedFiles {
       guard self.documentManager.openDocuments.contains(uri) else {
         continue
@@ -648,17 +639,10 @@ extension SourceKitServer: BuildSystemDelegate {
     }
   }
 
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has been asyncified
-  public nonisolated func filesDependenciesUpdated(_ changedFiles: Set<DocumentURI>) {
-    Task {
-      await filesDependenciesUpdatedImpl(changedFiles)
-    }
-  }
-
   /// Handle a dependencies updated notification from the `BuildSystem`.
   /// We inform the respective language services as long as the given file is open
   /// (not queued for opening).
-  public func filesDependenciesUpdatedImpl(_ changedFiles: Set<DocumentURI>) async {
+  public func filesDependenciesUpdated(_ changedFiles: Set<DocumentURI>) async {
     // Split the changedFiles into the workspaces they belong to.
     // Then invoke affectedOpenDocumentsForChangeSet for each workspace with its affected files.
     let changedFilesAndWorkspace = await changedFiles.asyncMap {
@@ -678,14 +662,7 @@ extension SourceKitServer: BuildSystemDelegate {
     }
   }
 
-  // FIXME: (async) Make this method isolated once `BuildSystemDelegate` has been asyncified
-  public nonisolated func fileHandlingCapabilityChanged() {
-    Task {
-      await fileHandlingCapabilityChangedImpl()
-    }
-  }
-
-  public func fileHandlingCapabilityChangedImpl() {
+  public func fileHandlingCapabilityChanged() {
     self.uriToWorkspaceCache = [:]
   }
 }

Tests/SKCoreTests/BuildSystemManagerTests.swift

@@ -108,7 +108,7 @@ final class BuildSystemManagerTests: XCTestCase {
     bs.map[a] = nil
     let changed = expectation(description: "changed settings")
     await del.setExpected([(a, .swift, nil, changed, #file, #line)])
-    bsm.fileBuildSettingsChanged([a])
+    await bsm.fileBuildSettingsChanged([a])
     try await fulfillmentOfOrThrow([changed])
   }
 
@@ -129,7 +129,7 @@ final class BuildSystemManagerTests: XCTestCase {
     bs.map[a] = FileBuildSettings(compilerArguments: ["x"])
     let changed = expectation(description: "changed settings")
     await del.setExpected([(a, .swift, bs.map[a]!, changed, #file, #line)])
-    bsm.fileBuildSettingsChanged([a])
+    await bsm.fileBuildSettingsChanged([a])
     try await fulfillmentOfOrThrow([changed])
   }
 
@@ -152,13 +152,13 @@ final class BuildSystemManagerTests: XCTestCase {
     bs.map[a] = FileBuildSettings(compilerArguments: ["non-fallback", "args"])
     let changed = expectation(description: "changed settings")
     await del.setExpected([(a, .swift, bs.map[a]!, changed, #file, #line)])
-    bsm.fileBuildSettingsChanged([a])
+    await bsm.fileBuildSettingsChanged([a])
     try await fulfillmentOfOrThrow([changed])
 
     bs.map[a] = nil
     let revert = expectation(description: "revert to fallback settings")
     await del.setExpected([(a, .swift, fallbackSettings, revert, #file, #line)])
-    bsm.fileBuildSettingsChanged([a])
+    await bsm.fileBuildSettingsChanged([a])
     try await fulfillmentOfOrThrow([revert])
   }
 
@@ -186,7 +186,7 @@ final class BuildSystemManagerTests: XCTestCase {
     bs.map[b] = FileBuildSettings(compilerArguments: ["yy"])
     let changed = expectation(description: "changed settings")
     await del.setExpected([(a, .swift, bs.map[a]!, changed, #file, #line)])
-    bsm.fileBuildSettingsChanged([a])
+    await bsm.fileBuildSettingsChanged([a])
     try await fulfillmentOfOrThrow([changed])
 
     // Test multiple changes.
@@ -198,7 +198,7 @@ final class BuildSystemManagerTests: XCTestCase {
       (a, .swift, bs.map[a]!, changedBothA, #file, #line),
       (b, .swift, bs.map[b]!, changedBothB, #file, #line),
     ])
-    bsm.fileBuildSettingsChanged([a, b])
+    await bsm.fileBuildSettingsChanged([a, b])
     try await fulfillmentOfOrThrow([changedBothA, changedBothB])
   }
 
@@ -228,7 +228,7 @@ final class BuildSystemManagerTests: XCTestCase {
     bs.map[b] = nil
     let changed = expectation(description: "changed settings")
     await del.setExpected([(b, .swift, nil, changed, #file, #line)])
-    bsm.fileBuildSettingsChanged([b])
+    await bsm.fileBuildSettingsChanged([b])
     try await fulfillmentOfOrThrow([changed])
   }
 
@@ -327,7 +327,7 @@ final class BuildSystemManagerTests: XCTestCase {
       (h1, .c, newArgsH1, changed1, #file, #line),
       (h2, .c, newArgsH2, changed2, #file, #line),
     ])
-    bsm.fileBuildSettingsChanged([cpp])
+    await bsm.fileBuildSettingsChanged([cpp])
 
     try await fulfillmentOfOrThrow([changed1, changed2])
   }
@@ -370,7 +370,7 @@ final class BuildSystemManagerTests: XCTestCase {
     await bsm.unregisterForChangeNotifications(for: c)
     // At this point only b is registered, but that can race with notifications,
     // so ensure nothing bad happens and we still get the notification for b.
-    bsm.fileBuildSettingsChanged([a, b, c])
+    await bsm.fileBuildSettingsChanged([a, b, c])
 
     try await fulfillmentOfOrThrow([changedB])
   }
@@ -396,7 +396,7 @@ final class BuildSystemManagerTests: XCTestCase {
     let depUpdate2 = expectation(description: "dependencies update 2")
     await del.setExpectedDependenciesUpdate([(a, depUpdate2, #file, #line)])
 
-    bsm.filesDependenciesUpdated([a])
+    await bsm.filesDependenciesUpdated([a])
     try await fulfillmentOfOrThrow([depUpdate2])
   }
 }

Tests/SKCoreTests/CompilationDatabaseTests.swift

@@ -217,7 +217,7 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      let settings = await buildSystem._settings(for: DocumentURI(URL(fileURLWithPath: "/a/a.swift")))
+      let settings = await buildSystem.buildSettings(for: DocumentURI(URL(fileURLWithPath: "/a/a.swift")), language: .swift)
       XCTAssertNotNil(settings)
       XCTAssertEqual(settings?.workingDirectory, "/a")
       XCTAssertEqual(settings?.compilerArguments, ["-swift-version", "4", "/a/a.swift"])

Tests/SourceKitLSPTests/SourceKitTests.swift

@@ -243,7 +243,7 @@ final class SKTests: XCTestCase {
       XCTAssertEqual(note.params.diagnostics.count, 0)
       finishExpectation.fulfill()
     }
-    server.filesDependenciesUpdated([DocumentURI(moduleRef.url)])
+    await server.filesDependenciesUpdated([DocumentURI(moduleRef.url)])
 
     try await fulfillmentOfOrThrow([finishExpectation])
   }
@@ -286,7 +286,7 @@ final class SKTests: XCTestCase {
       XCTAssertEqual(note.params.diagnostics.count, 0)
       finishExpectation.fulfill()
     }
-    server.filesDependenciesUpdated([DocumentURI(moduleRef.url)])
+    await server.filesDependenciesUpdated([DocumentURI(moduleRef.url)])
 
     try await fulfillmentOfOrThrow([finishExpectation])
   }