preserve TSC Diagnostic location and data (#3792)

motivation: downstream clients still depend on TSC Diagnostics Location

changes:
* preserve original TSC location in metadata and bridge it back when exposing TSC diagnostics
* preserve original TSC data in metadata and bridge it back when exposing TSC diagnostics
* adjust and add tests

Author: tomerd

Sources/Basics/Observability.swift

@@ -174,7 +174,7 @@ extension DiagnosticsEmitterProtocol {
         // FIXME: this brings in the TSC API still
         if let errorProvidingLocation = error as? DiagnosticLocationProviding, let diagnosticLocation = errorProvidingLocation.diagnosticLocation {
             metadata = metadata ?? ObservabilityMetadata()
-            metadata?.legacyLocation = diagnosticLocation.description
+            metadata?.legacyDiagnosticLocation = .init(diagnosticLocation)
         }
 
         let message: String
@@ -422,6 +422,21 @@ public struct ObservabilityMetadata: Equatable, CustomDebugStringConvertible {
         }
     }
 
+    //@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
+    public func droppingLegacyKeys() -> ObservabilityMetadata? {
+        var metadata = ObservabilityMetadata()
+        self.forEach { (key, value) in
+            if key.keyType == LegacyLocationKey.self {
+                return
+            }
+            if key.keyType == LegacyDataKey.self {
+                return
+            }
+            metadata._storage[key] = value
+        }
+        return metadata.isEmpty ? .none : metadata
+    }
+
     /// A type-erased `ObservabilityMetadataKey` used when iterating through the `ObservabilityMetadata` using its `forEach` method.
     public struct AnyKey {
         /// The key's type represented erased to an `Any.Type`.
@@ -460,13 +475,11 @@ extension ObservabilityScope {
 //@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
 extension Diagnostic {
     init?(_ diagnostic: TSCDiagnostic) {
-        var metadata: ObservabilityMetadata?
-        if diagnostic.location is UnknownLocation {
-            metadata = .none
-        } else {
-            metadata = ObservabilityMetadata()
-            metadata?.legacyLocation = diagnostic.location.description
+        var metadata = ObservabilityMetadata()
+        if !(diagnostic.location is UnknownLocation) {
+            metadata.legacyDiagnosticLocation = .init(diagnostic.location)
         }
+        metadata.legacyDiagnosticData = .init(diagnostic.data)
 
         switch diagnostic.behavior {
         case .error:
@@ -504,28 +517,35 @@ extension ObservabilitySystem {
 extension TSCDiagnostic {
     init(_ diagnostic: Diagnostic) {
         let location: DiagnosticLocation
-        if let metadata = diagnostic.metadata {
-            location = metadata.legacyDiagnosticLocation
+        if let legacyLocation = diagnostic.metadata?.legacyDiagnosticLocation {
+            location = legacyLocation.underlying
         } else {
             location = UnknownLocation.location
         }
 
+        let data: DiagnosticData
+        if let legacyData = diagnostic.metadata?.legacyDiagnosticData {
+            data = legacyData.underlying
+        } else {
+            data = StringDiagnostic(diagnostic.message)
+        }
+
         switch diagnostic.severity {
         case .error:
-            self = .init(message: .error(diagnostic.message), location: location)
+            self = .init(message: .error(data), location: location)
         case .warning:
-            self = .init(message: .warning(diagnostic.message), location: location)
+            self = .init(message: .warning(data), location: location)
         case .info:
-            self = .init(message: .note(diagnostic.message), location: location)
+            self = .init(message: .note(data), location: location)
         case .debug:
-            self = .init(message: .note(diagnostic.message), location: location)
+            self = .init(message: .note(data), location: location)
         }
     }
 }
 
 //@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
 extension ObservabilityMetadata {
-    public var legacyLocation: String? {
+    public var legacyDiagnosticLocation: DiagnosticLocationWrapper? {
         get {
             self[LegacyLocationKey.self]
         }
@@ -534,25 +554,47 @@ extension ObservabilityMetadata {
         }
     }
 
-    enum LegacyLocationKey: Key {
-        typealias Value = String
+    private enum LegacyLocationKey: Key {
+        typealias Value = DiagnosticLocationWrapper
+    }
+
+    public struct DiagnosticLocationWrapper: CustomStringConvertible {
+        let underlying: DiagnosticLocation
+
+        public init (_ underlying: DiagnosticLocation) {
+            self.underlying = underlying
+        }
+
+        public var description: String {
+            self.underlying.description
+        }
     }
 }
 
-@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
+//@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
 extension ObservabilityMetadata {
-    fileprivate var legacyDiagnosticLocation: DiagnosticLocation {
-        if let legacyLocation = self.legacyLocation {
-            return DiagnosticLocationWrapper(legacyLocation)
+    var legacyDiagnosticData: DiagnosticDataWrapper? {
+        get {
+            self[LegacyDataKey.self]
         }
-        return UnknownLocation.location
+        set {
+            self[LegacyDataKey.self] = newValue
+        }
+    }
+
+    private enum LegacyDataKey: Key {
+        typealias Value = DiagnosticDataWrapper
     }
 
-    private struct DiagnosticLocationWrapper: DiagnosticLocation {
-        var description: String
+    struct DiagnosticDataWrapper: CustomStringConvertible {
+        let underlying: DiagnosticData
+
+        public init (_ underlying: DiagnosticData) {
+            self.underlying = underlying
+        }
 
-        init (_ location: String) {
-            self.description = location
+        public var description: String {
+            self.underlying.description
         }
     }
 }

Sources/Commands/SwiftTool.swift

@@ -1066,10 +1066,10 @@ private final class InteractiveWriter {
 // we should remove this as we make use of the new scope and metadata to provide better contextual information
 extension ObservabilityMetadata {
     fileprivate var diagnosticPrefix: String? {
-        if let legacyLocation = self.legacyLocation {
-            return legacyLocation
-        } else if let packageIdentity = self.packageIdentity, let packageLocation = self.packageLocation {
+        if let packageIdentity = self.packageIdentity, let packageLocation = self.packageLocation {
             return "'\(packageIdentity)' \(packageLocation)"
+        } else if let legacyLocation = self.legacyDiagnosticLocation {
+            return legacyLocation.description
         } else {
             return .none
         }

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -97,7 +97,7 @@ extension PackageGraph {
         for node in allNodes {
             let nodeObservabilityScope = observabilityScope.makeChildScope(
                 description: "loading package \(node.identity)",
-                metadata: .packageMetadata(identity: node.identity, location: node.manifest.packageLocation)
+                metadata: .packageMetadata(identity: node.identity, location: node.manifest.packageLocation, path: node.manifest.path.parentDirectory)
             )
 
             let manifest = node.manifest

Sources/PackageLoading/PackageBuilder.swift

@@ -283,7 +283,7 @@ public final class PackageBuilder {
         self.warnAboutImplicitExecutableTargets = warnAboutImplicitExecutableTargets
         self.observabilityScope = observabilityScope.makeChildScope(
             description: "PackageBuilder",
-            metadata: .packageMetadata(identity: self.identity, location: self.manifest.packageLocation)
+            metadata: .packageMetadata(identity: self.identity, location: self.manifest.packageLocation, path: self.packagePath)
         )
         self.fileSystem = fileSystem
     }

Sources/PackageLoading/TargetSourcesBuilder.swift

@@ -80,7 +80,7 @@ public struct TargetSourcesBuilder {
         self.fileSystem = fileSystem
 
         self.observabilityScope = observabilityScope.makeChildScope(description: "TargetSourcesBuilder") {
-            var metadata = ObservabilityMetadata.packageMetadata(identity: packageIdentity, location: packageLocation)
+            var metadata = ObservabilityMetadata.packageMetadata(identity: packageIdentity, location: packageLocation, path: packagePath)
             metadata.targetName = target.name
             return metadata
         }

Sources/PackageModel/Package.swift

@@ -8,12 +8,12 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
+import Basics
+import struct Foundation.URL
 import TSCBasic
 import TSCUtility
-
 // Re-export Version from PackageModel, since it is a key part of the model.
 @_exported import struct TSCUtility.Version
-import Basics
 
 /// The basic package representation.
 ///
@@ -115,7 +115,7 @@ extension Package {
 
 extension Package {
     public var diagnosticsMetadata: ObservabilityMetadata {
-        return .packageMetadata(identity: self.identity, location: self.manifest.packageLocation)
+        return .packageMetadata(identity: self.identity, location: self.manifest.packageLocation, path: self.path)
     }
 }
 
@@ -139,12 +139,12 @@ extension Package.Error: CustomStringConvertible {
 }
 
 extension ObservabilityMetadata {
-    public static func packageMetadata(identity: PackageIdentity, location: String) -> Self {
+    public static func packageMetadata(identity: PackageIdentity, location: String, path: AbsolutePath) -> Self {
         var metadata = ObservabilityMetadata()
         metadata.packageIdentity = identity
         metadata.packageLocation = location
         // FIXME: (diagnostics) remove once transition to Observability API is complete
-        metadata.legacyLocation = "'\(identity)' \(location)"
+        metadata.legacyDiagnosticLocation = .init(PackageLocation.Local(name: identity.description, packagePath: path))
         return metadata
     }
 }

Sources/SPMTestSupport/Observability.swift

@@ -126,7 +126,9 @@ public class DiagnosticsTestResult {
 
         XCTAssertMatch(diagnostic.message, message, file: file, line: line)
         XCTAssertEqual(diagnostic.severity, severity, file: file, line: line)
-        XCTAssertEqual(diagnostic.metadata, metadata, file: file, line: line)
+        // FIXME: (diagnostics) compare complete metadata when legacy bridge is removed
+        //XCTAssertEqual(diagnostic.metadata, metadata, file: file, line: line)
+        XCTAssertEqual(diagnostic.metadata?.droppingLegacyKeys(), metadata?.droppingLegacyKeys(), file: file, line: line)
     }
 
     public func checkUnordered(
@@ -145,9 +147,13 @@ public class DiagnosticsTestResult {
             return XCTFail("No diagnostics match \(diagnosticPattern)", file: file, line: line)
         } else if matching.count == 1, let diagnostic = matching.first, let index = self.uncheckedDiagnostics.firstIndex(where: { $0 == diagnostic }) {
             XCTAssertEqual(diagnostic.severity, severity, file: file, line: line)
-            XCTAssertEqual(diagnostic.metadata, metadata, file: file, line: line)
+            // FIXME: (diagnostics) compare complete metadata when legacy bridge is removed
+            //XCTAssertEqual(diagnostic.metadata, metadata, file: file, line: line)
+            XCTAssertEqual(diagnostic.metadata?.droppingLegacyKeys(), metadata?.droppingLegacyKeys(), file: file, line: line)
             self.uncheckedDiagnostics.remove(at: index)
-        } else if let index = self.uncheckedDiagnostics.firstIndex(where: { diagnostic in diagnostic.severity == severity && diagnostic.metadata == metadata}) {
+        // FIXME: (diagnostics) compare complete metadata when legacy bridge is removed
+        //} else if let index = self.uncheckedDiagnostics.firstIndex(where: { diagnostic in diagnostic.severity == severity && diagnostic.metadata == metadata}) {
+        } else if let index = self.uncheckedDiagnostics.firstIndex(where: { diagnostic in diagnostic.severity == severity && diagnostic.metadata?.droppingLegacyKeys() == metadata?.droppingLegacyKeys()}) {
             self.uncheckedDiagnostics.remove(at: index)
         }
     }

Tests/BasicsTests/ObservabilitySystemTests.swift

@@ -160,6 +160,51 @@ final class ObservabilitySystemTest: XCTestCase {
         }
     }
 
+    @available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
+    func testBridging() throws {
+        do {
+            let collector = Collector()
+            let observabilitySystem = ObservabilitySystem(collector)
+            let diagnosticsEngine = observabilitySystem.topScope.makeDiagnosticsEngine()
+
+            let data = TestData()
+            let location = TestLocation()
+
+            diagnosticsEngine.emit(.error(data), location: location)
+
+            var expectedMetadata = ObservabilityMetadata()
+            expectedMetadata.legacyDiagnosticLocation = .init(location)
+            expectedMetadata.legacyDiagnosticData = .init(data)
+
+            XCTAssertEqual(collector.diagnostics.count, 1)
+            XCTAssertEqual(collector.diagnostics.first?.metadata, expectedMetadata)
+        }
+
+        do {
+            let diagnosticsEngine1 = DiagnosticsEngine()
+            let observabilitySystem = ObservabilitySystem(diagnosticEngine: diagnosticsEngine1)
+            let diagnosticsEngine2 = observabilitySystem.topScope.makeDiagnosticsEngine()
+
+            let data = TestData()
+            let location = TestLocation()
+
+            diagnosticsEngine2.emit(.error(data), location: location)
+
+            XCTAssertEqual(diagnosticsEngine1.diagnostics.count, 1)
+            XCTAssertEqual(diagnosticsEngine1.diagnostics.first!.message.data as? TestData, data)
+            XCTAssertEqual(diagnosticsEngine1.diagnostics.first!.location as? TestLocation, location)
+        }
+
+        struct TestData: DiagnosticData, Equatable {
+            var description: String = UUID().uuidString
+        }
+
+        struct TestLocation: DiagnosticLocation, Equatable {
+            var description: String = UUID().uuidString
+
+        }
+    }
+
     struct Collector: ObservabilityHandlerProvider, DiagnosticsHandler {
         private let _diagnostics = ThreadSafeArrayStore<Diagnostic>()