Skip to content

Commit 80d3dec

Browse files
committed
restore TSC Diagnostics Location behavior
motivation: downstream clients still depend on TSC Diagnostics Location changes: * preserve original TSC Location in metadata and bridge it back when exposing TSC diagnostics * adjust test
1 parent 0a0bee8 commit 80d3dec

File tree

10 files changed

+107
-113
lines changed

10 files changed

+107
-113
lines changed

Sources/Basics/Observability.swift

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ extension DiagnosticsEmitterProtocol {
174174
// FIXME: this brings in the TSC API still
175175
if let errorProvidingLocation = error as? DiagnosticLocationProviding, let diagnosticLocation = errorProvidingLocation.diagnosticLocation {
176176
metadata = metadata ?? ObservabilityMetadata()
177-
metadata?.legacyLocation = diagnosticLocation.description
177+
metadata?.legacyDiagnosticLocation = .init(diagnosticLocation)
178178
}
179179

180180
let message: String
@@ -465,7 +465,7 @@ extension Diagnostic {
465465
metadata = .none
466466
} else {
467467
metadata = ObservabilityMetadata()
468-
metadata?.legacyLocation = diagnostic.location.description
468+
metadata?.legacyDiagnosticLocation = .init(diagnostic.location)
469469
}
470470

471471
switch diagnostic.behavior {
@@ -504,8 +504,8 @@ extension ObservabilitySystem {
504504
extension TSCDiagnostic {
505505
init(_ diagnostic: Diagnostic) {
506506
let location: DiagnosticLocation
507-
if let metadata = diagnostic.metadata {
508-
location = metadata.legacyDiagnosticLocation
507+
if let legacyLocation = diagnostic.metadata?.legacyDiagnosticLocation {
508+
location = legacyLocation.underlying
509509
} else {
510510
location = UnknownLocation.location
511511
}
@@ -525,7 +525,7 @@ extension TSCDiagnostic {
525525

526526
//@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
527527
extension ObservabilityMetadata {
528-
public var legacyLocation: String? {
528+
public var legacyDiagnosticLocation: DiagnosticLocationWrapper? {
529529
get {
530530
self[LegacyLocationKey.self]
531531
}
@@ -535,24 +535,18 @@ extension ObservabilityMetadata {
535535
}
536536

537537
enum LegacyLocationKey: Key {
538-
typealias Value = String
538+
typealias Value = DiagnosticLocationWrapper
539539
}
540-
}
541540

542-
@available(*, deprecated, message: "temporary for transition DiagnosticsEngine -> DiagnosticsEmitter")
543-
extension ObservabilityMetadata {
544-
fileprivate var legacyDiagnosticLocation: DiagnosticLocation {
545-
if let legacyLocation = self.legacyLocation {
546-
return DiagnosticLocationWrapper(legacyLocation)
547-
}
548-
return UnknownLocation.location
549-
}
541+
public struct DiagnosticLocationWrapper: DiagnosticLocation {
542+
let underlying: DiagnosticLocation
550543

551-
private struct DiagnosticLocationWrapper: DiagnosticLocation {
552-
var description: String
544+
public init (_ underlying: DiagnosticLocation) {
545+
self.underlying = underlying
546+
}
553547

554-
init (_ location: String) {
555-
self.description = location
548+
public var description: String {
549+
self.underlying.description
556550
}
557551
}
558552
}

Sources/Commands/SwiftTool.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,10 +1066,10 @@ private final class InteractiveWriter {
10661066
// we should remove this as we make use of the new scope and metadata to provide better contextual information
10671067
extension ObservabilityMetadata {
10681068
fileprivate var diagnosticPrefix: String? {
1069-
if let legacyLocation = self.legacyLocation {
1070-
return legacyLocation
1071-
} else if let packageIdentity = self.packageIdentity, let packageLocation = self.packageLocation {
1069+
if let packageIdentity = self.packageIdentity, let packageLocation = self.packageLocation {
10721070
return "'\(packageIdentity)' \(packageLocation)"
1071+
} else if let legacyLocation = self.legacyDiagnosticLocation {
1072+
return legacyLocation.description
10731073
} else {
10741074
return .none
10751075
}

Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ extension PackageGraph {
9797
for node in allNodes {
9898
let nodeObservabilityScope = observabilityScope.makeChildScope(
9999
description: "loading package \(node.identity)",
100-
metadata: .packageMetadata(identity: node.identity, location: node.manifest.packageLocation)
100+
metadata: .packageMetadata(identity: node.identity, location: node.manifest.packageLocation, path: node.manifest.path.parentDirectory)
101101
)
102102

103103
let manifest = node.manifest

Sources/PackageLoading/PackageBuilder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ public final class PackageBuilder {
283283
self.warnAboutImplicitExecutableTargets = warnAboutImplicitExecutableTargets
284284
self.observabilityScope = observabilityScope.makeChildScope(
285285
description: "PackageBuilder",
286-
metadata: .packageMetadata(identity: self.identity, location: self.manifest.packageLocation)
286+
metadata: .packageMetadata(identity: self.identity, location: self.manifest.packageLocation, path: self.packagePath)
287287
)
288288
self.fileSystem = fileSystem
289289
}

Sources/PackageLoading/TargetSourcesBuilder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public struct TargetSourcesBuilder {
8080
self.fileSystem = fileSystem
8181

8282
self.observabilityScope = observabilityScope.makeChildScope(description: "TargetSourcesBuilder") {
83-
var metadata = ObservabilityMetadata.packageMetadata(identity: packageIdentity, location: packageLocation)
83+
var metadata = ObservabilityMetadata.packageMetadata(identity: packageIdentity, location: packageLocation, path: packagePath)
8484
metadata.targetName = target.name
8585
return metadata
8686
}

Sources/PackageModel/Package.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11+
import Basics
12+
import struct Foundation.URL
1113
import TSCBasic
1214
import TSCUtility
13-
1415
// Re-export Version from PackageModel, since it is a key part of the model.
1516
@_exported import struct TSCUtility.Version
16-
import Basics
1717

1818
/// The basic package representation.
1919
///
@@ -115,7 +115,7 @@ extension Package {
115115

116116
extension Package {
117117
public var diagnosticsMetadata: ObservabilityMetadata {
118-
return .packageMetadata(identity: self.identity, location: self.manifest.packageLocation)
118+
return .packageMetadata(identity: self.identity, location: self.manifest.packageLocation, path: self.path)
119119
}
120120
}
121121

@@ -139,12 +139,12 @@ extension Package.Error: CustomStringConvertible {
139139
}
140140

141141
extension ObservabilityMetadata {
142-
public static func packageMetadata(identity: PackageIdentity, location: String) -> Self {
142+
public static func packageMetadata(identity: PackageIdentity, location: String, path: AbsolutePath) -> Self {
143143
var metadata = ObservabilityMetadata()
144144
metadata.packageIdentity = identity
145145
metadata.packageLocation = location
146146
// FIXME: (diagnostics) remove once transition to Observability API is complete
147-
metadata.legacyLocation = "'\(identity)' \(location)"
147+
metadata.legacyDiagnosticLocation = .init(PackageLocation.Local(name: identity.description, packagePath: path))
148148
return metadata
149149
}
150150
}

Tests/PackageGraphTests/PackageGraphTests.swift

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -514,12 +514,12 @@ class PackageGraphTests: XCTestCase {
514514
result.check(
515515
diagnostic: "Source files for target Bar should be located under /Bar/Sources/Bar",
516516
severity: .warning,
517-
metadata: .packageMetadata(identity: .plain("bar"), location: "/Bar")
517+
metadata: .packageMetadata(identity: .plain("bar"), location: "/Bar", path: .init("/Bar"))
518518
)
519519
result.check(
520520
diagnostic: "target 'Bar' referenced in product 'Bar' is empty",
521521
severity: .error,
522-
metadata: .packageMetadata(identity: .plain("bar"), location: "/Bar")
522+
metadata: .packageMetadata(identity: .plain("bar"), location: "/Bar", path: .init("/Bar"))
523523
)
524524
}
525525
}
@@ -547,7 +547,7 @@ class PackageGraphTests: XCTestCase {
547547
result.check(
548548
diagnostic: "product 'Barx' required by package 'foo' target 'FooTarget' not found.",
549549
severity: .error,
550-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
550+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
551551
)
552552
}
553553
}
@@ -580,7 +580,7 @@ class PackageGraphTests: XCTestCase {
580580
result.check(
581581
diagnostic: "product 'Foo' is declared in the same package 'foo' and can't be used as a dependency for target 'FooTests'.",
582582
severity: .error,
583-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
583+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
584584
)
585585
}
586586
}
@@ -659,7 +659,7 @@ class PackageGraphTests: XCTestCase {
659659
result.check(
660660
diagnostic: "product 'Barx' required by package 'foo' target 'FooTarget' not found in package 'Bar'.",
661661
severity: .error,
662-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
662+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
663663
)
664664
}
665665
}
@@ -689,7 +689,7 @@ class PackageGraphTests: XCTestCase {
689689
result.check(
690690
diagnostic: "product 'Barx' required by package 'foo' target 'FooTarget' not found.",
691691
severity: .error,
692-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
692+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
693693
)
694694
}
695695
}
@@ -757,21 +757,21 @@ class PackageGraphTests: XCTestCase {
757757
dependency 'BarLib' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "BarLib", package: "Bar")'
758758
""",
759759
severity: .error,
760-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
760+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
761761
)
762762
result.checkUnordered(
763763
diagnostic: """
764764
dependency 'Biz' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "Biz", package: "BizPath")'
765765
""",
766766
severity: .error,
767-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
767+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
768768
)
769769
result.checkUnordered(
770770
diagnostic: """
771771
dependency 'FizLib' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "FizLib", package: "FizPath")'
772772
""",
773773
severity: .error,
774-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
774+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
775775
)
776776
}
777777
}
@@ -1354,7 +1354,7 @@ class PackageGraphTests: XCTestCase {
13541354
product 'Unknown' required by package 'foo' target 'Foo' not found.
13551355
""",
13561356
severity: .error,
1357-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1357+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
13581358
)
13591359
}
13601360
}
@@ -1436,7 +1436,7 @@ class PackageGraphTests: XCTestCase {
14361436
product 'Unknown' required by package 'foo' target 'Foo' not found.
14371437
""",
14381438
severity: .error,
1439-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1439+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
14401440
)
14411441
}
14421442
}
@@ -1479,7 +1479,7 @@ class PackageGraphTests: XCTestCase {
14791479
dependency 'ProductBar' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "ProductBar", package: "Bar")'
14801480
""",
14811481
severity: .error,
1482-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1482+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
14831483
)
14841484
}
14851485
}
@@ -1540,7 +1540,7 @@ class PackageGraphTests: XCTestCase {
15401540
dependency 'ProductBar' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "ProductBar", package: "Bar")'
15411541
""",
15421542
severity: .error,
1543-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1543+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
15441544
)
15451545
}
15461546
}
@@ -1600,7 +1600,7 @@ class PackageGraphTests: XCTestCase {
16001600
dependency 'Bar' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "Bar", package: "Some-Bar")'
16011601
""",
16021602
severity: .error,
1603-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1603+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
16041604
)
16051605
}
16061606
}
@@ -1659,7 +1659,7 @@ class PackageGraphTests: XCTestCase {
16591659
dependency 'ProductBar' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "ProductBar", package: "Some-Bar")'
16601660
""",
16611661
severity: .error,
1662-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1662+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
16631663
)
16641664
}
16651665
}
@@ -1756,7 +1756,7 @@ class PackageGraphTests: XCTestCase {
17561756
dependency 'ProductBar' in target 'Foo' requires explicit declaration; reference the package in the target dependency with '.product(name: "ProductBar", package: "Bar")'
17571757
""",
17581758
severity: .error,
1759-
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo")
1759+
metadata: .packageMetadata(identity: .plain("foo"), location: "/Foo", path: .init("/Foo"))
17601760
)
17611761
}
17621762
}

0 commit comments

Comments
 (0)