Skip to content

Commit 3d22598

Browse files
authored
improve manifest loading diagnostics handling (#3720)
motivation: manifest loading uses its own diagniostics engine to be able and communicate errors through the delegate, but the code did not actually use the right dignostics engine changes: more clearly name the variable and make sure the right diagnotics engine is used while loading the manifest
1 parent cde3573 commit 3d22598

File tree

1 file changed

+9
-11
lines changed

1 file changed

+9
-11
lines changed

Sources/Workspace/Workspace.swift

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,9 +1619,8 @@ extension Workspace {
16191619
diagnostics: DiagnosticsEngine,
16201620
completion: @escaping (Result<Manifest, Error>) -> Void
16211621
) {
1622-
// Load the manifest, bracketed by the calls to the delegate callbacks. The delegate callback is only passed any diagnostics emited during the parsing of the manifest, but they are also forwarded up to the caller.
1622+
// Load the manifest, bracketed by the calls to the delegate callbacks.
16231623
delegate?.willLoadManifest(packagePath: packagePath, url: packageLocation, version: version, packageKind: packageKind)
1624-
let manifestDiagnostics = DiagnosticsEngine(handlers: [{diagnostics.emit($0)}])
16251624
diagnostics.with(location: PackageLocation.Local(packagePath: packagePath)) { diagnostics in
16261625
do {
16271626
// Load the tools version for the package.
@@ -1631,6 +1630,8 @@ extension Workspace {
16311630
try toolsVersion.validateToolsVersion(currentToolsVersion, packagePath: packagePath.pathString)
16321631

16331632
// Load the manifest.
1633+
// The delegate callback is only passed any diagnostics emitted during the parsing of the manifest, but they are also forwarded up to the caller.
1634+
let manifestLoadingDiagnostics = DiagnosticsEngine(handlers: [{diagnostics.emit($0)}])
16341635
manifestLoader.load(at: packagePath,
16351636
packageIdentity: packageIdentity,
16361637
packageKind: packageKind,
@@ -1640,20 +1641,17 @@ extension Workspace {
16401641
toolsVersion: toolsVersion,
16411642
identityResolver: self.identityResolver,
16421643
fileSystem: localFileSystem,
1643-
diagnostics: diagnostics,
1644+
diagnostics: manifestLoadingDiagnostics,
16441645
on: .sharedConcurrent) { result in
16451646

16461647
switch result {
1648+
// Diagnostics.fatalError indicates that a more specific diagnostic has already been added.
1649+
case .failure(Diagnostics.fatalError):
1650+
break
16471651
case .failure(let error):
1648-
// Diagnostics.fatalError indicates that a more specific diagnostic has already been added.
1649-
switch error {
1650-
case Diagnostics.fatalError:
1651-
break
1652-
default:
1653-
diagnostics.emit(error)
1654-
}
1652+
diagnostics.emit(error)
16551653
case .success(let manifest):
1656-
self.delegate?.didLoadManifest(packagePath: packagePath, url: packageLocation, version: version, packageKind: packageKind, manifest: manifest, diagnostics: manifestDiagnostics.diagnostics)
1654+
self.delegate?.didLoadManifest(packagePath: packagePath, url: packageLocation, version: version, packageKind: packageKind, manifest: manifest, diagnostics: manifestLoadingDiagnostics.diagnostics)
16571655
}
16581656
completion(result)
16591657
}

0 commit comments

Comments
 (0)