Skip to content

Commit f97cba5

Browse files
authored
fix warnings (#3849)
motivations: less warnings, safer code changes: * use package identifier for display name in collections * reduce use of DiagnosticsEngine where possible * remove use of PackageLocation from Git errors, metadata is already carried on assocaited error * adjust tests
1 parent d8d4c77 commit f97cba5

File tree

5 files changed

+19
-17
lines changed

5 files changed

+19
-17
lines changed

Sources/Commands/SwiftPackageTool.swift

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -488,18 +488,25 @@ extension SwiftPackageTool {
488488
_ comparisonResult: SwiftAPIDigester.ComparisonResult,
489489
observabilityScope: ObservabilityScope
490490
) {
491-
// TODO: use observabilityScope directly
492-
let diagnosticsEngine = observabilityScope.makeDiagnosticsEngine()
493491
for diagnostic in comparisonResult.otherDiagnostics {
492+
let metadata = diagnostic.location.map { location -> ObservabilityMetadata in
493+
var metadata = ObservabilityMetadata()
494+
metadata.fileLocation = .init(
495+
.init(location.filename),
496+
line: location.line < Int.max ? Int(location.line) : .none
497+
)
498+
return metadata
499+
}
500+
494501
switch diagnostic.level {
495502
case .error, .fatal:
496-
diagnosticsEngine.emit(error: diagnostic.text, location: diagnostic.location)
503+
observabilityScope.emit(error: diagnostic.text, metadata: metadata)
497504
case .warning:
498-
diagnosticsEngine.emit(warning: diagnostic.text, location: diagnostic.location)
505+
observabilityScope.emit(warning: diagnostic.text, metadata: metadata)
499506
case .note:
500-
diagnosticsEngine.emit(note: diagnostic.text, location: diagnostic.location)
507+
observabilityScope.emit(info: diagnostic.text, metadata: metadata)
501508
case .remark:
502-
diagnosticsEngine.emit(remark: diagnostic.text, location: diagnostic.location)
509+
observabilityScope.emit(info: diagnostic.text, metadata: metadata)
503510
case .ignored:
504511
break
505512
}

Sources/PackageCollections/Model/Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,6 @@ extension PackageCollectionsModel.Package.Version {
290290

291291
extension Model.Package {
292292
var displayName: String {
293-
self.latestVersion?.packageName ?? self.identity.description
293+
self.identity.description
294294
}
295295
}

Sources/PackageModel/Package.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,6 @@ extension ObservabilityMetadata {
143143
var metadata = ObservabilityMetadata()
144144
metadata.packageIdentity = identity
145145
metadata.packageKind = kind
146-
//metadata.packageLocation = location
147-
// FIXME: (diagnostics) remove once transition to Observability API is complete
148-
//metadata.legacyDiagnosticLocation = .init(PackageLocation.Local(name: identity.description, packagePath: path))
149146
return metadata
150147
}
151148
}

Sources/Workspace/SourceControlPackageContainer.swift

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
2323

2424
// A wrapper for getDependencies() errors. This adds additional information
2525
// about the container to identify it for diagnostics.
26-
public struct GetDependenciesError: Error, CustomStringConvertible, DiagnosticLocationProviding {
26+
public struct GetDependenciesError: Error, CustomStringConvertible {
2727

2828
/// The repository that encountered the error.
2929
public let repository: RepositorySpecifier
@@ -37,10 +37,6 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
3737
/// Optional suggestion for how to resolve the error.
3838
public let suggestion: String?
3939

40-
public var diagnosticLocation: DiagnosticLocation? {
41-
return PackageLocation.Remote(url: self.repository.location.description, reference: self.reference)
42-
}
43-
4440
/// Description shown for errors of this kind.
4541
public var description: String {
4642
var desc = "\(underlyingError) in \(self.repository.location)"

Tests/WorkspaceTests/PackageContainerProviderTests.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,15 +572,17 @@ class PackageContainerProviderTests: XCTestCase {
572572
catch let error as SourceControlPackageContainer.GetDependenciesError {
573573
// We expect to get an error message that mentions main.
574574
XCTAssertMatch(error.description, .and(.prefix("could not find a branch named ‘master’"), .suffix("(did you mean ‘main’?)")))
575-
XCTAssertMatch(error.diagnosticLocation?.description, .suffix("/SomePackage @ master"))
575+
XCTAssertMatch(error.repository.description, .suffix("/SomePackage"))
576+
XCTAssertMatch(error.reference, "master")
576577
}
577578

578579
// Simulate accessing a fictitious dependency on some random commit that doesn't exist, and check that we get back the expected error.
579580
do { _ = try container.getDependencies(at: "535f4cb5b4a0872fa691473e82d7b27b9894df00", productFilter: .everything) }
580581
catch let error as SourceControlPackageContainer.GetDependenciesError {
581582
// We expect to get an error message about the specific commit.
582583
XCTAssertMatch(error.description, .prefix("could not find the commit 535f4cb5b4a0872fa691473e82d7b27b9894df00"))
583-
XCTAssertMatch(error.diagnosticLocation?.description, .suffix("/SomePackage @ 535f4cb5b4a0872fa691473e82d7b27b9894df00"))
584+
XCTAssertMatch(error.repository.description, .suffix("/SomePackage"))
585+
XCTAssertMatch(error.reference, "535f4cb5b4a0872fa691473e82d7b27b9894df00")
584586
}
585587
}
586588
}

0 commit comments

Comments
 (0)