Fix discrepancies with usage of package display name/package identity in swift package edit command (#7941)

Fixes #7931

`swift package edit` was reliant on the user-passed `packageName`
argument to be able to both find the dependency (which needs the
identity) as well as to match the manifest display name.

Default to treating the user-passed argument as a package identity, and
fix the comparisons made to the manifest by instead assuring that the
canonical locations for the manifest and the dependency match. The
display name was intended to be deprecated.

### Motivation:

The vscode-swift extension made use of of the package identities when
listing dependencies in its UI. When a user would execute a `Use Local
Version` on one of these dependencies, it would call `swift package edit
<package-name>` under the hood, using the identity for the package-name
argument. However, this would fail as the `edit` command would also
treat the package name argument as the package's manifest `displayName`,
which is not guaranteed to match the identity.

### Modifications:

Treat the user-passed argument as the package identity, and deprecate
the usage of `displayName` in the `edit` command. Fix any necessary test
cases to follow suite with this change.

### Result:

The`edit` command should now be using the package identity, and there
should no longer be any discrepancies.

Author: bripeticca

Sources/Commands/PackageCommands/EditCommands.swift

@@ -32,16 +32,16 @@ extension SwiftPackageCommand {
         @Option(help: "Create or use the checkout at this path")
         var path: AbsolutePath?
 
-        @Argument(help: "The name of the package to edit")
-        var packageName: String
+        @Argument(help: "The identity of the package to edit")
+        var packageIdentity: String
 
         func run(_ swiftCommandState: SwiftCommandState) async throws {
             try await swiftCommandState.resolve()
             let workspace = try swiftCommandState.getActiveWorkspace()
 
             // Put the dependency in edit mode.
             await workspace.edit(
-                packageName: packageName,
+                packageIdentity: packageIdentity,
                 path: path,
                 revision: revision,
                 checkoutBranch: checkoutBranch,
@@ -61,15 +61,15 @@ extension SwiftPackageCommand {
               help: "Unedit the package even if it has uncommitted and unpushed changes")
         var shouldForceRemove: Bool = false
 
-        @Argument(help: "The name of the package to unedit")
-        var packageName: String
+        @Argument(help: "The identity of the package to unedit")
+        var packageIdentity: String
 
         func run(_ swiftCommandState: SwiftCommandState) async throws {
             try await swiftCommandState.resolve()
             let workspace = try swiftCommandState.getActiveWorkspace()
 
             try await workspace.unedit(
-                packageName: packageName,
+                packageIdentity: packageIdentity,
                 forceRemove: shouldForceRemove,
                 root: swiftCommandState.getWorkspaceRoot(),
                 observabilityScope: swiftCommandState.observabilityScope

Sources/Workspace/Workspace+Editing.swift

@@ -21,15 +21,15 @@ import struct SourceControl.Revision
 extension Workspace {
     /// Edit implementation.
     func _edit(
-        packageName: String,
+        packageIdentity: String,
         path: AbsolutePath? = nil,
         revision: Revision? = nil,
         checkoutBranch: String? = nil,
         observabilityScope: ObservabilityScope
     ) async throws {
         // Look up the dependency and check if we can edit it.
-        guard let dependency = self.state.dependencies[.plain(packageName)] else {
-            observabilityScope.emit(.dependencyNotFound(packageName: packageName))
+        guard let dependency = self.state.dependencies[.plain(packageIdentity)] else {
+            observabilityScope.emit(.dependencyNotFound(packageName: packageIdentity))
             return
         }
 
@@ -58,10 +58,10 @@ extension Workspace {
 
         // If a path is provided then we use it as destination. If not, we
         // use the folder with packageName inside editablesPath.
-        let destination = path ?? self.location.editsDirectory.appending(component: packageName)
+        let destination = path ?? self.location.editsDirectory.appending(component: packageIdentity)
 
         // If there is something present at the destination, we confirm it has
-        // a valid manifest with name same as the package we are trying to edit.
+        // a valid manifest with name canonical location as the package we are trying to edit.
         if fileSystem.exists(destination) {
             // FIXME: this should not block
             let manifest = try await withCheckedThrowingContinuation { continuation in
@@ -77,23 +77,23 @@ extension Workspace {
                 )
             }
 
-            guard manifest.displayName == packageName else {
+            guard dependency.packageRef.canonicalLocation == manifest.canonicalPackageLocation else {
                 return observabilityScope
                     .emit(
-                        error: "package at '\(destination)' is \(manifest.displayName) but was expecting \(packageName)"
+                        error: "package at '\(destination)' is \(dependency.packageRef.identity) but was expecting \(packageIdentity)"
                     )
             }
 
             // Emit warnings for branch and revision, if they're present.
             if let checkoutBranch {
                 observabilityScope.emit(.editBranchNotCheckedOut(
-                    packageName: packageName,
+                    packageName: packageIdentity,
                     branchName: checkoutBranch
                 ))
             }
             if let revision {
                 observabilityScope.emit(.editRevisionNotUsed(
-                    packageName: packageName,
+                    packageName: packageIdentity,
                     revisionIdentifier: revision.identifier
                 ))
             }
@@ -135,7 +135,7 @@ extension Workspace {
             try fileSystem.createDirectory(self.location.editsDirectory)
             // FIXME: We need this to work with InMem file system too.
             if !(fileSystem is InMemoryFileSystem) {
-                let symLinkPath = self.location.editsDirectory.appending(component: packageName)
+                let symLinkPath = self.location.editsDirectory.appending(component: packageIdentity)
 
                 // Cleanup any existing symlink.
                 if fileSystem.isSymlink(symLinkPath) {
@@ -158,7 +158,7 @@ extension Workspace {
 
         // Save the new state.
         try self.state.dependencies.add(
-            dependency.edited(subpath: RelativePath(validating: packageName), unmanagedPath: path)
+            dependency.edited(subpath: RelativePath(validating: packageIdentity), unmanagedPath: path)
         )
         try self.state.save()
     }

Sources/Workspace/Workspace.swift

@@ -597,23 +597,23 @@ extension Workspace {
     /// Puts a dependency in edit mode creating a checkout in editables directory.
     ///
     /// - Parameters:
-    ///     - packageName: The name of the package to edit.
+    ///     - packageIdentity: The identity of the package to edit.
     ///     - path: If provided, creates or uses the checkout at this location.
     ///     - revision: If provided, the revision at which the dependency
     ///       should be checked out to otherwise current revision.
     ///     - checkoutBranch: If provided, a new branch with this name will be
     ///       created from the revision provided.
     ///     - observabilityScope: The observability scope that reports errors, warnings, etc
     public func edit(
-        packageName: String,
+        packageIdentity: String,
         path: AbsolutePath? = nil,
         revision: Revision? = nil,
         checkoutBranch: String? = nil,
         observabilityScope: ObservabilityScope
     ) async {
         do {
             try await self._edit(
-                packageName: packageName,
+                packageIdentity: packageIdentity,
                 path: path,
                 revision: revision,
                 checkoutBranch: checkoutBranch,
@@ -636,13 +636,13 @@ extension Workspace {
     ///     - root: The workspace root. This is used to resolve the dependencies post unediting.
     ///     - observabilityScope: The observability scope that reports errors, warnings, etc
     public func unedit(
-        packageName: String,
+        packageIdentity: String,
         forceRemove: Bool,
         root: PackageGraphRootInput,
         observabilityScope: ObservabilityScope
     ) async throws {
-        guard let dependency = self.state.dependencies[.plain(packageName)] else {
-            observabilityScope.emit(.dependencyNotFound(packageName: packageName))
+        guard let dependency = self.state.dependencies[.plain(packageIdentity)] else {
+            observabilityScope.emit(.dependencyNotFound(packageName: packageIdentity))
             return
         }
 

Sources/_InternalTestSupport/MockWorkspace.swift

@@ -372,7 +372,7 @@ public final class MockWorkspace {
     }
 
     public func checkEdit(
-        packageName: String,
+        packageIdentity: String,
         path: AbsolutePath? = nil,
         revision: Revision? = nil,
         checkoutBranch: String? = nil,
@@ -382,7 +382,7 @@ public final class MockWorkspace {
         await observability.topScope.trap {
             let ws = try self.getOrCreateWorkspace()
             await ws.edit(
-                packageName: packageName,
+                packageIdentity: packageIdentity,
                 path: path,
                 revision: revision,
                 checkoutBranch: checkoutBranch,
@@ -393,7 +393,7 @@ public final class MockWorkspace {
     }
 
     public func checkUnedit(
-        packageName: String,
+        packageIdentity: String,
         roots: [String],
         forceRemove: Bool = false,
         _ result: ([Basics.Diagnostic]) -> Void
@@ -403,7 +403,7 @@ public final class MockWorkspace {
             let rootInput = PackageGraphRootInput(packages: try rootPaths(for: roots))
             let ws = try self.getOrCreateWorkspace()
             try await ws.unedit(
-                packageName: packageName,
+                packageIdentity: packageIdentity,
                 forceRemove: forceRemove,
                 root: rootInput,
                 observabilityScope: observability.topScope

Sources/_InternalTestSupport/PackageGraphTester.swift

@@ -28,20 +28,10 @@ public final class PackageGraphResult {
         self.graph = graph
     }
 
-    // TODO: deprecate / transition to PackageIdentity
-    public func check(roots: String..., file: StaticString = #file, line: UInt = #line) {
-        XCTAssertEqual(graph.rootPackages.map{$0.manifest.displayName }.sorted(), roots.sorted(), file: file, line: line)
-    }
-
     public func check(roots: PackageIdentity..., file: StaticString = #file, line: UInt = #line) {
         XCTAssertEqual(graph.rootPackages.map{$0.identity }.sorted(), roots.sorted(), file: file, line: line)
     }
 
-    // TODO: deprecate / transition to PackageIdentity
-    public func check(packages: String..., file: StaticString = #file, line: UInt = #line) {
-        XCTAssertEqual(graph.packages.map {$0.manifest.displayName }.sorted(), packages.sorted(), file: file, line: line)
-    }
-
     public func check(packages: PackageIdentity..., file: StaticString = #file, line: UInt = #line) {
         XCTAssertEqual(graph.packages.map {$0.identity }.sorted(), packages.sorted(), file: file, line: line)
     }