Fix a bug that would cause a file to never be part of the syntactic index again after it got deleted once

Author: ahoppen

Sources/SourceKitLSP/Swift/SyntacticTestIndex.swift

@@ -39,10 +39,10 @@ fileprivate enum TaskMetadata: DependencyTracker, Equatable {
       // Adding the dependency also elevates the index task's priorities.
       return true
     case (.index(let lhsUris), .index(let rhsUris)):
-      // Technically, we should be able to allow simultaneous indexing of the same file. When a file gets re-scheduled
-      // for indexing, all previous index invocations should get cancelled. But conceptually the code becomes simpler
-      // if we don't need to think racing indexing tasks for the same file and it shouldn't make a performance impact
-      // in practice because of the cancellation described before.
+      // Technically, we should be able to allow simultaneous indexing of the same file. But conceptually the code
+      // becomes simpler if we don't need to think racing indexing tasks for the same file and it shouldn't make a
+      // performance impact in practice because if a first task indexes a file, a subsequent index task for the same
+      // file will realize that the index is already up-to-date based on the file's mtime and early exit.
       return !lhsUris.intersection(rhsUris).isEmpty
     }
   }
@@ -82,14 +82,18 @@ fileprivate func testItems(in url: URL) async -> [TestItem] {
   return await swiftTestingTests + xcTests
 }
 
+/// An in-memory syntactic index of test items within a workspace.
+///
+/// The index does not get persisted to disk but instead gets rebuilt every time a workspace is opened (ie. usually when
+/// sourcekit-lsp is launched). Building it takes only a few seconds, even for large projects.
 actor SyntacticTestIndex {
   /// The tests discovered by the index.
   private var indexedTests: [DocumentURI: IndexedTests] = [:]
 
   /// Files that have been removed using `removeFileForIndex`.
   ///
   /// We need to keep track of these files because when the files get removed, there might be an in-progress indexing
-  /// operation running for that file. We need to ensure that this indexing operation doesn't write add the removed file
+  /// operation running for that file. We need to ensure that this indexing operation doesn't add the removed file
   /// back to `indexTests`.
   private var removedFiles: Set<DocumentURI> = []
 
@@ -138,6 +142,10 @@ actor SyntacticTestIndex {
 
   /// Called when a list of files was updated. Re-scans those files
   private func rescanFiles(_ uris: [DocumentURI]) {
+    // If we scan a file again, it might have been added after being removed before. Remove it from the list of removed
+    // files.
+    removedFiles.subtract(uris)
+
     // Divide the files into multiple batches. This is more efficient than spawning a new task for every file, mostly
     // because it keeps the number of pending items in `indexingQueue` low and adding a new task to `indexingQueue` is
     // in O(number of pending tasks), since we need to scan for dependency edges to add, which would make scanning files

Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift

@@ -748,4 +748,63 @@ final class WorkspaceTestDiscoveryTests: XCTestCase {
       ]
     )
   }
+
+  func testDeleteFileAndAddItAgain() async throws {
+    let markedFileContents = """
+      import XCTest
+
+      1️⃣class MyTests: XCTestCase {
+        2️⃣func testMyLibrary() {}3️⃣
+        func unrelatedFunc() {}
+        var testVariable: Int = 0
+      }4️⃣
+      """
+
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Tests/MyLibraryTests/MyTests.swift": markedFileContents
+      ],
+      manifest: packageManifestWithTestTarget
+    )
+
+    let expectedTests = [
+      TestItem(
+        id: "MyTests",
+        label: "MyTests",
+        disabled: false,
+        style: TestStyle.xcTest,
+        location: try project.location(from: "1️⃣", to: "4️⃣", in: "MyTests.swift"),
+        children: [
+          TestItem(
+            id: "MyTests/testMyLibrary()",
+            label: "testMyLibrary()",
+            disabled: false,
+            style: TestStyle.xcTest,
+            location: try project.location(from: "2️⃣", to: "3️⃣", in: "MyTests.swift"),
+            children: [],
+            tags: []
+          )
+        ],
+        tags: []
+      )
+    ]
+
+    let testsBeforeFileRemove = try await project.testClient.send(WorkspaceTestsRequest())
+    XCTAssertEqual(testsBeforeFileRemove, expectedTests)
+
+    let myTestsUri = try project.uri(for: "MyTests.swift")
+    let myTestsUrl = try XCTUnwrap(myTestsUri.fileURL)
+
+    try FileManager.default.removeItem(at: myTestsUrl)
+    project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: myTestsUri, type: .deleted)]))
+
+    let testsAfterFileRemove = try await project.testClient.send(WorkspaceTestsRequest())
+    XCTAssertEqual(testsAfterFileRemove, [])
+
+    try extractMarkers(markedFileContents).textWithoutMarkers.write(to: myTestsUrl, atomically: true, encoding: .utf8)
+    project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: myTestsUri, type: .created)]))
+
+    let testsAfterFileReAdded = try await project.testClient.send(WorkspaceTestsRequest())
+    XCTAssertEqual(testsAfterFileReAdded, expectedTests)
+  }
 }