Skip to content

Commit 710e3cf

Browse files
committed
Add idempotentancy to AddTargetPlugin
1 parent 0c74056 commit 710e3cf

File tree

6 files changed

+281
-77
lines changed

6 files changed

+281
-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: 80 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
///
@@ -54,19 +54,33 @@ public struct AddTargetPlugin {
5454
}
5555

5656
guard let stringLiteral = nameArgument.expression.as(StringLiteralExprSyntax.self),
57-
let literalValue = stringLiteral.representedLiteralValue else {
57+
let literalValue = stringLiteral.representedLiteralValue
58+
else {
5859
return false
5960
}
6061

6162
return literalValue == targetName
6263
}
6364

64-
guard let targetCall = FunctionCallExprSyntax.findFirst(in: targetArray, matching: matchesTargetCall) else {
65+
guard let targetCall = FunctionCallExprSyntax.findFirst(
66+
in: targetArray,
67+
matching: matchesTargetCall
68+
) else {
6569
throw ManifestEditError.cannotFindTarget(targetName: targetName)
6670
}
6771

72+
guard try !self.pluginAlreadyAdded(
73+
plugin,
74+
to: targetName,
75+
in: targetCall
76+
)
77+
else {
78+
return PackageEditResult(manifestEdits: [])
79+
}
80+
6881
let newTargetCall = try addTargetPluginLocal(
69-
plugin, to: targetCall
82+
plugin,
83+
to: targetCall
7084
)
7185

7286
return PackageEditResult(
@@ -76,16 +90,76 @@ public struct AddTargetPlugin {
7690
)
7791
}
7892

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

158+
extension TargetDescription.PluginUsage {
159+
fileprivate var identifier: String {
160+
switch self {
161+
case .plugin(let name, _):
162+
name
163+
}
164+
}
165+
}

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)