Reload Swift Package when new file creation is indicated by DidChangeWatchedFileNotification

README.md

@@ -54,8 +54,5 @@ SourceKit-LSP is still in early development, so you may run into rough edges wit
 
 ### Caveats
 
-* SwiftPM build settings are not updated automatically after files are added/removed.
-	* **Workaround**: close and reopen the project after adding/removing files
-
 * SourceKit-LSP does not update its global index in the background, but instead relies on indexing-while-building to provide data. This only affects global queries like find-references and jump-to-definition.
 	* **Workaround**: build the project to update the index

Sources/SKCore/BuildServerBuildSystem.swift

@@ -219,6 +219,8 @@ extension BuildServerBuildSystem: BuildSystem {
       }
     }
   }
+
+  public func filesDidChange(_ events: [FileEvent]) {}
 }
 
 private func loadBuildServerConfig(path: AbsolutePath, fileSystem: FileSystem) throws -> BuildServerConfig {

Sources/SKCore/BuildSystem.swift

@@ -59,6 +59,9 @@ public protocol BuildSystem: AnyObject {
   /// Returns the output paths for the requested build targets
   func buildTargetOutputPaths(targets: [BuildTargetIdentifier],
                               reply: @escaping (LSPResult<[OutputsItem]>) -> Void)
+
+  /// Called when files in the project change.
+  func filesDidChange(_ events: [FileEvent])
 }
 
 public let buildTargetsNotSupported =

Sources/SKCore/BuildSystemManager.swift

@@ -125,6 +125,13 @@ public final class BuildSystemManager {
     self.fallbackSettingsTimeout = fallbackSettingsTimeout
     self.buildSystem?.delegate = self
   }
+
+  public func filesDidChange(_ events: [FileEvent]) {
+    queue.async {
+      self.buildSystem?.filesDidChange(events)
+      self.fallbackBuildSystem?.filesDidChange(events)
+    }
+  }
 }
 
 extension BuildSystemManager: BuildSystem {

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -109,6 +109,8 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
 
     return compdb
   }
+
+  public func filesDidChange(_ events: [FileEvent]) {}
 }
 
 extension CompilationDatabaseBuildSystem {

Sources/SKCore/FallbackBuildSystem.swift

@@ -98,4 +98,6 @@ public final class FallbackBuildSystem: BuildSystem {
     args.append(file)
     return FileBuildSettings(compilerArguments: args)
   }
+
+  public func filesDidChange(_ events: [FileEvent]) {}
 }

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

