Skip to content

Commit 621ae13

Browse files
committed
[SwiftRefactor] PackageManifest: Add duplicate dependency checking to AddPackageDependency
This was introduced by swiftlang/swift-package-manager#8534
1 parent 58a01d3 commit 621ae13

File tree

3 files changed

+98
-0
lines changed

3 files changed

+98
-0
lines changed

Sources/SwiftRefactor/PackageManifest/AddPackageDependency.swift

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ public struct AddPackageDependency: ManifestEditRefactoringProvider {
4747
throw ManifestEditError.cannotFindPackage
4848
}
4949

50+
guard
51+
try !dependencyAlreadyAdded(
52+
dependency,
53+
in: packageCall
54+
)
55+
else {
56+
return PackageEdit(manifestEdits: [])
57+
}
58+
5059
let newPackageCall = try addPackageDependencyLocal(
5160
dependency,
5261
to: packageCall
@@ -59,6 +68,52 @@ public struct AddPackageDependency: ManifestEditRefactoringProvider {
5968
)
6069
}
6170

71+
/// Return `true` if the dependency already exists in the manifest, otherwise return `false`.
72+
/// Throws an error if a dependency already exists with the same id or url, but different arguments.
73+
private static func dependencyAlreadyAdded(
74+
_ dependency: PackageDependency,
75+
in packageCall: FunctionCallExprSyntax
76+
) throws -> Bool {
77+
let dependencySyntax = dependency.asSyntax()
78+
guard let dependencyFnSyntax = dependencySyntax.as(FunctionCallExprSyntax.self) else {
79+
throw ManifestEditError.cannotFindPackage
80+
}
81+
82+
guard
83+
let id = dependencyFnSyntax.arguments.first(where: {
84+
$0.label?.text == "url" || $0.label?.text == "id" || $0.label?.text == "path"
85+
})
86+
else {
87+
throw ManifestEditError.malformedManifest(error: "missing id or url argument in dependency syntax")
88+
}
89+
90+
if let existingDependencies = packageCall.findArgument(labeled: "dependencies") {
91+
// If we have an existing dependencies array, we need to check if
92+
if let expr = existingDependencies.expression.as(ArrayExprSyntax.self) {
93+
// Iterate through existing dependencies and look for an argument that matches
94+
// either the `id` or `url` argument of the new dependency.
95+
let existingArgument = expr.elements.first { elem in
96+
if let funcExpr = elem.expression.as(FunctionCallExprSyntax.self) {
97+
return funcExpr.arguments.contains {
98+
$0.trimmedDescription == id.trimmedDescription
99+
}
100+
}
101+
return true
102+
}
103+
104+
if let existingArgument {
105+
let normalizedExistingArgument = existingArgument.detached.with(\.trailingComma, nil)
106+
// This exact dependency already exists, return false to indicate we should do nothing.
107+
if normalizedExistingArgument.trimmedDescription == dependencySyntax.trimmedDescription {
108+
return true
109+
}
110+
throw ManifestEditError.existingDependency(dependencyName: dependency.identifier)
111+
}
112+
}
113+
}
114+
return false
115+
}
116+
62117
/// Implementation of adding a package dependency to an existing call.
63118
static func addPackageDependencyLocal(
64119
_ dependency: PackageDependency,
@@ -71,3 +126,16 @@ public struct AddPackageDependency: ManifestEditRefactoringProvider {
71126
)
72127
}
73128
}
129+
130+
fileprivate extension PackageDependency {
131+
var identifier: String {
132+
switch self {
133+
case .sourceControl(let info):
134+
return info.identity
135+
case .fileSystem(let info):
136+
return info.identity
137+
case .registry(let info):
138+
return info.identity
139+
}
140+
}
141+
}

Sources/SwiftRefactor/PackageManifest/ManifestEditError.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ public enum ManifestEditError: Error, Equatable {
1919
case cannotFindTargets
2020
case cannotFindTarget(targetName: String)
2121
case cannotFindArrayLiteralArgument(argumentName: String)
22+
case existingDependency(dependencyName: String)
23+
case malformedManifest(error: String)
2224
}
2325

2426
extension ManifestEditError: CustomStringConvertible {
@@ -32,6 +34,10 @@ extension ManifestEditError: CustomStringConvertible {
3234
return "unable to find target named '\(name)' in package"
3335
case .cannotFindArrayLiteralArgument(argumentName: let name):
3436
return "unable to find array literal for '\(name)' argument"
37+
case .existingDependency(let name):
38+
return "unable to add dependency '\(name)' because it already exists in the list of dependencies"
39+
case .malformedManifest(let error):
40+
return "invalid manifest: \(error)"
3541
}
3642
}
3743
}

Tests/SwiftRefactorTest/ManifestEditTests.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,30 @@ final class ManifestEditTests: XCTestCase {
163163
)
164164
}
165165

166+
func testAddPackageDependencyDuplicates() throws {
167+
XCTAssertThrowsError(
168+
try AddPackageDependency.manifestRefactor(
169+
syntax: """
170+
// swift-tools-version: 5.5
171+
let package = Package(
172+
name: "packages",
173+
dependencies: [
174+
.package(url: "https://github.com/apple/swift-system.git", from: "510.0.1")
175+
]
176+
)
177+
""",
178+
in: .init(dependency: Self.swiftSystemPackageDependency)
179+
)
180+
) { (error: any Error) in
181+
guard let error = error as? ManifestEditError,
182+
case .existingDependency("swift-system") = error
183+
else {
184+
XCTFail("unexpected error thrown: \(error)")
185+
return
186+
}
187+
}
188+
}
189+
166190
func testAddPackageDependencyExistingAppended() throws {
167191
try assertManifestRefactor(
168192
"""

0 commit comments

Comments
 (0)