Merge pull request #1148 from hartbit/workspace-warnings-to-diagnostics

Migrate Workspace delegate warnings to diagnostics

Author: aciidgh

Sources/Workspace/Diagnostics.swift

@@ -109,6 +109,8 @@ public enum ResolverDiagnostics {
 
 public enum WorkspaceDiagnostics {
 
+    //MARK: - Errors
+
     /// The diagnostic triggered when an operation fails because its completion
     /// would loose the uncommited changes in a repository.
     public struct UncommitedChanges: DiagnosticData, Swift.Error {
@@ -261,4 +263,84 @@ public enum WorkspaceDiagnostics {
         /// The package found at the edit location.
         public let destinationPackage: String?
     }
+
+    //MARK: - Warnings
+
+    /// The diagnostic triggered when a checked-out dependency is missing
+    /// from the file-system.
+    public struct CheckedOutDependencyMissing: DiagnosticData, Swift.Error {
+        public static var id = DiagnosticID(
+            type: CheckedOutDependencyMissing.self,
+            name: "org.swift.diags.workspace.checked-out-dependency-missing",
+            defaultBehavior: .warning,
+            description: {
+                $0 <<< "The dependency"
+                $0 <<< { "'\($0.packageName)'" }
+                $0 <<< "is missing and has been cloned again."
+            })
+
+        /// The package name of the dependency.
+        public let packageName: String
+    }
+
+    /// The diagnostic triggered when an edited dependency is missing
+    /// from the file-system.
+    public struct EditedDependencyMissing: DiagnosticData, Swift.Error {
+        public static var id = DiagnosticID(
+            type: EditedDependencyMissing.self,
+            name: "org.swift.diags.workspace.edited-dependency-missing",
+            defaultBehavior: .warning,
+            description: {
+                $0 <<< "The dependency"
+                $0 <<< { "'\($0.packageName)'" }
+                $0 <<< "was being edited but is missing. Falling back to original checkout."
+            })
+        
+        /// The package name of the dependency.
+        public let packageName: String
+    }
+
+    /// The diagnostic triggered when a dependency is edited from a revision
+    /// but the dependency already exists at the target location.
+    public struct EditRevisionNotUsed: DiagnosticData, Swift.Error {
+        public static var id = DiagnosticID(
+            type: EditRevisionNotUsed.self,
+            name: "org.swift.diags.workspace.edit-revision-not-used",
+            defaultBehavior: .warning,
+            description: {
+                $0 <<< "The dependency"
+                $0 <<< { "'\($0.packageName)'" }
+                $0 <<< "already exists at the edit destination. Not using revision"
+                $0 <<< { "'\($0.revisionIdentifier)'" }
+                $0 <<< "."
+            })
+        
+        /// The package name of the dependency.
+        public let packageName: String
+
+        /// The edit revision.
+        public let revisionIdentifier: String
+    }
+
+    /// The diagnostic triggered when a dependency is edited with a branch
+    /// but the dependency already exists at the target location.
+    public struct EditBranchNotCheckedOut: DiagnosticData, Swift.Error {
+        public static var id = DiagnosticID(
+            type: EditBranchNotCheckedOut.self,
+            name: "org.swift.diags.workspace.edit-branch-not-used",
+            defaultBehavior: .warning,
+            description: {
+                $0 <<< "The dependency"
+                $0 <<< { "'\($0.packageName)'" }
+                $0 <<< "already exists at the edit destination. Not checking-out branch"
+                $0 <<< { "'\($0.branchName)'" }
+                $0 <<< "."
+            })
+        
+        /// The package name of the dependency.
+        public let packageName: String
+
+        /// The branch name
+        public let branchName: String
+    }
 }

Sources/Workspace/Workspace.swift

@@ -50,9 +50,6 @@ public protocol WorkspaceDelegate: class {
     /// The workspace is removing this repository because it is no longer needed.
     func removing(repository: String)
 
-    /// The workspace operation emitted this warning.
-    func warning(message: String)
-
     /// Called when the managed dependencies are updated.
     func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>)
 }
@@ -612,10 +609,14 @@ extension Workspace {
 
             // Emit warnings for branch and revision, if they're present.
             if let checkoutBranch = checkoutBranch {
-                delegate.warning(message: "not checking out branch '\(checkoutBranch)' for dependency '\(packageName)'")
+                diagnostics.emit(WorkspaceDiagnostics.EditBranchNotCheckedOut(
+                    packageName: packageName,
+                    branchName: checkoutBranch))
             }
             if let revision = revision {
-                delegate.warning(message: "not using revsion '\(revision.identifier)' for dependency '\(packageName)'")
+                diagnostics.emit(WorkspaceDiagnostics.EditRevisionNotUsed(
+                    packageName: packageName,
+                    revisionIdentifier: revision.identifier))
             }
         } else {
             // Otherwise, create a checkout at the destination from our repository store.
@@ -780,10 +781,8 @@ extension Workspace {
     ) -> DependencyManifests {
 
         // Try to load current managed dependencies, or emit and return.
-        do {
-            try fixManagedDependencies()
-        } catch {
-            diagnostics.emit(error)
+        fixManagedDependencies(with: diagnostics)
+        guard !diagnostics.hasErrors else {
             return DependencyManifests(root: root, dependencies: [])
         }
 
@@ -1169,22 +1168,21 @@ extension Workspace {
     /// If some checkout dependency is reomved form the file system, clone it again.
     /// If some edited dependency is removed from the file system, mark it as unedited and
     /// fallback on the original checkout.
-    fileprivate func fixManagedDependencies() throws {
+    fileprivate func fixManagedDependencies(with diagnostics: DiagnosticsEngine) {
         for dependency in managedDependencies.values {
-            let dependencyPath = path(for: dependency)
-            if !fileSystem.isDirectory(dependencyPath) {
-                switch dependency.state {
-                case .checkout(let checkoutState):
-                    // If some checkout dependency has been removed, clone it again.
-                    _ = try clone(repository: dependency.repository, at: checkoutState)
-                    // FIXME: Use diagnostics engine when we have that.
-                    delegate.warning(message: "\(dependency.subpath.asString) is missing and has been cloned again.")
-                case .edited:
-                    // If some edited dependency has been removed, mark it as unedited.
-                    try unedit(dependency: dependency, forceRemove: true)
-                    // FIXME: Use diagnostics engine when we have that.
-                    delegate.warning(message: "\(dependency.subpath.asString) was being edited but has been removed, " +
-                        "falling back to original checkout.")
+            diagnostics.wrap {
+                let dependencyPath = path(for: dependency)
+                if !fileSystem.isDirectory(dependencyPath) {
+                    switch dependency.state {
+                    case .checkout(let checkoutState):
+                        // If some checkout dependency has been removed, clone it again.
+                        _ = try clone(repository: dependency.repository, at: checkoutState)
+                        diagnostics.emit(WorkspaceDiagnostics.CheckedOutDependencyMissing(packageName: dependency.name))
+                    case .edited:
+                        // If some edited dependency has been removed, mark it as unedited.
+                        try unedit(dependency: dependency, forceRemove: true)
+                        diagnostics.emit(WorkspaceDiagnostics.EditedDependencyMissing(packageName: dependency.name))
+                    }
                 }
             }
         }

Tests/CommandsTests/PackageToolTests.swift

@@ -207,7 +207,7 @@ final class PackageToolTests: XCTestCase {
             try localFileSystem.writeFileContents(editsPath.appending(components: "Sources", "bar.swift"), bytes: "public let theValue = 88888\n")
             let buildOutput = try build()
 
-            XCTAssert(buildOutput.contains("baz was being edited but has been removed, falling back to original checkout."))
+            XCTAssert(buildOutput.contains("The dependency 'baz' was being edited but is missing. Falling back to original checkout."))
             // We should be able to see that modification now.
             XCTAssertEqual(try Process.checkNonZeroExit(arguments: exec), "88888\n")
             // The branch of edited package should be the one we provided when putting it in edit mode.

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -37,7 +37,6 @@ private class TestWorkspaceDelegate: WorkspaceDelegate {
     /// Map of checkedout repos with key as repository and value as the reference (version or revision).
     var checkedOut = [String: String]()
     var removed = [String]()
-    var warnings = [String]()
     var managedDependenciesData = [AnySequence<ManagedDependency>]()
 
     typealias PartialGraphData = (currentGraph: PackageGraph, dependencies: AnySequence<ManagedDependency>, missingURLs: Set<String>)
@@ -66,10 +65,6 @@ private class TestWorkspaceDelegate: WorkspaceDelegate {
         removed.append(repository)
     }
 
-    func warning(message: String) {
-        warnings.append(message)
-    }
-
     func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>) {
         managedDependenciesData.append(dependencies)
     }
@@ -1027,7 +1022,7 @@ final class WorkspaceTests: XCTestCase {
 
             // Pinning non existant version should fail.
             pin(at: "1.0.2", diagnostics: diagnostics)
-            XCTAssertTrue(diagnostics.diagnostics[0].localizedDescription.contains("A @ 1.0.2"))
+            XCTAssertTrue(diagnostics.diagnostics.contains(where: { $0.localizedDescription.contains("A @ 1.0.2") }))
 
             // Pinning an unstatisfiable version should fail.
             diagnostics = DiagnosticsEngine()
@@ -1385,9 +1380,12 @@ final class WorkspaceTests: XCTestCase {
 
             // Remove edited checkout.
             try removeFileTree(workspace.editablesPath)
-            delegate.warnings.removeAll()
             workspace.loadPackageGraph(rootPackages: [path], diagnostics: diagnostics)
-            XCTAssertTrue(delegate.warnings[0].hasSuffix("A was being edited but has been removed, falling back to original checkout."))
+            XCTAssertFalse(diagnostics.hasErrors)
+            XCTAssertTrue(diagnostics.diagnostics.contains(where: {
+                $0.behavior == .warning &&
+                $0.localizedDescription == "The dependency 'A' was being edited but is missing. Falling back to original checkout."
+            }))
         }
     }
 
@@ -1712,7 +1710,10 @@ final class WorkspaceTests: XCTestCase {
 
             workspace.loadPackageGraph(rootPackages: [barRoot], diagnostics: diagnostics)
             XCTAssertFalse(diagnostics.hasErrors)
-            XCTAssertTrue(delegate.warnings.contains(where: { $0.hasPrefix("Foo") && $0.hasSuffix(" is missing and has been cloned again.") }))
+            XCTAssertTrue(diagnostics.diagnostics.contains(where: {
+                $0.behavior == .warning &&
+                $0.localizedDescription == "The dependency 'Foo' is missing and has been cloned again."
+            }))
             XCTAssertTrue(isDirectory(workspace.checkoutsPath))
         }
     }