Fully specify test IDs if they are not unique

It is possible for swift-testing test IDs to be identical in certain
circumstances. For instance if there are two parameterized tests where
only the type of the parameter differs. Another example is having two
identical @test definitions marked private in two different files in the
same test target.

To overcome this we do the same thing that SwiftPM does when running
`swift test list` in this situation, which is to fully qualify the
duplicate test IDs by appending their filename:line:column.

Issue: #1661

Author: plemarquand

Sources/SourceKitLSP/TestDiscovery.swift

@@ -433,6 +433,7 @@ fileprivate extension Array<AnnotatedTestItem> {
   /// A node's parent is identified by the node's ID with the last component dropped.
   func mergingTestsInExtensions() -> [TestItem] {
     var itemDict: [String: AnnotatedTestItem] = [:]
+    var duplicatedIds: Set<String> = []
     for item in self {
       let id = item.testItem.id
       if var rootItem = itemDict[id] {
@@ -443,6 +444,10 @@ fileprivate extension Array<AnnotatedTestItem> {
           var newItem = item
           newItem.testItem.children += rootItem.testItem.children
           rootItem = newItem
+        } else if (rootItem.testItem.children.isEmpty && item.testItem.children.isEmpty) {
+          duplicatedIds.insert(item.testItem.id)
+          itemDict[item.testItem.fullyQualifiedTestId] = item
+          continue
         } else {
           rootItem.testItem.children += item.testItem.children
         }
@@ -475,10 +480,26 @@ fileprivate extension Array<AnnotatedTestItem> {
       .sorted { ($0.isExtension != $1.isExtension) ? !$0.isExtension : ($0.testItem.location < $1.testItem.location) }
 
     let result = sortedItems.map {
+      var newItem = $0.testItem
+
+      // If multiple testItems share the same ID we add more context to make it unique.
+      // Two tests can share the same ID when two swift testing tests accept
+      // arguments of different types, i.e:
+      // @Test(arguments: [1,2,3]) func foo(_ x: Int) {}
+      // @Test(arguments: ["a", "b", "c"]) func foo(_ x: String) {}
+      // or when tests are in separate files but don't conflict because they are marked
+      // private, i.e:
+      // File1.swift: @Test private func foo() {}
+      // File2.swift: @Test private func foo() {}
+      // If we encounter one of these cases, we need to deduplicate the ID
+      // by appending /filename:filename:lineNumber.
+      if duplicatedIds.contains(newItem.id) {
+        newItem.id = newItem.fullyQualifiedTestId
+      }
+
       guard !$0.testItem.children.isEmpty, mergedIds.contains($0.testItem.id) else {
-        return $0.testItem
+        return newItem
       }
-      var newItem = $0.testItem
       newItem.children = newItem.children
         .map { AnnotatedTestItem(testItem: $0, isExtension: false) }
         .mergingTestsInExtensions()
@@ -498,6 +519,20 @@ fileprivate extension Array<AnnotatedTestItem> {
 }
 
 extension TestItem {
+  fileprivate var fullyQualifiedTestId: String {
+    return "\(self.id)/\(self.sourceLocation)"
+  }
+
+  private var sourceLocation: String {
+    let filename = self.location.uri.arbitrarySchemeURL.lastPathComponent
+    let position = location.range.lowerBound
+    // Lines and columns start at 1.
+    // swift-testing tests start from _after_ the @ symbol in @Test, so we need to add an extra column.
+    // see https://github.com/swiftlang/swift-testing/blob/cca6de2be617aded98ecdecb0b3b3a81eec013f3/Sources/TestingMacros/Support/AttributeDiscovery.swift#L153
+    let columnOffset = self.style == TestStyle.swiftTesting ? 2 : 1
+    return "\(filename):\(position.line + 1):\(position.utf16index + columnOffset)"
+  }
+
   fileprivate func prefixIDWithModuleName(workspace: Workspace) async -> TestItem {
     guard let canonicalTarget = await workspace.buildSystemManager.canonicalTarget(for: self.location.uri),
       let moduleName = await workspace.buildSystemManager.moduleName(for: self.location.uri, in: canonicalTarget)

Tests/SourceKitLSPTests/DocumentTestDiscoveryTests.swift

@@ -418,6 +418,43 @@ final class DocumentTestDiscoveryTests: XCTestCase {
     )
   }
 
+  func testSwiftTestingTestsWithDuplicateFunctionIdentifiers() async throws {
+    let testClient = try await TestSourceKitLSPClient()
+    let uri = DocumentURI(for: .swift)
+
+    let positions = testClient.openDocument(
+      """
+      import Testing
+
+      1️⃣@Test(arguments: [1, 2, 3])
+      func foo(_ x: Int) {}2️⃣
+      3️⃣@Test(arguments: ["a", "b", "c"])
+      func foo(_ x: String) {}4️⃣
+      """,
+      uri: uri
+    )
+
+    let filename = uri.fileURL?.lastPathComponent ?? ""
+    let tests = try await testClient.send(DocumentTestsRequest(textDocument: TextDocumentIdentifier(uri)))
+    XCTAssertEqual(
+      tests,
+      [
+        TestItem(
+          id: "foo(_:)/\(filename):\(positions["1️⃣"].line + 1):\(positions["1️⃣"].utf16index + 2)",
+          label: "foo(_:)",
+          style: TestStyle.swiftTesting,
+          location: Location(uri: uri, range: positions["1️⃣"]..<positions["2️⃣"])
+        ),
+        TestItem(
+          id: "foo(_:)/\(filename):\(positions["3️⃣"].line + 1):\(positions["3️⃣"].utf16index + 2)",
+          label: "foo(_:)",
+          style: TestStyle.swiftTesting,
+          location: Location(uri: uri, range: positions["3️⃣"]..<positions["4️⃣"])
+        ),
+      ]
+    )
+  }
+
   func testSwiftTestingSuiteWithNoTests() async throws {
     let testClient = try await TestSourceKitLSPClient()
     let uri = DocumentURI(for: .swift)

Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift

@@ -245,6 +245,49 @@ final class WorkspaceTestDiscoveryTests: XCTestCase {
     )
   }
 
+  func testSwiftTestingTestsWithDuplicateFunctionIdentifiersAcrossDocuments() async throws {
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Tests/MyLibraryTests/MyTests1.swift": """
+        import Testing
+
+        1️⃣@Test(arguments: [1, 2, 3])
+        private func foo(_ x: Int) {}2️⃣
+        """,
+        "Tests/MyLibraryTests/MyTests2.swift": """
+        import Testing
+
+        3️⃣@Test(arguments: [1, 2, 3])
+        private func foo(_ x: Int) {}4️⃣
+        """,
+      ],
+      manifest: packageManifestWithTestTarget
+    )
+
+    let test1Position = try project.position(of: "1️⃣", in: "MyTests1.swift")
+    let test2Position = try project.position(of: "3️⃣", in: "MyTests2.swift")
+
+    let tests = try await project.testClient.send(WorkspaceTestsRequest())
+
+    XCTAssertEqual(
+      tests,
+      [
+        TestItem(
+          id: "MyLibraryTests.foo(_:)/MyTests1.swift:\(test1Position.line + 1):\(test1Position.utf16index + 2)",
+          label: "foo(_:)",
+          style: TestStyle.swiftTesting,
+          location: try project.location(from: "1️⃣", to: "2️⃣", in: "MyTests1.swift")
+        ),
+        TestItem(
+          id: "MyLibraryTests.foo(_:)/MyTests2.swift:\(test2Position.line + 1):\(test2Position.utf16index + 2)",
+          label: "foo(_:)",
+          style: TestStyle.swiftTesting,
+          location: try project.location(from: "3️⃣", to: "4️⃣", in: "MyTests2.swift")
+        ),
+      ]
+    )
+  }
+
   func testSwiftTestingAndXCTestInTheSameFile() async throws {
     try SkipUnless.longTestsEnabled()