Add idempotentancy to AddTargetPlugin

Author: hi2gage

Sources/Commands/PackageCommands/AddTargetPlugin.swift

@@ -20,6 +20,8 @@ import SwiftSyntax
 import TSCBasic
 import TSCUtility
 import Workspace
+import PackageGraph
+import Foundation
 
 extension SwiftPackageCommand {
     struct AddTargetPlugin: SwiftCommand {

Sources/PackageModelSyntax/AddPackageDependency.swift

@@ -130,4 +130,4 @@ fileprivate extension MappablePackageDependency.Kind {
                 return id
         }
     }
-}
+}

Sources/PackageModelSyntax/AddTargetPlugin.swift

@@ -18,7 +18,7 @@ import SwiftSyntax
 import SwiftSyntaxBuilder
 
 /// Add a target plugin to a manifest's source code.
-public struct AddTargetPlugin {
+public enum AddTargetPlugin {
     /// The set of argument labels that can occur after the "plugins"
     /// argument in the various target initializers.
     ///
@@ -41,8 +41,10 @@ public struct AddTargetPlugin {
         }
 
         // Dig out the array of targets.
-        guard let targetsArgument = packageCall.findArgument(labeled: "targets"),
-              let targetArray = targetsArgument.expression.findArrayArgument() else {
+        guard
+            let targetsArgument = packageCall.findArgument(labeled: "targets"),
+            let targetArray = targetsArgument.expression.findArrayArgument()
+        else {
             throw ManifestEditError.cannotFindTargets
         }
 
@@ -53,20 +55,40 @@ public struct AddTargetPlugin {
                 return false
             }
 
-            guard let stringLiteral = nameArgument.expression.as(StringLiteralExprSyntax.self),
-                let literalValue = stringLiteral.representedLiteralValue else {
+            guard
+                let stringLiteral = nameArgument.expression.as(
+                    StringLiteralExprSyntax.self
+                ),
+                let literalValue = stringLiteral.representedLiteralValue
+            else {
                 return false
             }
 
             return literalValue == targetName
         }
 
-        guard let targetCall = FunctionCallExprSyntax.findFirst(in: targetArray, matching: matchesTargetCall) else {
+        guard
+            let targetCall = FunctionCallExprSyntax.findFirst(
+                in: targetArray,
+                matching: matchesTargetCall
+            )
+        else {
             throw ManifestEditError.cannotFindTarget(targetName: targetName)
         }
 
+        guard
+            try !self.pluginAlreadyAdded(
+                plugin,
+                to: targetName,
+                in: targetCall
+            )
+        else {
+            return PackageEditResult(manifestEdits: [])
+        }
+
         let newTargetCall = try addTargetPluginLocal(
-            plugin, to: targetCall
+            plugin,
+            to: targetCall
         )
 
         return PackageEditResult(
@@ -76,16 +98,78 @@ public struct AddTargetPlugin {
         )
     }
 
+    private static func pluginAlreadyAdded(
+        _ plugin: TargetDescription.PluginUsage,
+        to targetName: String,
+        in packageCall: FunctionCallExprSyntax
+    ) throws -> Bool {
+        let pluginSyntax = plugin.asSyntax()
+        guard let pluginFnSyntax = pluginSyntax.as(FunctionCallExprSyntax.self)
+        else {
+            throw ManifestEditError.cannotFindPackage
+        }
+
+        guard
+            let id = pluginFnSyntax.arguments.first(where: {
+                $0.label?.text == "name"
+            })
+        else {
+            throw InternalError("Missing 'name' argument in plugin syntax")
+        }
+
+        if let existingPlugins = packageCall.findArgument(labeled: "plugins") {
+            // If we have an existing plugins array, we need to check if
+            if let expr = existingPlugins.expression.as(ArrayExprSyntax.self) {
+                // Iterate through existing dependencies and look for an argument that matches
+                // either the `id` or `url` argument of the new dependency.
+                let existingArgument = expr.elements.first { elem in
+                    if let funcExpr = elem.expression.as(
+                        FunctionCallExprSyntax.self
+                    ) {
+                        return funcExpr.arguments.contains {
+                            $0.with(\.trailingComma, nil).trimmedDescription
+                                == id.with(\.trailingComma, nil)
+                                .trimmedDescription
+                        }
+                    }
+                    return true
+                }
+
+                if let existingArgument {
+                    let normalizedExistingArgument = existingArgument.detached.with(\.trailingComma, nil)
+                    // This exact dependency already exists, return false to indicate we should do nothing.
+                    if normalizedExistingArgument.trimmedDescription == pluginSyntax.trimmedDescription {
+                        return true
+                    }
+                    throw ManifestEditError.existingPlugin(
+                        pluginName: plugin.identifier,
+                        taget: targetName
+                    )
+                }
+            }
+        }
+
+        return false
+    }
+
     /// Implementation of adding a target dependency to an existing call.
     static func addTargetPluginLocal(
         _ plugin: TargetDescription.PluginUsage,
         to targetCall: FunctionCallExprSyntax
     ) throws -> FunctionCallExprSyntax {
         try targetCall.appendingToArrayArgument(
             label: "plugins",
-            trailingLabels: Self.argumentLabelsAfterDependencies,
+            trailingLabels: self.argumentLabelsAfterDependencies,
             newElement: plugin.asSyntax()
         )
     }
 }
 
+extension TargetDescription.PluginUsage {
+    fileprivate var identifier: String {
+        switch self {
+        case .plugin(let name, _):
+            name
+        }
+    }
+}

Sources/PackageModelSyntax/ManifestEditError.swift

@@ -24,6 +24,7 @@ package enum ManifestEditError: Error {
     case oldManifest(ToolsVersion, expected: ToolsVersion)
     case cannotAddSettingsToPluginTarget
     case existingDependency(dependencyName: String)
+    case existingPlugin(pluginName: String, taget: String)
 }
 
 extension ToolsVersion {
@@ -49,6 +50,8 @@ extension ManifestEditError: CustomStringConvertible {
             "plugin targets do not support settings"
         case .existingDependency(let name):
             "unable to add dependency '\(name)' because it already exists in the list of dependencies"
+        case .existingPlugin(let name, let taget):
+            "unable to add plugin '\(name)' to taget '\(taget)' because it already exists in the list of plugins"
         }
     }
 }

Sources/PackageModelSyntax/PluginUsage+Syntax.swift

@@ -12,6 +12,7 @@
 
 import PackageModel
 import SwiftSyntax
+import SwiftSyntaxBuilder
 
 extension TargetDescription.PluginUsage: ManifestSyntaxRepresentable {
     func asSyntax() -> ExprSyntax {