Use the BSP output file paths request to exclude units from the index that are no longer part of any targets

Author: ahoppen

Contributor Documentation/BSP Extensions.md

@@ -50,7 +50,9 @@ If `data` contains a string value for the `workDoneProgressTitle` key, then the
 
 For all the source files in this target, the output paths that are used during indexing, ie. the `-index-unit-output-path` for the file, if it is specified in the compiler arguments or the file that is passed as `-o`, if `-index-unit-output-path` is not specified.
 
- server communicates during the initialize handshake whether this method is supported or not by setting `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
+This allows SourceKit-LSP to remove index entries for source files that are removed from a target but remain present on disk.
+
+The server communicates during the initialize handshake whether this method is supported or not by setting `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
 
 - method: `buildTarget/outputPaths`
 - params: `OutputPathsParams`

Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift

@@ -323,15 +323,18 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send
     if case .string(let indexStorePath) = dictionary[CodingKeys.indexStorePath.stringValue] {
       self.indexStorePath = indexStorePath
     }
-    if let watchers = dictionary[CodingKeys.watchers.stringValue] {
-      self.watchers = [FileSystemWatcher](fromLSPArray: watchers)
+    if case .bool(let outputPathsProvider) = dictionary[CodingKeys.outputPathsProvider.stringValue] {
+      self.outputPathsProvider = outputPathsProvider
     }
     if case .bool(let prepareProvider) = dictionary[CodingKeys.prepareProvider.stringValue] {
       self.prepareProvider = prepareProvider
     }
     if case .bool(let sourceKitOptionsProvider) = dictionary[CodingKeys.sourceKitOptionsProvider.stringValue] {
       self.sourceKitOptionsProvider = sourceKitOptionsProvider
     }
+    if let watchers = dictionary[CodingKeys.watchers.stringValue] {
+      self.watchers = [FileSystemWatcher](fromLSPArray: watchers)
+    }
   }
 
   public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny {
@@ -342,15 +345,18 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send
     if let indexStorePath {
       result[CodingKeys.indexStorePath.stringValue] = .string(indexStorePath)
     }
-    if let watchers {
-      result[CodingKeys.watchers.stringValue] = watchers.encodeToLSPAny()
+    if let outputPathsProvider {
+      result[CodingKeys.outputPathsProvider.stringValue] = .bool(outputPathsProvider)
     }
     if let prepareProvider {
       result[CodingKeys.prepareProvider.stringValue] = .bool(prepareProvider)
     }
     if let sourceKitOptionsProvider {
       result[CodingKeys.sourceKitOptionsProvider.stringValue] = .bool(sourceKitOptionsProvider)
     }
+    if let watchers {
+      result[CodingKeys.watchers.stringValue] = watchers.encodeToLSPAny()
+    }
     return .dictionary(result)
   }
 }

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -631,7 +631,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
         // All targets might have changed
         return true
       }
-      return !updatedTargets.intersection(cacheKey.targets).isEmpty
+      return !updatedTargets.isDisjoint(with: cacheKey.targets)
     }
     self.cachedSourceFilesAndDirectories.clearAll(isolation: self)
 
@@ -1167,7 +1167,17 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     return response.items
   }
 
