Migrate CompilationDatabaseBuildSystem to be an actor

Author: ahoppen

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -25,18 +25,10 @@ import var TSCBasic.localFileSystem
 ///
 /// Provides build settings from a `CompilationDatabase` found by searching a project. For now, only
 /// one compilation database, located at the project root.
-public final class CompilationDatabaseBuildSystem {
-
-  /// Queue guarding the following properties:
-  /// - `compdb`
-  /// - `watchedFiles`
-  /// - `_indexStorePath`
-  let queue: DispatchQueue = .init(label: "CompilationDatabaseBuildSystem.queue", qos: .userInitiated)
-
+public actor CompilationDatabaseBuildSystem {
   /// The compilation database.
   var compdb: CompilationDatabase? = nil {
     didSet {
-      dispatchPrecondition(condition: .onQueue(queue))
       // Build settings have changed and thus the index store path might have changed.
       // Recompute it on demand.
       _indexStorePath = nil
@@ -60,24 +52,22 @@ public final class CompilationDatabaseBuildSystem {
 
   private var _indexStorePath: AbsolutePath?
   public var indexStorePath: AbsolutePath? {
-    return queue.sync {
-      if let indexStorePath = _indexStorePath {
-        return indexStorePath
-      }
+    if let indexStorePath = _indexStorePath {
+      return indexStorePath
+    }
 
-      if let allCommands = self.compdb?.allCommands {
-        for command in allCommands {
-          let args = command.commandLine
-          for i in args.indices.reversed() {
-            if args[i] == "-index-store-path" && i != args.endIndex - 1 {
-              _indexStorePath = try? AbsolutePath(validating: args[i+1])
-              return _indexStorePath
-            }
+    if let allCommands = self.compdb?.allCommands {
+      for command in allCommands {
+        let args = command.commandLine
+        for i in args.indices.reversed() {
+          if args[i] == "-index-store-path" && i != args.endIndex - 1 {
+            _indexStorePath = try? AbsolutePath(validating: args[i+1])
+            return _indexStorePath
           }
         }
       }
-      return nil
     }
+    return nil
   }
 
   public init(projectRoot: AbsolutePath? = nil, fileSystem: FileSystem = localFileSystem) {
@@ -98,44 +88,31 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
   public var indexPrefixMappings: [PathPrefixMapping] { return [] }
 
   public func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? {
-    // FIXME: (async) Convert this to an async function once `CompilationDatabaseBuildSystem` is an actor.
-    return await withCheckedContinuation { continuation in
-      self.queue.async {
-        continuation.resume(returning: self.settings(for: document))
-      }
-    }
+    return self.settings(for: document)
   }
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
-    queue.async {
-      self.watchedFiles[uri] = language
+    self.watchedFiles[uri] = language
 
-      guard let delegate = self.delegate else { return }
+    guard let delegate = self.delegate else { return }
 
-      let settings = self.settings(for: uri)
-      delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)])
-    }
+    let settings = self.settings(for: uri)
+    delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)])
   }
 
   /// We don't support change watching.
   public func unregisterForChangeNotifications(for uri: DocumentURI) {
-    queue.async {
-      self.watchedFiles[uri] = nil
-    }
+    self.watchedFiles[uri] = nil
   }
 
-  /// Must be invoked on `queue`.
   private func database(for url: URL) -> CompilationDatabase? {
-    dispatchPrecondition(condition: .onQueue(queue))
     if let path = try? AbsolutePath(validating: url.path) {
       return database(for: path)
     }
     return compdb
   }
 
-  /// Must be invoked on `queue`.
   private func database(for path: AbsolutePath) -> CompilationDatabase? {
-    dispatchPrecondition(condition: .onQueue(queue))
     if compdb == nil {
       var dir = path
       while !dir.isRoot {
@@ -165,10 +142,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
 
   /// The compilation database has been changed on disk.
   /// Reload it and notify the delegate about build setting changes.
-  /// Must be called on `queue`.
   private func reloadCompilationDatabase() {
-    dispatchPrecondition(condition: .onQueue(queue))
-
     guard let projectRoot = self.projectRoot else { return }
 
     self.compdb = tryLoadCompilationDatabase(directory: projectRoot, self.fileSystem)
@@ -187,31 +161,25 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
   }
 
   public func filesDidChange(_ events: [FileEvent]) {
-    queue.async {
-      if events.contains(where: { self.fileEventShouldTriggerCompilationDatabaseReload(event: $0) }) {
-        self.reloadCompilationDatabase()
-      }
+    if events.contains(where: { self.fileEventShouldTriggerCompilationDatabaseReload(event: $0) }) {
+      self.reloadCompilationDatabase()
     }
   }
 
   public func fileHandlingCapability(for uri: DocumentURI) -> FileHandlingCapability {
     guard let fileUrl = uri.fileURL else {
       return .unhandled
     }
-    return queue.sync {
-      if database(for: fileUrl) != nil {
-        return .handled
-      } else {
-        return .unhandled
-      }
+    if database(for: fileUrl) != nil {
+      return .handled
+    } else {
+      return .unhandled
     }
   }
 }
 
 extension CompilationDatabaseBuildSystem {
-  /// Must be invoked on `queue`.
   private func settings(for uri: DocumentURI) -> FileBuildSettings? {
-    dispatchPrecondition(condition: .onQueue(queue))
     guard let url = uri.fileURL else {
       // We can't determine build settings for non-file URIs.
       return nil
@@ -225,8 +193,6 @@ extension CompilationDatabaseBuildSystem {
 
   /// Exposed for *testing*.
   public func _settings(for uri: DocumentURI) -> FileBuildSettings? {
-    return queue.sync {
-      return self.settings(for: uri)
-    }
+    return self.settings(for: uri)
   }
 }

Tests/SKCoreTests/CompilationDatabaseTests.swift

@@ -207,8 +207,8 @@ final class CompilationDatabaseTests: XCTestCase {
     ])
   }
 
-  func testCompilationDatabaseBuildSystem() throws {
-    try checkCompilationDatabaseBuildSystem("""
+  func testCompilationDatabaseBuildSystem() async throws {
+    try await checkCompilationDatabaseBuildSystem("""
     [
       {
         "file": "/a/a.swift",
@@ -217,23 +217,23 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      let settings = buildSystem._settings(for: DocumentURI(URL(fileURLWithPath: "/a/a.swift")))
+      let settings = await buildSystem._settings(for: DocumentURI(URL(fileURLWithPath: "/a/a.swift")))
       XCTAssertNotNil(settings)
       XCTAssertEqual(settings?.workingDirectory, "/a")
       XCTAssertEqual(settings?.compilerArguments, ["-swift-version", "4", "/a/a.swift"])
-      XCTAssertNil(buildSystem.indexStorePath)
-      XCTAssertNil(buildSystem.indexDatabasePath)
+      assertNil(await buildSystem.indexStorePath)
+      assertNil(await buildSystem.indexDatabasePath)
     }
   }
 
-  func testCompilationDatabaseBuildSystemIndexStoreSwift0() throws {
-    try checkCompilationDatabaseBuildSystem("[]") { buildSystem in
-      XCTAssertNil(buildSystem.indexStorePath)
+  func testCompilationDatabaseBuildSystemIndexStoreSwift0() async throws {
+    try await checkCompilationDatabaseBuildSystem("[]") { buildSystem in
+      assertNil(await buildSystem.indexStorePath)
     }
   }
 
-  func testCompilationDatabaseBuildSystemIndexStoreSwift1() throws {
-    try checkCompilationDatabaseBuildSystem("""
+  func testCompilationDatabaseBuildSystemIndexStoreSwift1() async throws {
+    try await checkCompilationDatabaseBuildSystem("""
     [
       {
         "file": "/a/a.swift",
@@ -242,13 +242,13 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      XCTAssertEqual(URL(fileURLWithPath: buildSystem.indexStorePath?.pathString ?? "").path, "/b")
-      XCTAssertEqual(URL(fileURLWithPath: buildSystem.indexDatabasePath?.pathString ?? "").path, "/IndexDatabase")
+      assertEqual(URL(fileURLWithPath: await buildSystem.indexStorePath?.pathString ?? "").path, "/b")
+      assertEqual(URL(fileURLWithPath: await buildSystem.indexDatabasePath?.pathString ?? "").path, "/IndexDatabase")
     }
   }
 
-  func testCompilationDatabaseBuildSystemIndexStoreSwift2() throws {
-    try checkCompilationDatabaseBuildSystem("""
+  func testCompilationDatabaseBuildSystemIndexStoreSwift2() async throws {
+    try await checkCompilationDatabaseBuildSystem("""
     [
       {
         "file": "/a/a.swift",
@@ -267,12 +267,12 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      XCTAssertEqual(buildSystem.indexStorePath, try AbsolutePath(validating: "/b"))
+      await assertEqual(buildSystem.indexStorePath, try AbsolutePath(validating: "/b"))
     }
   }
 
-  func testCompilationDatabaseBuildSystemIndexStoreSwift3() throws {
-    try checkCompilationDatabaseBuildSystem("""
+  func testCompilationDatabaseBuildSystemIndexStoreSwift3() async throws {
+    try await checkCompilationDatabaseBuildSystem("""
     [
       {
         "file": "/a/a.swift",
@@ -281,12 +281,12 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      XCTAssertEqual(buildSystem.indexStorePath, try AbsolutePath(validating: "/b"))
+      assertEqual(await buildSystem.indexStorePath, try AbsolutePath(validating: "/b"))
     }
   }
 
-  func testCompilationDatabaseBuildSystemIndexStoreSwift4() throws {
-    try checkCompilationDatabaseBuildSystem("""
+  func testCompilationDatabaseBuildSystemIndexStoreSwift4() async throws {
+    try await checkCompilationDatabaseBuildSystem("""
     [
       {
         "file": "/a/a.swift",
@@ -295,12 +295,12 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      XCTAssertNil(buildSystem.indexStorePath)
+      assertNil(await buildSystem.indexStorePath)
     }
   }
 
-  func testCompilationDatabaseBuildSystemIndexStoreClang() throws {
-    try checkCompilationDatabaseBuildSystem("""
+  func testCompilationDatabaseBuildSystemIndexStoreClang() async throws {
+    try await checkCompilationDatabaseBuildSystem("""
     [
       {
         "file": "/a/a.cpp",
@@ -319,16 +319,16 @@ final class CompilationDatabaseTests: XCTestCase {
       }
     ]
     """) { buildSystem in
-      XCTAssertEqual(URL(fileURLWithPath: buildSystem.indexStorePath?.pathString ?? "").path, "/b")
-      XCTAssertEqual(URL(fileURLWithPath: buildSystem.indexDatabasePath?.pathString ?? "").path, "/IndexDatabase")
+      assertEqual(URL(fileURLWithPath: await buildSystem.indexStorePath?.pathString ?? "").path, "/b")
+      assertEqual(URL(fileURLWithPath: await buildSystem.indexDatabasePath?.pathString ?? "").path, "/IndexDatabase")
     }
   }
 }
 
-private func checkCompilationDatabaseBuildSystem(_ compdb: ByteString, block: (CompilationDatabaseBuildSystem) throws -> ()) throws {
+private func checkCompilationDatabaseBuildSystem(_ compdb: ByteString, block: (CompilationDatabaseBuildSystem) async throws -> ()) async throws {
   let fs = InMemoryFileSystem()
   try fs.createDirectory(AbsolutePath(validating: "/a"))
   try fs.writeFileContents(AbsolutePath(validating: "/a/compile_commands.json"), bytes: compdb)
   let buildSystem = CompilationDatabaseBuildSystem(projectRoot: try AbsolutePath(validating: "/a"), fileSystem: fs)
-  try block(buildSystem)
+  try await block(buildSystem)
 }