Merge pull request #1526 from ahoppen/6.0/re-index

[6.0] Add a request to re-index all files in SourceKit-LSP

Author: ahoppen

Documentation/LSP Extensions.md

@@ -458,3 +458,17 @@ New request that returns symbols for all the test classes and test methods withi
 ```ts
 export interface WorkspaceTestsParams {}
 ```
+
+## `workspace/triggerReindex`
+
+New request to re-index all files open in the SourceKit-LSP server.
+
+Users should not need to rely on this request. The index should always be updated automatically in the background. Having to invoke this request means there is a bug in SourceKit-LSP's automatic re-indexing. It does, however, offer a workaround to re-index files when such a bug occurs where otherwise there would be no workaround.
+
+
+- params: `TriggerReindexParams`
+- result: `void`
+
+```ts
+export interface TriggerReindexParams {}
+```

Sources/LanguageServerProtocol/CMakeLists.txt

@@ -77,6 +77,7 @@ add_library(LanguageServerProtocol STATIC
   Requests/ShutdownRequest.swift
   Requests/SignatureHelpRequest.swift
   Requests/SymbolInfoRequest.swift
+  Requests/TriggerReindexRequest.swift
   Requests/TypeDefinitionRequest.swift
   Requests/TypeHierarchyPrepareRequest.swift
   Requests/TypeHierarchySubtypesRequest.swift

Sources/LanguageServerProtocol/Requests/TriggerReindexRequest.swift

@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+/// Re-index all files open in the SourceKit-LSP server.
+///
+/// Users should not need to rely on this request. The index should always be updated automatically in the background.
+/// Having to invoke this request means there is a bug in SourceKit-LSP's automatic re-indexing. It does, however, offer
+/// a workaround to re-index files when such a bug occurs where otherwise there would be no workaround.
+///
+/// **LSP Extension**
+public struct TriggerReindexRequest: RequestType {
+  public static let method: String = "workspace/triggerReindex"
+  public typealias Response = VoidResponse
+
+  public init() {}
+}

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -238,15 +238,15 @@ public final actor SemanticIndexManager {
   /// Returns immediately after scheduling that task.
   ///
   /// Indexing is being performed with a low priority.
-  private func scheduleBackgroundIndex(files: some Collection<DocumentURI>) async {
-    _ = await self.scheduleIndexing(of: files, priority: .low)
+  private func scheduleBackgroundIndex(files: some Collection<DocumentURI>, indexFilesWithUpToDateUnit: Bool) async {
+    _ = await self.scheduleIndexing(of: files, indexFilesWithUpToDateUnit: indexFilesWithUpToDateUnit, priority: .low)
   }
 
   /// Regenerate the build graph (also resolving package dependencies) and then index all the source files known to the
   /// build system that don't currently have a unit with a timestamp that matches the mtime of the file.
   ///
   /// This method is intended to initially update the index of a project after it is opened.
-  public func scheduleBuildGraphGenerationAndBackgroundIndexAllFiles() async {
+  public func scheduleBuildGraphGenerationAndBackgroundIndexAllFiles(indexFilesWithUpToDateUnit: Bool = false) async {
     generateBuildGraphTask = Task(priority: .low) {
       await withLoggingSubsystemAndScope(subsystem: indexLoggingSubsystem, scope: "build-graph-generation") {
         logger.log(
@@ -270,16 +270,26 @@ public final actor SemanticIndexManager {
         // potentially not knowing about unit files, which causes the corresponding source files to be re-indexed.
         index.pollForUnitChangesAndWait()
         await testHooks.buildGraphGenerationDidFinish?()
-        let index = index.checked(for: .modifiedFiles)
-        let filesToIndex = await self.buildSystemManager.sourceFiles().lazy.map(\.uri)
-          .filter { !index.hasUpToDateUnit(for: $0) }
-        await scheduleBackgroundIndex(files: filesToIndex)
+        var filesToIndex: any Collection<DocumentURI> = await self.buildSystemManager.sourceFiles().lazy.map(\.uri)
+        if !indexFilesWithUpToDateUnit {
+          let index = index.checked(for: .modifiedFiles)
+          filesToIndex = filesToIndex.filter { !index.hasUpToDateUnit(for: $0) }
+        }
+        await scheduleBackgroundIndex(files: filesToIndex, indexFilesWithUpToDateUnit: indexFilesWithUpToDateUnit)
         generateBuildGraphTask = nil
       }
     }
     indexProgressStatusDidChange()
   }
 
+  /// Causes all files to be re-indexed even if the unit file for the source file is up to date.
+  /// See `TriggerReindexRequest`.
+  public func scheduleReindex() async {
+    await indexStoreUpToDateTracker.markAllKnownOutOfDate()
+    await preparationUpToDateTracker.markAllKnownOutOfDate()
+    await scheduleBuildGraphGenerationAndBackgroundIndexAllFiles(indexFilesWithUpToDateUnit: true)
+  }
+
   /// Wait for all in-progress index tasks to finish.
   public func waitForUpToDateIndex() async {
     logger.info("Waiting for up-to-date index")
@@ -319,7 +329,7 @@ public final actor SemanticIndexManager {
     // Create a new index task for the files that aren't up-to-date. The newly scheduled index tasks will
     // - Wait for the existing index operations to finish if they have the same number of files.
     // - Reschedule the background index task in favor of an index task with fewer source files.
-    await self.scheduleIndexing(of: uris, priority: nil).value
+    await self.scheduleIndexing(of: uris, indexFilesWithUpToDateUnit: false, priority: nil).value
     index.pollForUnitChangesAndWait()
     logger.debug("Done waiting for up-to-date index")
   }
@@ -354,7 +364,7 @@ public final actor SemanticIndexManager {
       await preparationUpToDateTracker.markOutOfDate(inProgressPreparationTasks.keys)
     }
 
-    await scheduleBackgroundIndex(files: changedFiles)
+    await scheduleBackgroundIndex(files: changedFiles, indexFilesWithUpToDateUnit: false)
   }
 
   /// Returns the files that should be indexed to get up-to-date index information for the given files.
@@ -507,6 +517,7 @@ public final actor SemanticIndexManager {
   /// Update the index store for the given files, assuming that their targets have already been prepared.
   private func updateIndexStore(
     for filesAndTargets: [FileAndTarget],
+    indexFilesWithUpToDateUnit: Bool,
     preparationTaskID: UUID,
     priority: TaskPriority?
   ) async {
@@ -516,6 +527,7 @@ public final actor SemanticIndexManager {
         buildSystemManager: self.buildSystemManager,
         index: index,
         indexStoreUpToDateTracker: indexStoreUpToDateTracker,
+        indexFilesWithUpToDateUnit: indexFilesWithUpToDateUnit,
         logMessageToIndexLog: logMessageToIndexLog,
         timeout: updateIndexStoreTimeout,
         testHooks: testHooks
@@ -553,6 +565,7 @@ public final actor SemanticIndexManager {
   /// The returned task finishes when all files are indexed.
   private func scheduleIndexing(
     of files: some Collection<DocumentURI>,
+    indexFilesWithUpToDateUnit: Bool,
     priority: TaskPriority?
   ) async -> Task<Void, Never> {
     // Perform a quick initial check to whether the files is up-to-date, in which case we don't need to schedule a
@@ -627,6 +640,7 @@ public final actor SemanticIndexManager {
               taskGroup.addTask {
                 await self.updateIndexStore(
                   for: fileBatch.map { FileAndTarget(file: $0, target: target) },
+                  indexFilesWithUpToDateUnit: indexFilesWithUpToDateUnit,
                   preparationTaskID: preparationTaskID,
                   priority: priority
                 )

Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift

@@ -106,12 +106,18 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
   /// The build system manager that is used to get the toolchain and build settings for the files to index.
   private let buildSystemManager: BuildSystemManager
 
-  private let indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>
-
   /// A reference to the underlying index store. Used to check if the index is already up-to-date for a file, in which
   /// case we don't need to index it again.
   private let index: UncheckedIndex
 
+  private let indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>
+
+  /// Whether files that have an up-to-date unit file should be indexed.
+  ///
+  /// In general, this should be `false`. The only situation when this should be set to `true` is when the user
+  /// explicitly requested a re-index of all files.
+  private let indexFilesWithUpToDateUnit: Bool
+
   /// See `SemanticIndexManager.logMessageToIndexLog`.
   private let logMessageToIndexLog: @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void
 
@@ -145,6 +151,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
     buildSystemManager: BuildSystemManager,
     index: UncheckedIndex,
     indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>,
+    indexFilesWithUpToDateUnit: Bool,
     logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void,
     timeout: Duration,
     testHooks: IndexTestHooks
@@ -153,6 +160,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
     self.buildSystemManager = buildSystemManager
     self.index = index
     self.indexStoreUpToDateTracker = indexStoreUpToDateTracker
+    self.indexFilesWithUpToDateUnit = indexFilesWithUpToDateUnit
     self.logMessageToIndexLog = logMessageToIndexLog
     self.timeout = timeout
     self.testHooks = testHooks
@@ -214,7 +222,9 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
       // If we know that the file is up-to-date without having ot hit the index, do that because it's fastest.
       return
     }
-    guard !index.checked(for: .modifiedFiles).hasUpToDateUnit(for: file.sourceFile, mainFile: file.mainFile)
+    guard
+      indexFilesWithUpToDateUnit
+        || !index.checked(for: .modifiedFiles).hasUpToDateUnit(for: file.sourceFile, mainFile: file.mainFile)
     else {
       logger.debug("Not indexing \(file.forLogging) because index has an up-to-date unit")
       // We consider a file's index up-to-date if we have any up-to-date unit. Changing build settings does not

Sources/SourceKitLSP/MessageHandlingDependencyTracker.swift

@@ -199,6 +199,8 @@ enum MessageHandlingDependencyTracker: DependencyTracker {
       self = .freestanding
     case is ShutdownRequest:
       self = .globalConfigurationChange
+    case is TriggerReindexRequest:
+      self = .globalConfigurationChange
     case is TypeHierarchySubtypesRequest:
       self = .freestanding
     case is TypeHierarchySupertypesRequest:

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -762,6 +762,8 @@ extension SourceKitLSPServer: MessageHandler {
       await self.handleRequest(for: request, requestHandler: self.prepareRename)
     case let request as RequestAndReply<IndexedRenameRequest>:
       await self.handleRequest(for: request, requestHandler: self.indexedRename)
+    case let request as RequestAndReply<TriggerReindexRequest>:
+      await request.reply { try await triggerReindex(request.params) }
     // IMPORTANT: When adding a new entry to this switch, also add it to the `MessageHandlingDependencyTracker` initializer.
     default:
       await request.reply { throw ResponseError.methodNotFound(R.method) }
@@ -2340,6 +2342,13 @@ extension SourceKitLSPServer {
     }
     return VoidResponse()
   }
+
+  func triggerReindex(_ req: TriggerReindexRequest) async throws -> VoidResponse {
+    for workspace in workspaces {
+      await workspace.semanticIndexManager?.scheduleReindex()
+    }
+    return VoidResponse()
+  }
 }
 
 private func languageClass(for language: Language) -> [Language] {

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -1275,6 +1275,104 @@ final class BackgroundIndexingTests: XCTestCase {
     // also testing that we don't wait for type checking of Test.swift to finish.
     XCTAssert(Date().timeIntervalSince(dateStarted) < 30)
   }
+
+  func testManualReindex() async throws {
+    // This test relies on the issue described in https://github.com/apple/sourcekit-lsp/issues/1264 that we don't
+    // re-index dependent files if a function of a low-level module gains a new default parameter, which changes the
+    // function's USR but is API compatible with all dependencies.
+    // Check that after running the re-index request, the index gets updated.
+
+    let project = try await SwiftPMTestProject(
+      files: [
+        "LibA/LibA.swift": """
+        public func 1️⃣getInt() -> Int {
+          return 1
+        }
+        """,
+        "LibB/LibB.swift": """
+        import LibA
+
+        public func 2️⃣test() -> Int {
+          return 3️⃣getInt()
+        }
+        """,
+      ],
+      manifest: """
+        let package = Package(
+          name: "MyLibrary",
+          targets: [
+           .target(name: "LibA"),
+           .target(name: "LibB", dependencies: ["LibA"]),
+          ]
+        )
+        """,
+      enableBackgroundIndexing: true
+    )
+
+    let expectedCallHierarchyItem = CallHierarchyIncomingCall(
+      from: CallHierarchyItem(
+        name: "test()",
+        kind: .function,
+        tags: nil,
+        uri: try project.uri(for: "LibB.swift"),
+        range: try project.range(from: "2️⃣", to: "2️⃣", in: "LibB.swift"),
+        selectionRange: try project.range(from: "2️⃣", to: "2️⃣", in: "LibB.swift"),
+        data: .dictionary([
+          "usr": .string("s:4LibB4testSiyF"),
+          "uri": .string(try project.uri(for: "LibB.swift").stringValue),
+        ])
+      ),
+      fromRanges: [try project.range(from: "3️⃣", to: "3️⃣", in: "LibB.swift")]
+    )
+
+    /// Start by making a call hierarchy request to check that we get the expected results without any edits.
+    let (uri, positions) = try project.openDocument("LibA.swift")
+    let prepareBeforeUpdate = try await project.testClient.send(
+      CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    let callHierarchyBeforeUpdate = try await project.testClient.send(
+      CallHierarchyIncomingCallsRequest(item: XCTUnwrap(prepareBeforeUpdate?.only))
+    )
+    XCTAssertEqual(callHierarchyBeforeUpdate, [expectedCallHierarchyItem])
+
+    // Now add a new default parameter to `getInt`.
+    project.testClient.send(DidCloseTextDocumentNotification(textDocument: TextDocumentIdentifier(uri)))
+    let newLibAContents = """
+      public func getInt(value: Int = 1) -> Int {
+        return value
+      }
+      """
+    try newLibAContents.write(to: XCTUnwrap(uri.fileURL), atomically: true, encoding: .utf8)
+    project.testClient.send(
+      DidOpenTextDocumentNotification(
+        textDocument: TextDocumentItem(uri: uri, language: .swift, version: 0, text: newLibAContents)
+      )
+    )
+    project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: uri, type: .changed)]))
+    _ = try await project.testClient.send(PollIndexRequest())
+
+    // The USR of `getInt` has changed but LibB.swift has not been re-indexed due to
+    // https://github.com/apple/sourcekit-lsp/issues/1264. We expect to get an empty call hierarchy.
+    let prepareAfterUpdate = try await project.testClient.send(
+      CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    let callHierarchyAfterUpdate = try await project.testClient.send(
+      CallHierarchyIncomingCallsRequest(item: XCTUnwrap(prepareAfterUpdate?.only))
+    )
+    XCTAssertEqual(callHierarchyAfterUpdate, [])
+
+    // After re-indexing, we expect to get a full call hierarchy again.
+    _ = try await project.testClient.send(TriggerReindexRequest())
+    _ = try await project.testClient.send(PollIndexRequest())
+
+    let prepareAfterReindex = try await project.testClient.send(
+      CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    let callHierarchyAfterReindex = try await project.testClient.send(
+      CallHierarchyIncomingCallsRequest(item: XCTUnwrap(prepareAfterReindex?.only))
+    )
+    XCTAssertEqual(callHierarchyAfterReindex, [expectedCallHierarchyItem])
+  }
 }
 
 extension HoverResponseContents {