Skip to content

Commit 28ca16e

Browse files
committed
Add idempotentancy to AddTargetPlugin
1 parent 0c74056 commit 28ca16e

File tree

6 files changed

+285
-77
lines changed

6 files changed

+285
-77
lines changed

Sources/Commands/PackageCommands/AddTargetPlugin.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import SwiftSyntax
2020
import TSCBasic
2121
import TSCUtility
2222
import Workspace
23+
import PackageGraph
24+
import Foundation
2325

2426
extension SwiftPackageCommand {
2527
struct AddTargetPlugin: SwiftCommand {

Sources/PackageModelSyntax/AddPackageDependency.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,4 @@ fileprivate extension MappablePackageDependency.Kind {
130130
return id
131131
}
132132
}
133-
}
133+
}

Sources/PackageModelSyntax/AddTargetPlugin.swift

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import SwiftSyntax
1818
import SwiftSyntaxBuilder
1919

2020
/// Add a target plugin to a manifest's source code.
21-
public struct AddTargetPlugin {
21+
public enum AddTargetPlugin {
2222
/// The set of argument labels that can occur after the "plugins"
2323
/// argument in the various target initializers.
2424
///
@@ -53,20 +53,37 @@ public struct AddTargetPlugin {
5353
return false
5454
}
5555

56-
guard let stringLiteral = nameArgument.expression.as(StringLiteralExprSyntax.self),
57-
let literalValue = stringLiteral.representedLiteralValue else {
56+
guard
57+
let stringLiteral = nameArgument.expression.as(
58+
StringLiteralExprSyntax.self
59+
),
60+
let literalValue = stringLiteral.representedLiteralValue
61+
else {
5862
return false
5963
}
6064

6165
return literalValue == targetName
6266
}
6367

64-
guard let targetCall = FunctionCallExprSyntax.findFirst(in: targetArray, matching: matchesTargetCall) else {
68+
guard let targetCall = FunctionCallExprSyntax.findFirst(
69+
in: targetArray,
70+
matching: matchesTargetCall
71+
) else {
6572
throw ManifestEditError.cannotFindTarget(targetName: targetName)
6673
}
6774

75+
guard try !self.pluginAlreadyAdded(
76+
plugin,
77+
to: targetName,
78+
in: targetCall
79+
)
80+
else {
81+
return PackageEditResult(manifestEdits: [])
82+
}
83+
6884
let newTargetCall = try addTargetPluginLocal(
69-
plugin, to: targetCall
85+
plugin,
86+
to: targetCall
7087
)
7188

7289
return PackageEditResult(
@@ -76,16 +93,77 @@ public struct AddTargetPlugin {
7693
)
7794
}
7895

96+
private static func pluginAlreadyAdded(
97+
_ plugin: TargetDescription.PluginUsage,
98+
to targetName: String,
99+
in packageCall: FunctionCallExprSyntax
100+
) throws -> Bool {
101+
let pluginSyntax = plugin.asSyntax()
102+
guard let pluginFnSyntax = pluginSyntax.as(FunctionCallExprSyntax.self)
103+
else {
104+
throw ManifestEditError.cannotFindPackage
105+
}
106+
107+
guard let id = pluginFnSyntax.arguments.first(where: {
108+
$0.label?.text == "name"
109+
})
110+
else {
111+
throw InternalError("Missing 'name' argument in plugin syntax")
112+
}
113+
114+
if let existingPlugins = packageCall.findArgument(labeled: "plugins") {
115+
// If we have an existing plugins array, we need to check if
116+
if let expr = existingPlugins.expression.as(ArrayExprSyntax.self) {
117+
// Iterate through existing dependencies and look for an argument that matches
118+
// either the `id` or `url` argument of the new dependency.
119+
let existingArgument = expr.elements.first { elem in
120+
if let funcExpr = elem.expression.as(
121+
FunctionCallExprSyntax.self
122+
) {
123+
return funcExpr.arguments.contains {
124+
$0.with(\.trailingComma, nil).trimmedDescription
125+
== id.with(\.trailingComma, nil)
126+
.trimmedDescription
127+
}
128+
}
129+
return true
130+
}
131+
132+
if let existingArgument {
133+
let normalizedExistingArgument = existingArgument.detached.with(\.trailingComma, nil)
134+
// This exact dependency already exists, return false to indicate we should do nothing.
135+
if normalizedExistingArgument.trimmedDescription == pluginSyntax.trimmedDescription {
136+
return true
137+
}
138+
throw ManifestEditError.existingPlugin(
139+
pluginName: plugin.identifier,
140+
taget: targetName
141+
)
142+
}
143+
}
144+
}
145+
146+
return false
147+
}
148+
79149
/// Implementation of adding a target dependency to an existing call.
80150
static func addTargetPluginLocal(
81151
_ plugin: TargetDescription.PluginUsage,
82152
to targetCall: FunctionCallExprSyntax
83153
) throws -> FunctionCallExprSyntax {
84154
try targetCall.appendingToArrayArgument(
85155
label: "plugins",
86-
trailingLabels: Self.argumentLabelsAfterDependencies,
156+
trailingLabels: self.argumentLabelsAfterDependencies,
87157
newElement: plugin.asSyntax()
88158
)
89159
}
90160
}
91161

162+
extension TargetDescription.PluginUsage {
163+
fileprivate var identifier: String {
164+
switch self {
165+
case .plugin(let name, _):
166+
name
167+
}
168+
}
169+
}

Sources/PackageModelSyntax/ManifestEditError.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ package enum ManifestEditError: Error {
2424
case oldManifest(ToolsVersion, expected: ToolsVersion)
2525
case cannotAddSettingsToPluginTarget
2626
case existingDependency(dependencyName: String)
27+
case existingPlugin(pluginName: String, taget: String)
2728
}
2829

2930
extension ToolsVersion {
@@ -49,6 +50,8 @@ extension ManifestEditError: CustomStringConvertible {
4950
"plugin targets do not support settings"
5051
case .existingDependency(let name):
5152
"unable to add dependency '\(name)' because it already exists in the list of dependencies"
53+
case .existingPlugin(let name, let taget):
54+
"unable to add plugin '\(name)' to taget '\(taget)' because it already exists in the list of plugins"
5255
}
5356
}
5457
}

Sources/PackageModelSyntax/PluginUsage+Syntax.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import PackageModel
1414
import SwiftSyntax
15+
import SwiftSyntaxBuilder
1516

1617
extension TargetDescription.PluginUsage: ManifestSyntaxRepresentable {
1718
func asSyntax() -> ExprSyntax {

0 commit comments

Comments
 (0)