Prepend module name to TestItem IDs

It is possible to have two identically named suites in two different
test targets. These were being erroniously rolled up in to the same
parent TestItem.

Disambiguate these TestItems by prepending the module name. This has the
added benefit of making the TestItem IDs a fully qualified name that can
be passed to `swift test`.

The module name is pulled from the compiler arguments for the target. If
no module name can be found we fall back to the `targetID` for the
`ConfiguredTarget`.

Author: plemarquand

Sources/SKCore/BuildSystemManager.swift

@@ -14,6 +14,7 @@ import BuildServerProtocol
 import Dispatch
 import LSPLogging
 import LanguageServerProtocol
+import SwiftExtensions
 
 import struct TSCBasic.AbsolutePath
 
@@ -154,6 +155,37 @@ extension BuildSystemManager {
       .first
   }
 
+  /// Returns the target's module name as parsed from the `ConfiguredTarget`'s compiler arguments.
+  public func moduleName(for document: DocumentURI, in target: ConfiguredTarget) async -> String? {
+    guard let language = await self.defaultLanguage(for: document),
+      let buildSettings = await buildSettings(for: document, in: target, language: language)
+    else {
+      return nil
+    }
+
+    switch language {
+    case .swift:
+      // Module name is specified in the form -module-name MyLibrary
+      guard let moduleNameFlagIndex = buildSettings.compilerArguments.firstIndex(of: "-module-name") else {
+        return nil
+      }
+      return buildSettings.compilerArguments[safe: moduleNameFlagIndex + 1]
+    case .objective_c:
+      // Specified in the form -fmodule-name=MyLibrary
+      guard
+        let moduleNameArgument = buildSettings.compilerArguments.first(where: {
+          $0.starts(with: "-fmodule-name=")
+        }),
+        let moduleName = moduleNameArgument.split(separator: "=").last
+      else {
+        return nil
+      }
+      return String(moduleName)
+    default:
+      return nil
+    }
+  }
+
   /// Returns the build settings for `document` from `buildSystem`.
   ///
   /// Implementation detail of `buildSettings(for:language:)`.

Sources/SourceKitLSP/TestDiscovery.swift

@@ -260,7 +260,7 @@ extension SourceKitLSPServer {
 
   func workspaceTests(_ req: WorkspaceTestsRequest) async throws -> [TestItem] {
     return await self.workspaces
-      .concurrentMap { await self.tests(in: $0) }
+      .concurrentMap { await self.tests(in: $0).prefixTestsWithModuleName(workspace: $0) }
       .flatMap { $0 }
       .sorted { $0.testItem.location < $1.testItem.location }
       .mergingTestsInExtensions()
@@ -272,6 +272,7 @@ extension SourceKitLSPServer {
     languageService: LanguageService
   ) async throws -> [TestItem] {
     return try await documentTestsWithoutMergingExtensions(req, workspace: workspace, languageService: languageService)
+      .prefixTestsWithModuleName(workspace: workspace)
       .mergingTestsInExtensions()
   }
 
@@ -476,6 +477,30 @@ fileprivate extension Array<AnnotatedTestItem> {
     }
     return result
   }
+
+  func prefixTestsWithModuleName(workspace: Workspace) async -> Self {
+    return await self.asyncMap({
+      return AnnotatedTestItem(
+        testItem: await $0.testItem.prefixIDWithModuleName(workspace: workspace),
+        isExtension: $0.isExtension
+      )
+    })
+  }
+}
+
+extension TestItem {
+  fileprivate func prefixIDWithModuleName(workspace: Workspace) async -> TestItem {
+    guard let configuredTarget = await workspace.buildSystemManager.canonicalConfiguredTarget(for: self.location.uri),
+      let moduleName = await workspace.buildSystemManager.moduleName(for: self.location.uri, in: configuredTarget)
+    else {
+      return self
+    }
+
+    var newTest = self
+    newTest.id = "\(moduleName).\(newTest.id)"
+    newTest.children = await newTest.children.asyncMap({ await $0.prefixIDWithModuleName(workspace: workspace) })
+    return newTest
+  }
 }
 
 extension SwiftLanguageService {

Sources/SwiftExtensions/Array+Safe.swift

@@ -0,0 +1,19 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2018 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
+//
+//===----------------------------------------------------------------------===//
+
+extension Array {
+  /// Returns the element at the specified index if it is within the Array's
+  /// bounds, otherwise `nil`.
+  public subscript(safe index: Index) -> Element? {
+    return index >= 0 && index < count ? self[index] : nil
+  }
+}

Sources/SwiftExtensions/CMakeLists.txt

@@ -1,5 +1,6 @@
 
 add_library(SwiftExtensions STATIC
+  Array+Safe.swift
   AsyncQueue.swift
   AsyncUtils.swift
   Collection+Only.swift