@@ -58,6 +58,18 @@ public final class SwiftPMWorkspace {
   var fileToTarget: [AbsolutePath: TargetBuildDescription] = [:]
   var sourceDirToTarget: [AbsolutePath: TargetBuildDescription] = [:]
 
+  /// 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] = [:]
+
+  /// Queue guarding the following properties:
+  /// - `delegate`
+  /// - `watchedFiles`
+  /// - `packageGraph`
+  /// - `fileToTarget`
+  /// - `sourceDirToTarget`
+  let queue: DispatchQueue = .init(label: "SwiftPMWorkspace.queue", qos: .utility)
+
   /// Creates a build system using the Swift Package Manager, if this workspace is a package.
   ///
   /// - Parameters:
@@ -156,13 +168,14 @@ extension SwiftPMWorkspace {
 
   /// (Re-)load the package settings by parsing the manifest and resolving all the targets and
   /// dependencies.
+  /// Must only be called on `queue` or from the initializer.
   func reloadPackage() throws {
 
     let observabilitySystem = ObservabilitySystem({ scope, diagnostic in
         log(diagnostic.description, level: diagnostic.severity.asLogLevel)
     })
 
-    self.packageGraph = try self.workspace.loadPackageGraph(
+    let packageGraph = try self.workspace.loadPackageGraph(
       rootInput: PackageGraphRootInput(packages: [packageRoot]),
       observabilityScope: observabilitySystem.topScope
     )
@@ -174,6 +187,11 @@ extension SwiftPMWorkspace {
       observabilityScope: observabilitySystem.topScope
     )
 
+    /// Make sure to execute any throwing statements before setting any
+    /// properties because otherwise we might end up in an inconsistent state
+    /// with only some properties modified.
+    self.packageGraph = packageGraph
+
     self.fileToTarget = [AbsolutePath: TargetBuildDescription](
       packageGraph.allTargets.flatMap { target in
         return target.sources.paths.compactMap {
@@ -197,6 +215,19 @@ extension SwiftPMWorkspace {
         // FIXME: is there  a preferred target?
         return td
     })
+
+    guard let delegate = self.delegate else { return }
+    var changedFiles: [DocumentURI: FileBuildSettingsChange] = [:]
+    for (uri, language) in self.watchedFiles {
+      orLog {
+        if let settings = try self.settings(for: uri, language) {
+          changedFiles[uri] = FileBuildSettingsChange(settings)
+        } else {
+          changedFiles[uri] = .removedOrUnavailable
+        }
+      }
+    }
+    delegate.fileBuildSettingsChanged(changedFiles)
   }
 }
 
@@ -214,10 +245,22 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
     return buildPath.appending(components: "index", "db")
   }
 
-  public func settings(
+  /// **Public for testing only**
+  public func _settings(
     for uri: DocumentURI,
     _ language: Language) throws -> FileBuildSettings?
   {
+    return try queue.sync {
+      try self.settings(for: uri, language)
+    }
+  }
+
+  /// Must only be called on `queue`.
+  private func settings(
+    for uri: DocumentURI,
+    _ language: Language) throws -> FileBuildSettings?
+  {
+    dispatchPrecondition(condition: .onQueue(queue))
     guard let url = uri.fileURL else {
       // We can't determine build settings for non-file URIs.
       return nil
@@ -242,16 +285,17 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
   }
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
-    guard let delegate = self.delegate else { return }
-
-    // TODO: Support for change detection (via file watching)
-    var settings: FileBuildSettings? = nil
-    do {
-      settings = try self.settings(for: uri, language)
-    } catch {
-      log("error computing settings: \(error)")
-    }
-    DispatchQueue.global().async {
+    queue.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.settings(for: uri, language)
+      } catch {
+        log("error computing settings: \(error)")
+      }
       if let settings = settings {
         delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)])
       } else {
@@ -263,7 +307,9 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
   /// 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) {
-    // TODO: Support for change detection (via file watching)
+    queue.async {
+      self.watchedFiles[uri] = nil
+    }
   }
 
   public func buildTargets(reply: @escaping (LSPResult<[BuildTarget]>) -> Void) {
@@ -280,7 +326,9 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
   }
 
   /// Returns the resolved target description for the given file, if one is known.
-  func targetDescription(for file: AbsolutePath) -> TargetBuildDescription? {
+  /// Must only be called on `queue`.
+  private func targetDescription(for file: AbsolutePath) -> TargetBuildDescription? {
+    dispatchPrecondition(condition: .onQueue(queue))
     if let td = fileToTarget[file] {
       return td
     }
@@ -293,6 +341,32 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
 
     return nil
   }
+
+  /// An event is relevant if it modifies a file that matches one of the file rules used by the SwiftPM workspace.
+  private func fileEventShouldTriggerPackageReload(event: FileEvent) -> Bool {
+    guard let fileURL = event.uri.fileURL else {
+      return false
+    }
+    switch event.type {
+    case .created, .deleted:
+      return self.workspace.fileAffectsSwiftOrClangBuildSettings(filePath: AbsolutePath(fileURL.path), packageGraph: self.packageGraph)
+    case .changed:
+      return false
+    default: // Unknown file change type
+      return false
+    }
+  }
+
+  public func filesDidChange(_ events: [FileEvent]) {
+    queue.async {
+      if events.contains(where: { self.fileEventShouldTriggerPackageReload(event: $0) }) {
+        orLog {
+          // TODO: It should not be necessary to reload the entire package just to get build settings for one file.
+          try self.reloadPackage()
+        }
+      }
+    }
+  }
 }
 
 extension SwiftPMWorkspace {
@@ -318,7 +392,9 @@ extension SwiftPMWorkspace {
   }
 
   /// Retrieve settings for a package manifest (Package.swift).
-  func settings(forPackageManifest path: AbsolutePath) -> FileBuildSettings? {
+  /// Must only be called on `queue`.
+  private func settings(forPackageManifest path: AbsolutePath) -> FileBuildSettings? {
+    dispatchPrecondition(condition: .onQueue(queue))
     func impl(_ path: AbsolutePath) -> FileBuildSettings? {
       for package in packageGraph.packages where path == package.manifest.path {
         let compilerArgs = workspace.interpreterFlags(for: package.path) + [path.pathString]
@@ -336,7 +412,9 @@ extension SwiftPMWorkspace {
   }
 
   /// Retrieve settings for a given header file.
-  public func settings(forHeader path: AbsolutePath, _ language: Language) throws -> FileBuildSettings? {
+  /// Must only be called on `queue`.
+  private func settings(forHeader path: AbsolutePath, _ language: Language) throws -> FileBuildSettings? {
+    dispatchPrecondition(condition: .onQueue(queue))
     func impl(_ path: AbsolutePath) throws -> FileBuildSettings? {
       var dir = path.parentDirectory
       while !dir.isRoot {

Sources/SKTestSupport/INPUTS/SwiftPMPackage/Sources/lib/lib.swift

@@ -3,3 +3,7 @@ public struct Lib {
 
   public init() {}
 }
+
+func topLevelFunction() {
+  /*Lib.topLevelFunction:body*/
+}

Sources/SKTestSupport/SKSwiftPMTestWorkspace.swift

@@ -133,6 +133,10 @@ extension SKSwiftPMTestWorkspace {
       version: 1,
       text: try sources.sourceCache.get(url))))
   }
+
+  public func closeDocument(_ url: URL) {
+    sk.send(DidCloseTextDocumentNotification(textDocument: TextDocumentIdentifier(DocumentURI(url))))
+  }
 }
 
 extension XCTestCase {

Sources/SourceKitLSP/CapabilityRegistry.swift

@@ -29,6 +29,9 @@ public final class CapabilityRegistry {
   /// Dynamically registered semantic tokens options.
   private var semanticTokens: [CapabilityRegistration: SemanticTokensRegistrationOptions] = [:]
 
+  /// Dynamically registered file watchers.
+  private var didChangeWatchedFiles: DidChangeWatchedFilesRegistrationOptions?
+
   /// Dynamically registered command IDs.
   private var commandIds: Set<String> = []
 
@@ -54,6 +57,10 @@ public final class CapabilityRegistry {
     clientCapabilities.workspace?.executeCommand?.dynamicRegistration == true
   }
 
+  public var clientHasDynamicDidChangeWatchedFilesRegistration: Bool {
+    clientCapabilities.workspace?.didChangeWatchedFiles?.dynamicRegistration == true
+  }
+
   /// Dynamically register completion capabilities if the client supports it and
   /// we haven't yet registered any completion capabilities for the given
   /// languages.
@@ -82,6 +89,28 @@ public final class CapabilityRegistry {
     registerOnClient(registration)
   }
 
+  public func registerDidChangeWatchedFiles(
+    watchers: [FileSystemWatcher],
+    registerOnClient: ClientRegistrationHandler
+  ) {
+    guard clientHasDynamicDidChangeWatchedFilesRegistration else { return }
+    if let registration = didChangeWatchedFiles {
+      if watchers != registration.watchers {
+        log("Unable to register new file system watchers \(watchers) due to pre-existing options \(registration.watchers)", level: .warning)
+      }
+      return
+    }
+    let registrationOptions = DidChangeWatchedFilesRegistrationOptions(
+      watchers: watchers)
+    let registration = CapabilityRegistration(
+      method: DidChangeWatchedFilesNotification.method,
+      registerOptions: self.encode(registrationOptions))
+
+    self.didChangeWatchedFiles = registrationOptions
+
+    registerOnClient(registration)
+  }
+
   /// Dynamically register folding range capabilities if the client supports it and
   /// we haven't yet registered any folding range capabilities for the given
   /// languages.

Sources/SourceKitLSP/SourceKitServer.swift

@@ -21,6 +21,7 @@ import SKSupport
 import TSCBasic
 import TSCLibc
 import TSCUtility
+import PackageLoading
 
 public typealias URL = Foundation.URL
 
@@ -75,6 +76,7 @@ public final class SourceKitServer: LanguageServer {
     registerWorkspaceNotfication(SourceKitServer.openDocument)
     registerWorkspaceNotfication(SourceKitServer.closeDocument)
     registerWorkspaceNotfication(SourceKitServer.changeDocument)
+    registerWorkspaceNotfication(SourceKitServer.didChangeWatchedFiles)
 
     registerToolchainTextDocumentNotification(SourceKitServer.willSaveDocument)
     registerToolchainTextDocumentNotification(SourceKitServer.didSaveDocument)
@@ -582,6 +584,14 @@ extension SourceKitServer {
         self.dynamicallyRegisterCapability($0, registry)
       }
     }
+
+    /// This must be a superset of the files that return true for SwiftPM's `Workspace.fileAffectsSwiftOrClangBuildSettings`.
+    let watchers = FileRuleDescription.builtinRules.flatMap({ $0.fileTypes }).map { fileExtension in
+      return FileSystemWatcher(globPattern: "**.\(fileExtension)", kind: [.create, .delete])
+    }
+    registry.registerDidChangeWatchedFiles(watchers: watchers) {
+      self.dynamicallyRegisterCapability($0, registry)
+    }
   }
 
   private func dynamicallyRegisterCapability(
@@ -662,7 +672,7 @@ extension SourceKitServer {
   func openDocument(_ note: Notification<DidOpenTextDocumentNotification>, workspace: Workspace) {
     openDocument(note.params, workspace: workspace)
   }
-  
+
   private func openDocument(_ note: DidOpenTextDocumentNotification, workspace: Workspace) {
     // Immediately open the document even if the build system isn't ready. This is important since
     // we check that the document is open when we receive messages from the build system.
@@ -748,6 +758,10 @@ extension SourceKitServer {
     languageService.didSaveDocument(note.params)
   }
 
+  func didChangeWatchedFiles(_ note: Notification<DidChangeWatchedFilesNotification>, workspace: Workspace) {
+    workspace.buildSystemManager.filesDidChange(note.params.changes)
+  }
+
   // MARK: - Language features
 
   func completion(

Tests/SKCoreTests/BuildSystemManagerTests.swift

@@ -503,6 +503,8 @@ class ManualBuildSystem: BuildSystem {
     reply: @escaping (LSPResult<[OutputsItem]>) -> Void) {
     fatalError()
   }
+
+  func filesDidChange(_ events: [FileEvent]) {}
 }
 
 /// A `BuildSystemDelegate` setup for testing.