Remove basedOn property for ManagedDependency

Author: mattt

Sources/SPMTestSupport/MockWorkspace.swift

@@ -459,15 +459,13 @@ public final class MockWorkspace {
 
         public func check(dependency name: String, at state: State, file: StaticString = #file, line: UInt = #line) {
             guard let dependency = managedDependencies[named: name] else {
-                XCTFail("\(name) does not exists", file: file, line: line)
-                return
+                return XCTFail("\(name) does not exists", file: file, line: line)
             }
 
             switch state {
             case .checkout(let checkoutState):
                 guard case .checkout(let dependencyCheckoutState) = dependency.state else {
-                    XCTFail("Expected checked out dependency; found '\(dependency.state)' instead", file: file, line: line)
-                    return
+                    return XCTFail("Expected checked out dependency; found '\(dependency.state)' instead", file: file, line: line)
                 }
 
                 switch checkoutState {
@@ -479,12 +477,12 @@ public final class MockWorkspace {
                     XCTAssertEqual(dependencyCheckoutState.branch, branch, file: file, line: line)
                 }
             case .edited(let editedState):
-                if dependency.state != .edited(editedState) {
-                    XCTFail("Expected edited dependency; found '\(dependency.state)' instead", file: file, line: line)
+                guard case .edited(_, editedState) = dependency.state else {
+                    return XCTFail("Expected edited dependency; found '\(dependency.state)' instead", file: file, line: line)
                 }
             case .local:
-                if dependency.state != .local {
-                    XCTFail("Expected local dependency", file: file, line: line)
+                guard case .local = dependency.state else {
+                    return XCTFail("Expected local dependency", file: file, line: line)
                 }
             }
         }

Sources/Workspace/ManagedDependency.swift

@@ -18,9 +18,9 @@ import TSCUtility
 ///
 /// Each dependency will have a checkout containing the sources at a
 /// particular revision, and may have an associated version.
-public class ManagedDependency {
+public struct ManagedDependency: Equatable {
     /// Represents the state of the managed dependency.
-    public enum State: Equatable {
+    public indirect enum State: Equatable {
         // The dependency is a local package.
         case local
 
@@ -32,7 +32,7 @@ public class ManagedDependency {
         /// If the path is non-nil, the dependency is managed by a user and is
         /// located at the path. In other words, this dependency is being used
         /// for top of the tree style development.
-        case edited(EditedState)
+        case edited(basedOn: ManagedDependency, EditedState)
     }
 
     /// The package reference.
@@ -57,43 +57,25 @@ public class ManagedDependency {
         }
     }
 
-    /// A dependency which in editable state is based on a dependency from
-    /// which it edited from.
-    ///
-    /// This information is useful so it can be restored when users
-    /// unedit a package.
-    public internal(set) var basedOn: ManagedDependency?
-
     /// Create a dependency present locally on the filesystem.
-    public static func local(
-        package: PackageReference
-    ) -> ManagedDependency {
-        return ManagedDependency(
-            package: package,
-            state: .local,
-            basedOn: nil
-        )
+    public static func local(package: PackageReference) -> ManagedDependency {
+        return ManagedDependency(package: package, state: .local)
     }
 
     /// Create an editable managed dependency based on a dependency which
     /// was *not* in edit state.
     public static func checkout(package: PackageReference, state: CheckoutState) -> ManagedDependency {
-        return ManagedDependency(package: package, state: .checkout(state), basedOn: nil)
+        return ManagedDependency(package: package, state: .checkout(state))
     }
 
     /// Create an editable managed dependency based on a dependency which
     /// was *not* in edit state.
     public static func edited(basedOn dependency: ManagedDependency, state: EditedState) -> ManagedDependency {
-        return ManagedDependency(package: dependency.package, state: .edited(state), basedOn: dependency)
+        return ManagedDependency(package: dependency.package, state: .edited(basedOn: dependency, state))
     }
 
-    internal init(
-        package: PackageReference,
-        state: State,
-        basedOn: ManagedDependency?
-    ) {
+    init(package: PackageReference, state: State) {
         self.package = package
-        self.basedOn = basedOn
         self.state = state
     }
 }

Sources/Workspace/Workspace.swift

@@ -1114,7 +1114,7 @@ extension Workspace {
         root: PackageGraphRootInput? = nil,
         diagnostics: DiagnosticsEngine
     ) throws {
-        guard case .edited(let editedState) = dependency.state else {
+        guard case let .edited(basedOn, editedState) = dependency.state else {
             throw WorkspaceDiagnostics.DependencyNotInEditMode(dependencyName: dependency.package.name)
         }
 
@@ -1144,7 +1144,8 @@ extension Workspace {
             try fileSystem.removeFileTree(self.location.editsDirectory)
         }
 
-        if case .checkout(let checkoutState) = dependency.basedOn?.state {
+
+        if case .checkout(let checkoutState) = basedOn.state {
             // Restore the original checkout.
             //
             // The clone method will automatically update the managed dependency state.
@@ -1469,9 +1470,9 @@ extension Workspace {
         switch dependency.state {
         case .checkout:
             return self.location.repositoryCheckoutDirectory(for: dependency.package)
-        case .edited(.managed):
-            return self.location.editDirectory(for: dependency.package)
-        case .edited(.unmanaged(let path)):
+        case .edited(basedOn: let basedOn, .managed):
+            return self.location.editDirectory(for: basedOn.package)
+        case .edited(_, .unmanaged(let path)):
             return path
         case .local:
             return AbsolutePath(dependency.package.location)
@@ -2457,13 +2458,14 @@ extension Workspace {
                 // original checkout in somewhat of a dangling state when computing
                 // the state changes this method. We basically need to ensure that
                 // the edited checkout is unchanged.
-                if let editedDependency = state.dependencies.first(where: {
-                    guard $0.basedOn != nil else { return false }
-                    return self.path(to: $0).pathString == packageRef.location
+                if let editedDependency = state.dependencies.first(where: { dependency in
+                    guard case .edited = dependency.state else { return false }
+                    return self.path(to: dependency).pathString == packageRef.location
                 }) {
                     currentDependency = editedDependency
-                    let originalReference = editedDependency.basedOn!.package // forced unwrap safe
-                    packageStateChanges[originalReference.location] = (originalReference, .unchanged)
+                    if case .edited(basedOn: let original, _) = editedDependency.state {
+                        packageStateChanges[original.package.location] = (original.package, .unchanged)
+                    }
                 } else {
                     currentDependency = nil
                 }
@@ -2832,7 +2834,7 @@ extension Workspace {
 
     /// Removes the clone and checkout of the provided specifier.
     fileprivate func remove(package: PackageReference) throws {
-        guard let dependency = state.dependencies[package.identity] else {
+        guard var dependency = state.dependencies[package.identity] else {
             throw InternalError("trying to remove \(package.name) which isn't in workspace")
         }
 
@@ -2855,10 +2857,10 @@ extension Workspace {
         // Compute the dependency which we need to remove.
         let dependencyToRemove: ManagedDependency
 
-        if let basedOn = dependency.basedOn {
+        if case .edited(basedOn: let original, _) = dependency.state {
             // Remove the underlying dependency for edited packages.
-            dependencyToRemove = basedOn
-            dependency.basedOn = nil
+            dependencyToRemove = original
+            dependency = ManagedDependency(package: package, state: original.state)
             state.dependencies.add(dependency)
         } else {
             dependencyToRemove = dependency

Sources/Workspace/WorkspaceState.swift

@@ -164,7 +164,12 @@ fileprivate struct WorkspaceStateStorage {
             init(_ dependency: ManagedDependency) {
                 self.packageRef = .init(dependency.package)
                 self.state = dependency.state
-                self.basedOn = dependency.basedOn.map{ .init($0) }
+
+                if case .edited(basedOn: let original, _) = dependency.state {
+                    self.basedOn = .init(original)
+                } else {
+                    self.basedOn = nil
+                }
             }
 
             private enum CodingKeys: String, CodingKey {
@@ -228,7 +233,9 @@ fileprivate struct WorkspaceStateStorage {
                 case .checkout(let checkoutState):
                     try nestedContainer.encode("checkout", forKey: .name)
                     try nestedContainer.encode(CheckoutInfo(checkoutState), forKey: .checkoutState)
-                case .edited(let editedState):
+                case .edited(basedOn: let dependency, let editedState):
+                    try container.encode(Dependency(dependency), forKey: .basedOn)
+
                     try nestedContainer.encode("edited", forKey: .name)
                     switch editedState {
                     case .unmanaged(let path):
@@ -335,12 +342,8 @@ fileprivate struct WorkspaceStateStorage {
 }
 
 extension ManagedDependency {
-    fileprivate convenience init(_ dependency: WorkspaceStateStorage.V4.Dependency) {
-        self.init(
-            package: .init(dependency.packageRef),
-            state: dependency.state,
-            basedOn: dependency.basedOn.map { .init($0) }
-        )
+    fileprivate init(_ dependency: WorkspaceStateStorage.V4.Dependency) {
+        self.init(package: .init(dependency.packageRef), state: dependency.state)
     }
 }
 

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -2277,8 +2277,9 @@ final class WorkspaceTests: XCTestCase {
         }
 
         // There should still be an entry for `foo`, which we can unedit.
-        let editedDependency = ws.state.dependencies[named: "foo"]!
-        XCTAssertNil(editedDependency.basedOn)
+        let editedDependency = ws.state.dependencies[named: "foo"]
+        XCTAssertNotNil(editedDependency)
+
         workspace.checkManagedDependencies { result in
             result.check(dependency: "foo", at: .edited(.managed))
         }