-  package func outputPath(from targets: Set<BuildTargetIdentifier>) async throws -> [String] {
+  /// Return the output paths for all source files known to the build server.
+  ///
+  /// See `BuildTargetOutputPathsRequest` for details.
+  package func outputPathInAllTargets() async throws -> [String] {
+    return try await outputPaths(in: Set(buildTargets().map(\.key)))
+  }
+
+  /// For all source files in the given targets, return their output file paths.
+  ///
+  /// See `BuildTargetOutputPathsRequest` for details.
+  package func outputPaths(in targets: Set<BuildTargetIdentifier>) async throws -> [String] {
     guard let buildSystemAdapter = await buildSystemAdapterAfterInitialized, !targets.isEmpty else {
       return []
     }

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -466,7 +466,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     signposter.endInterval("Reloading package", state)
   }
 
-  package nonisolated var supportsPreparationAndOutputPaths: Bool { true }
+  package nonisolated var supportsPreparationAndOutputPaths: Bool { options.backgroundIndexingOrDefault }
 
   package var buildPath: URL {
     return destinationBuildParameters.buildPath.asURL
@@ -489,11 +489,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
 
   /// Return the compiler arguments for the given source file within a target, making any necessary adjustments to
   /// account for differences in the SwiftPM versions being linked into SwiftPM and being installed in the toolchain.
-  private func compilerArguments(
-    for file: DocumentURI,
-    in buildTarget: any SwiftBuildTarget,
-    language: Language
-  ) async throws -> [String] {
+  private func compilerArguments(for file: DocumentURI, in buildTarget: any SwiftBuildTarget) async throws -> [String] {
     guard let fileURL = file.fileURL else {
       struct NonFileURIError: Swift.Error, CustomStringConvertible {
         let uri: DocumentURI
@@ -638,11 +634,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       // getting its compiler arguments and then patching up the compiler arguments by replacing the substitute file
       // with the `.cpp` file.
       let buildSettings = FileBuildSettings(
-        compilerArguments: try await compilerArguments(
-          for: DocumentURI(substituteFile),
-          in: swiftPMTarget,
-          language: request.language
-        ),
+        compilerArguments: try await compilerArguments(for: DocumentURI(substituteFile), in: swiftPMTarget),
         workingDirectory: try projectRoot.filePath,
         language: request.language
       ).patching(newFile: DocumentURI(try path.asURL.realpath), originalFile: DocumentURI(substituteFile))
@@ -653,11 +645,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     }
 
     return TextDocumentSourceKitOptionsResponse(
-      compilerArguments: try await compilerArguments(
-        for: request.textDocument.uri,
-        in: swiftPMTarget,
-        language: request.language
-      ),
+      compilerArguments: try await compilerArguments(for: request.textDocument.uri, in: swiftPMTarget),
       workingDirectory: try projectRoot.filePath
     )
   }

Sources/SemanticIndex/CheckedIndex.swift

@@ -58,7 +58,8 @@ package enum IndexCheckLevel {
 /// - SeeAlso: Comment on `IndexOutOfDateChecker`
 package final class CheckedIndex {
   private var checker: IndexOutOfDateChecker
-  private let index: IndexStoreDB
+  package let unchecked: UncheckedIndex
+  private var index: IndexStoreDB { unchecked.underlyingIndexStoreDB }
 
   /// Maps the USR of a symbol to its name and the name of all its containers, from outermost to innermost.
   ///
@@ -86,15 +87,11 @@ package final class CheckedIndex {
   /// `containerNamesCache[<usr of Foo>]` will be `["Bar", "Foo"]`.
   private var containerNamesCache: [String: [String]] = [:]
 
-  fileprivate init(index: IndexStoreDB, checkLevel: IndexCheckLevel) {
-    self.index = index
+  fileprivate init(unchecked: UncheckedIndex, checkLevel: IndexCheckLevel) {
+    self.unchecked = unchecked
     self.checker = IndexOutOfDateChecker(checkLevel: checkLevel)
   }
 
-  package var unchecked: UncheckedIndex {
-    return UncheckedIndex(index)
-  }
-
   @discardableResult
   package func forEachSymbolOccurrence(
     byUSR usr: String,
@@ -294,26 +291,43 @@ package final class CheckedIndex {
 /// access of the underlying `IndexStoreDB`. This makes sure that accesses to the raw `IndexStoreDB` are explicit (by
 /// calling `underlyingIndexStoreDB`) and we don't accidentally call into the `IndexStoreDB` when we wanted a
 /// `CheckedIndex`.
-package struct UncheckedIndex: Sendable {
-  package let underlyingIndexStoreDB: IndexStoreDB
+package final actor UncheckedIndex: Sendable {
+  package nonisolated let underlyingIndexStoreDB: IndexStoreDB
+
+  /// Whether the underlying `IndexStoreDB` uses has `useExplicitOutputUnits` enabled and thus needs to receive updates
+  /// updates as output paths are added or removed from the project.
+  package let usesExplicitOutputPaths: Bool
 
-  package init?(_ index: IndexStoreDB?) {
+  /// The set of unit output paths that are currently registered in the underlying `IndexStoreDB`.
+  private var unitOutputPaths: Set<String> = []
+
+  package init?(_ index: IndexStoreDB?, usesExplicitOutputPaths: Bool) {
     guard let index else {
       return nil
     }
+    self.usesExplicitOutputPaths = usesExplicitOutputPaths
     self.underlyingIndexStoreDB = index
   }
 
-  package init(_ index: IndexStoreDB) {
-    self.underlyingIndexStoreDB = index
+  /// Update the set of output paths that should be considered visible in the project. For example, if a source file is
+  /// removed from all targets in the project but remains on disk, this allows the index to start excluding it.
+  package func setUnitOutputPaths(_ paths: Set<String>) {
+    guard usesExplicitOutputPaths else {
+      return
+    }
+    let addedPaths = paths.filter { !unitOutputPaths.contains($0) }
+    let removedPaths = unitOutputPaths.filter { !paths.contains($0) }
+    underlyingIndexStoreDB.addUnitOutFilePaths(Array(addedPaths), waitForProcessing: false)
+    underlyingIndexStoreDB.removeUnitOutFilePaths(Array(removedPaths), waitForProcessing: false)
+    self.unitOutputPaths = paths
   }
 
-  package func checked(for checkLevel: IndexCheckLevel) -> CheckedIndex {
-    return CheckedIndex(index: underlyingIndexStoreDB, checkLevel: checkLevel)
+  package nonisolated func checked(for checkLevel: IndexCheckLevel) -> CheckedIndex {
+    return CheckedIndex(unchecked: self, checkLevel: checkLevel)
   }
 
   /// Wait for IndexStoreDB to be updated based on new unit files written to disk.
-  package func pollForUnitChangesAndWait() {
+  package nonisolated func pollForUnitChangesAndWait() {
     self.underlyingIndexStoreDB.pollForUnitChangesAndWait()
   }
 }

Sources/SemanticIndex/IndexHooks.swift

@@ -28,6 +28,10 @@ package protocol IndexInjector: Sendable {
     delegate: IndexDelegate,
     prefixMappings: [PathMapping]
   ) async throws -> IndexStoreDB
+
+  /// Whether the injected index uses explicit output paths and whether SourceKit-LSP should thus set the unit output
+  /// paths on the index as they change.
+  var usesExplicitOutputPaths: Bool { get async }
 }
 
 /// Callbacks that allow inspection of internal state modifications during testing.

Sources/SourceKitLSP/Workspace.swift

@@ -285,7 +285,6 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
       "Created workspace at \(rootUri.forLogging) with project root \(buildSystemSpec?.projectRoot.description ?? "<nil>")"
     )
 
-    var index: IndexStoreDB? = nil
     var indexDelegate: SourceKitIndexDelegate? = nil
 
     let indexOptions = options.indexOrDefault
@@ -301,36 +300,43 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
       } else {
         nil
       }
+    let supportsOutputPaths = await buildSystemManager.initializationData?.outputPathsProvider ?? false
+    var index: UncheckedIndex? = nil
     if let indexStorePath, let indexDatabasePath, let libPath = await toolchainRegistry.default?.libIndexStore {
       do {
         indexDelegate = SourceKitIndexDelegate()
         let prefixMappings =
           (indexOptions.indexPrefixMap ?? [:])
           .map { PathMapping(original: $0.key, replacement: $0.value) }
         if let indexInjector = hooks.indexHooks.indexInjector {
-          index = try await indexInjector.createIndex(
+          let indexStoreDB = try await indexInjector.createIndex(
             storePath: indexStorePath,
             databasePath: indexDatabasePath,
             indexStoreLibraryPath: libPath,
             delegate: indexDelegate!,
             prefixMappings: prefixMappings
           )
+          index = UncheckedIndex(indexStoreDB, usesExplicitOutputPaths: await indexInjector.usesExplicitOutputPaths)
         } else {
-          index = try IndexStoreDB(
+          let indexStoreDB = try IndexStoreDB(
             storePath: indexStorePath.filePath,
             databasePath: indexDatabasePath.filePath,
             library: IndexStoreLibrary(dylibPath: libPath.filePath),
             delegate: indexDelegate,
+            useExplicitOutputUnits: supportsOutputPaths,
             prefixMappings: prefixMappings
           )
-          logger.debug("Opened IndexStoreDB at \(indexDatabasePath) with store path \(indexStorePath)")
+          index = UncheckedIndex(indexStoreDB, usesExplicitOutputPaths: supportsOutputPaths)
+          logger.debug(
+            "Opened IndexStoreDB at \(indexDatabasePath) with store path \(indexStorePath) with explicit output files \(supportsOutputPaths)"
+          )
         }
       } catch {
         logger.error("Failed to open IndexStoreDB: \(error.localizedDescription)")
       }
     }
 
-    await buildSystemManager.setMainFilesProvider(UncheckedIndex(index))
+    await buildSystemManager.setMainFilesProvider(index)
 
     await self.init(
       sourceKitLSPServer: sourceKitLSPServer,
@@ -339,7 +345,7 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
       options: options,
       hooks: hooks,
       buildSystemManager: buildSystemManager,
-      index: UncheckedIndex(index),
+      index: index,
       indexDelegate: indexDelegate,
       indexTaskScheduler: indexTaskScheduler
     )
@@ -450,6 +456,18 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
       let testFiles = try await buildSystemManager.testFiles()
       await syntacticTestIndex.listOfTestFilesDidChange(testFiles)
     }
+
+    if await self.uncheckedIndex?.usesExplicitOutputPaths ?? false {
+      if await buildSystemManager.initializationData?.outputPathsProvider ?? false {
+        await orLog("Setting new list of unit output paths") {
+          try await self.uncheckedIndex?.setUnitOutputPaths(Set(buildSystemManager.outputPathInAllTargets()))
+        }
+      } else {
+        // This can only happen if an index got injected that uses explicit output paths but the build system does not
+        // support output paths.
+        logger.error("The index uses explicit output paths but the build system does not support output paths")
+      }
+    }
   }
 
   package var clientSupportsWorkDoneProgress: Bool {

Tests/SourceKitLSPTests/WorkspaceSymbolsTests.swift

@@ -117,4 +117,82 @@ class WorkspaceSymbolsTests: XCTestCase {
       ]
     )
   }
+
+  func testHideSymbolsFromExcludedFiles() async throws {
+    let project = try await SwiftPMTestProject(
+      files: [
+        "FileA.swift": "func 1️⃣doThingA() {}",
+        "FileB.swift": "func 2️⃣doThingB() {}",
+      ],
+      manifest: """
+        // swift-tools-version: 5.7
+
+        import PackageDescription
+
+        let package = Package(
+          name: "MyLibrary",
+          targets: [.target(name: "MyLibrary")]
+        )
+        """,
+      enableBackgroundIndexing: true
+    )
+    let symbolsBeforeDeletion = try await project.testClient.send(WorkspaceSymbolsRequest(query: "doThing"))
+    XCTAssertEqual(
+      symbolsBeforeDeletion,
+      [
+        .symbolInformation(
+          SymbolInformation(
+            name: "doThingA()",
+            kind: .function,
+            location: try project.location(from: "1️⃣", to: "1️⃣", in: "FileA.swift")
+          )
+        ),
+        .symbolInformation(
+          SymbolInformation(
+            name: "doThingB()",
+            kind: .function,
+            location: try project.location(from: "2️⃣", to: "2️⃣", in: "FileB.swift")
+          )
+        ),
+      ]
+    )
+
+    try """
+    // swift-tools-version: 5.7
+
+    import PackageDescription
+
+    let package = Package(
+      name: "MyLibrary",
+      targets: [.target(name: "MyLibrary", exclude: ["FileA.swift"])]
+    )
+    """.write(to: XCTUnwrap(project.uri(for: "Package.swift").fileURL), atomically: true, encoding: .utf8)
+
+    // Test that we exclude FileB.swift from the index after it has been removed from the package's target.
+    project.testClient.send(
+      DidChangeWatchedFilesNotification(
+        changes: [FileEvent(uri: try project.uri(for: "Package.swift"), type: .changed)]
+      )
+    )
+    try await repeatUntilExpectedResult {
+      let symbolsAfterDeletion = try await project.testClient.send(WorkspaceSymbolsRequest(query: "doThing"))
+      if symbolsAfterDeletion?.count == 2 {
+        // The exclusion hasn't been processed yet, try again.
+        return false
+      }
+      XCTAssertEqual(
+        symbolsAfterDeletion,
+        [
+          .symbolInformation(
+            SymbolInformation(
+              name: "doThingB()",
+              kind: .function,
+              location: try project.location(from: "2️⃣", to: "2️⃣", in: "FileB.swift")
+            )
+          )
+        ]
+      )
+      return true
+    }
+  }
 }