Skip to content

Commit aea38ca

Browse files
committed
Make add-dependency idempotent
If a manifest already contains the exact dependency is being added via a `add-dependency` call, don't add it twice. Instead, do nothing. This ensures we don't end up with an invalid manifest with two of the same dependencies in it. If a manifest contains a dependency with the supplied URL but a different version qualifier then throw an error explaining that the supplied dependency has already been added. Issue: #8519
1 parent 95ce2a3 commit aea38ca

File tree

3 files changed

+284
-184
lines changed

3 files changed

+284
-184
lines changed

Sources/PackageModelSyntax/AddPackageDependency.swift

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ public enum AddPackageDependency {
4444
throw ManifestEditError.cannotFindPackage
4545
}
4646

47+
guard try checkExistingDependency(
48+
dependency,
49+
in: packageCall
50+
) else {
51+
return PackageEditResult(manifestEdits: [])
52+
}
53+
4754
let newPackageCall = try addPackageDependencyLocal(
4855
dependency, to: packageCall
4956
)
@@ -55,6 +62,52 @@ public enum AddPackageDependency {
5562
)
5663
}
5764

65+
/// Check that the given package dependency doesn't already exist in the manifest.
66+
/// If the exact same dependency already exists, `false` is returned to indicate
67+
/// that no edits are needed. If a different dependency with the same id or url
68+
/// with different arguments exists, an error is thrown.
69+
private static func checkExistingDependency(
70+
_ dependency: MappablePackageDependency.Kind,
71+
in packageCall: FunctionCallExprSyntax
72+
) throws -> Bool {
73+
let dependencySyntax = dependency.asSyntax()
74+
guard let dependenctFnSyntax = dependencySyntax.as(FunctionCallExprSyntax.self) else {
75+
throw ManifestEditError.cannotFindPackage // TODO: Fix error
76+
}
77+
78+
guard let id = dependenctFnSyntax.arguments.first(where: {
79+
$0.label?.text == "url" || $0.label?.text == "id" || $0.label?.text == "path"
80+
}) else {
81+
throw InternalError("Missing id or url argument in dependency syntax")
82+
}
83+
84+
if let existingDependencies = packageCall.findArgument(labeled: "dependencies") {
85+
// If we have an existing dependencies array, we need to check if
86+
if let expr = existingDependencies.expression.as(ArrayExprSyntax.self) {
87+
// Iterate through existing dependencies and look for an argument that matches
88+
// either the `id` or `url` argument of the new dependency.
89+
let existingArgument = expr.elements.first { elem in
90+
if let funcExpr = elem.expression.as(FunctionCallExprSyntax.self) {
91+
return funcExpr.arguments.contains {
92+
$0.trimmedDescription == id.trimmedDescription
93+
}
94+
}
95+
return false
96+
}
97+
98+
if let existingArgument {
99+
let normalizedExistingArgument = existingArgument.detached.with(\.trailingComma, nil)
100+
// This exact dependency already exists, return false to indicate we should do nothing.
101+
if normalizedExistingArgument.trimmedDescription == dependencySyntax.trimmedDescription {
102+
return false
103+
}
104+
throw ManifestEditError.existingDependency(dependencyName: dependency.identifier)
105+
}
106+
}
107+
}
108+
return true
109+
}
110+
58111
/// Implementation of adding a package dependency to an existing call.
59112
static func addPackageDependencyLocal(
60113
_ dependency: MappablePackageDependency.Kind,
@@ -67,3 +120,16 @@ public enum AddPackageDependency {
67120
)
68121
}
69122
}
123+
124+
fileprivate extension MappablePackageDependency.Kind {
125+
var identifier: String {
126+
switch self {
127+
case .sourceControl(let name, let path, _):
128+
return name ?? path
129+
case .fileSystem(let name, let location):
130+
return name ?? location
131+
case .registry(let id, _):
132+
return id
133+
}
134+
}
135+
}

Sources/PackageModelSyntax/ManifestEditError.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ package enum ManifestEditError: Error {
2222
case cannotFindTarget(targetName: String)
2323
case cannotFindArrayLiteralArgument(argumentName: String, node: Syntax)
2424
case oldManifest(ToolsVersion)
25+
case existingDependency(dependencyName: String)
2526
}
2627

2728
extension ToolsVersion {
@@ -43,6 +44,8 @@ extension ManifestEditError: CustomStringConvertible {
4344
"unable to find array literal for '\(name)' argument"
4445
case .oldManifest(let version):
4546
"package manifest version \(version) is too old: please update to manifest version \(ToolsVersion.minimumManifestEditVersion) or newer"
47+
case .existingDependency(let name):
48+
"unable to add dependency '\(name)' because it already exists in the list of dependencies"
4649
}
4750
}
4851
}

0 commit comments

Comments
 (0)