Skip to content

Commit 3502dcb

Browse files
hartbithartbit
committed
Migrate Workspace delegate warnings to diagnostics
1 parent c258e1f commit 3502dcb

File tree

4 files changed

+115
-34
lines changed

4 files changed

+115
-34
lines changed

‎Sources/Workspace/Diagnostics.swift

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ public enum ResolverDiagnostics {
109109

110110
public enum WorkspaceDiagnostics {
111111

112+
//MARK: - Errors
113+
112114
/// The diagnostic triggered when an operation fails because its completion
113115
/// would loose the uncommited changes in a repository.
114116
public struct UncommitedChanges: DiagnosticData, Swift.Error {
@@ -261,4 +263,84 @@ public enum WorkspaceDiagnostics {
261263
/// The package found at the edit location.
262264
public let destinationPackage: String?
263265
}
266+
267+
//MARK: - Warnings
268+
269+
/// The diagnostic triggered when a checked-out dependency is missing
270+
/// from the file-system.
271+
public struct CheckedOutDependencyMissing: DiagnosticData, Swift.Error {
272+
public static var id = DiagnosticID(
273+
type: CheckedOutDependencyMissing.self,
274+
name: "org.swift.diags.workspace.checked-out-dependency-missing",
275+
defaultBehavior: .warning,
276+
description: {
277+
$0 <<< "The dependency"
278+
$0 <<< { "'\($0.packageName)'" }
279+
$0 <<< "is missing and has been cloned again."
280+
})
281+
282+
/// The package name of the dependency.
283+
public let packageName: String
284+
}
285+
286+
/// The diagnostic triggered when an edited dependency is missing
287+
/// from the file-system.
288+
public struct EditedDependencyMissing: DiagnosticData, Swift.Error {
289+
public static var id = DiagnosticID(
290+
type: EditedDependencyMissing.self,
291+
name: "org.swift.diags.workspace.edited-dependency-missing",
292+
defaultBehavior: .warning,
293+
description: {
294+
$0 <<< "The dependency"
295+
$0 <<< { "'\($0.packageName)'" }
296+
$0 <<< "was being edited but is missing. Falling back to original checkout."
297+
})
298+
299+
/// The package name of the dependency.
300+
public let packageName: String
301+
}
302+
303+
/// The diagnostic triggered when a dependency is edited from a revision
304+
/// but the dependency already exists at the target location.
305+
public struct EditRevisionNotUsed: DiagnosticData, Swift.Error {
306+
public static var id = DiagnosticID(
307+
type: EditRevisionNotUsed.self,
308+
name: "org.swift.diags.workspace.edit-revision-not-used",
309+
defaultBehavior: .warning,
310+
description: {
311+
$0 <<< "The dependency"
312+
$0 <<< { "'\($0.packageName)'" }
313+
$0 <<< "already exists at the edit destination. Not using revision"
314+
$0 <<< { "'\($0.revisionIdentifier)'" }
315+
$0 <<< "."
316+
})
317+
318+
/// The package name of the dependency.
319+
public let packageName: String
320+
321+
/// The edit revision.
322+
public let revisionIdentifier: String
323+
}
324+
325+
/// The diagnostic triggered when a dependency is edited with a branch
326+
/// but the dependency already exists at the target location.
327+
public struct EditBranchNotCheckedOut: DiagnosticData, Swift.Error {
328+
public static var id = DiagnosticID(
329+
type: EditBranchNotCheckedOut.self,
330+
name: "org.swift.diags.workspace.edit-branch-not-used",
331+
defaultBehavior: .warning,
332+
description: {
333+
$0 <<< "The dependency"
334+
$0 <<< { "'\($0.packageName)'" }
335+
$0 <<< "already exists at the edit destination. Not checking-out branch"
336+
$0 <<< { "'\($0.branchName)'" }
337+
$0 <<< "."
338+
})
339+
340+
/// The package name of the dependency.
341+
public let packageName: String
342+
343+
/// The branch name
344+
public let branchName: String
345+
}
264346
}

‎Sources/Workspace/Workspace.swift

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ public protocol WorkspaceDelegate: class {
5050
/// The workspace is removing this repository because it is no longer needed.
5151
func removing(repository: String)
5252

53-
/// The workspace operation emitted this warning.
54-
func warning(message: String)
55-
5653
/// Called when the managed dependencies are updated.
5754
func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>)
5855
}
@@ -612,10 +609,14 @@ extension Workspace {
612609

613610
// Emit warnings for branch and revision, if they're present.
614611
if let checkoutBranch = checkoutBranch {
615-
delegate.warning(message: "not checking out branch '\(checkoutBranch)' for dependency '\(packageName)'")
612+
diagnostics.emit(WorkspaceDiagnostics.EditBranchNotCheckedOut(
613+
packageName: packageName,
614+
branchName: checkoutBranch))
616615
}
617616
if let revision = revision {
618-
delegate.warning(message: "not using revsion '\(revision.identifier)' for dependency '\(packageName)'")
617+
diagnostics.emit(WorkspaceDiagnostics.EditRevisionNotUsed(
618+
packageName: packageName,
619+
revisionIdentifier: revision.identifier))
619620
}
620621
} else {
621622
// Otherwise, create a checkout at the destination from our repository store.
@@ -780,10 +781,8 @@ extension Workspace {
780781
) -> DependencyManifests {
781782

782783
// Try to load current managed dependencies, or emit and return.
783-
do {
784-
try fixManagedDependencies()
785-
} catch {
786-
diagnostics.emit(error)
784+
fixManagedDependencies(with: diagnostics)
785+
guard !diagnostics.hasErrors else {
787786
return DependencyManifests(root: root, dependencies: [])
788787
}
789788

@@ -1169,22 +1168,21 @@ extension Workspace {
11691168
/// If some checkout dependency is reomved form the file system, clone it again.
11701169
/// If some edited dependency is removed from the file system, mark it as unedited and
11711170
/// fallback on the original checkout.
1172-
fileprivate func fixManagedDependencies() throws {
1171+
fileprivate func fixManagedDependencies(with diagnostics: DiagnosticsEngine) {
11731172
for dependency in managedDependencies.values {
1174-
let dependencyPath = path(for: dependency)
1175-
if !fileSystem.isDirectory(dependencyPath) {
1176-
switch dependency.state {
1177-
case .checkout(let checkoutState):
1178-
// If some checkout dependency has been removed, clone it again.
1179-
_ = try clone(repository: dependency.repository, at: checkoutState)
1180-
// FIXME: Use diagnostics engine when we have that.
1181-
delegate.warning(message: "\(dependency.subpath.asString) is missing and has been cloned again.")
1182-
case .edited:
1183-
// If some edited dependency has been removed, mark it as unedited.
1184-
try unedit(dependency: dependency, forceRemove: true)
1185-
// FIXME: Use diagnostics engine when we have that.
1186-
delegate.warning(message: "\(dependency.subpath.asString) was being edited but has been removed, " +
1187-
"falling back to original checkout.")
1173+
diagnostics.wrap {
1174+
let dependencyPath = path(for: dependency)
1175+
if !fileSystem.isDirectory(dependencyPath) {
1176+
switch dependency.state {
1177+
case .checkout(let checkoutState):
1178+
// If some checkout dependency has been removed, clone it again.
1179+
_ = try clone(repository: dependency.repository, at: checkoutState)
1180+
diagnostics.emit(WorkspaceDiagnostics.CheckedOutDependencyMissing(packageName: dependency.name))
1181+
case .edited:
1182+
// If some edited dependency has been removed, mark it as unedited.
1183+
try unedit(dependency: dependency, forceRemove: true)
1184+
diagnostics.emit(WorkspaceDiagnostics.EditedDependencyMissing(packageName: dependency.name))
1185+
}
11881186
}
11891187
}
11901188
}

‎Tests/CommandsTests/PackageToolTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ final class PackageToolTests: XCTestCase {
207207
try localFileSystem.writeFileContents(editsPath.appending(components: "Sources", "bar.swift"), bytes: "public let theValue = 88888\n")
208208
let buildOutput = try build()
209209

210-
XCTAssert(buildOutput.contains("baz was being edited but has been removed, falling back to original checkout."))
210+
XCTAssert(buildOutput.contains("The dependency 'baz' was being edited but is missing. Falling back to original checkout."))
211211
// We should be able to see that modification now.
212212
XCTAssertEqual(try Process.checkNonZeroExit(arguments: exec), "88888\n")
213213
// The branch of edited package should be the one we provided when putting it in edit mode.

‎Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ private class TestWorkspaceDelegate: WorkspaceDelegate {
3737
/// Map of checkedout repos with key as repository and value as the reference (version or revision).
3838
var checkedOut = [String: String]()
3939
var removed = [String]()
40-
var warnings = [String]()
4140
var managedDependenciesData = [AnySequence<ManagedDependency>]()
4241

4342
typealias PartialGraphData = (currentGraph: PackageGraph, dependencies: AnySequence<ManagedDependency>, missingURLs: Set<String>)
@@ -66,10 +65,6 @@ private class TestWorkspaceDelegate: WorkspaceDelegate {
6665
removed.append(repository)
6766
}
6867

69-
func warning(message: String) {
70-
warnings.append(message)
71-
}
72-
7368
func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>) {
7469
managedDependenciesData.append(dependencies)
7570
}
@@ -1027,7 +1022,7 @@ final class WorkspaceTests: XCTestCase {
10271022

10281023
// Pinning non existant version should fail.
10291024
pin(at: "1.0.2", diagnostics: diagnostics)
1030-
XCTAssertTrue(diagnostics.diagnostics[0].localizedDescription.contains("A @ 1.0.2"))
1025+
XCTAssertTrue(diagnostics.diagnostics.contains(where: { $0.localizedDescription.contains("A @ 1.0.2") }))
10311026

10321027
// Pinning an unstatisfiable version should fail.
10331028
diagnostics = DiagnosticsEngine()
@@ -1385,9 +1380,12 @@ final class WorkspaceTests: XCTestCase {
13851380

13861381
// Remove edited checkout.
13871382
try removeFileTree(workspace.editablesPath)
1388-
delegate.warnings.removeAll()
13891383
workspace.loadPackageGraph(rootPackages: [path], diagnostics: diagnostics)
1390-
XCTAssertTrue(delegate.warnings[0].hasSuffix("A was being edited but has been removed, falling back to original checkout."))
1384+
XCTAssertFalse(diagnostics.hasErrors)
1385+
XCTAssertTrue(diagnostics.diagnostics.contains(where: {
1386+
$0.behavior == .warning &&
1387+
$0.localizedDescription == "The dependency 'A' was being edited but is missing. Falling back to original checkout."
1388+
}))
13911389
}
13921390
}
13931391

@@ -1712,7 +1710,10 @@ final class WorkspaceTests: XCTestCase {
17121710

17131711
workspace.loadPackageGraph(rootPackages: [barRoot], diagnostics: diagnostics)
17141712
XCTAssertFalse(diagnostics.hasErrors)
1715-
XCTAssertTrue(delegate.warnings.contains(where: { $0.hasPrefix("Foo") && $0.hasSuffix(" is missing and has been cloned again.") }))
1713+
XCTAssertTrue(diagnostics.diagnostics.contains(where: {
1714+
$0.behavior == .warning &&
1715+
$0.localizedDescription == "The dependency 'Foo' is missing and has been cloned again."
1716+
}))
17161717
XCTAssertTrue(isDirectory(workspace.checkoutsPath))
17171718
}
17181719
}

0 commit comments

Comments
 (0)