[6.2][Commands] Migrate: Avoid duplicate fix-its and manifest updates when…

Fixtures/SwiftMigrate/ExistentialAnyWithCommonPluginDependencyMigration/Package.swift

@@ -0,0 +1,13 @@
+// swift-tools-version:5.8
+
+import PackageDescription
+
+let package = Package(
+    name: "ExistentialAnyMigration",
+    targets: [
+        .target(name: "Library", dependencies: ["CommonLibrary"], plugins: [.plugin(name: "Plugin")]),
+        .plugin(name: "Plugin", capability: .buildTool, dependencies: ["Tool"]),
+        .executableTarget(name: "Tool", dependencies: ["CommonLibrary"]),
+        .target(name: "CommonLibrary"),
+    ]
+)

Fixtures/SwiftMigrate/ExistentialAnyWithCommonPluginDependencyMigration/Plugins/Plugin/Plugin.swift

@@ -0,0 +1,17 @@
+import PackagePlugin
+import Foundation
+
+@main struct Plugin: BuildToolPlugin {
+    func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
+        let tool = try context.tool(named: "Tool")
+        let output = context.pluginWorkDirectory.appending(["generated.swift"])
+        return [
+            .buildCommand(
+                displayName: "Plugin",
+                executable: tool.path,
+                arguments: [output],
+                inputFiles: [],
+                outputFiles: [output])
+        ]
+    }
+}

Fixtures/SwiftMigrate/ExistentialAnyWithCommonPluginDependencyMigration/Sources/CommonLibrary/Common.swift

@@ -0,0 +1,6 @@
+public func common() {}
+
+
+protocol P {}
+
+func needsMigration(_ p: P) {}

Fixtures/SwiftMigrate/ExistentialAnyWithCommonPluginDependencyMigration/Sources/Library/Test.swift

@@ -0,0 +1,6 @@
+import CommonLibrary
+
+func bar() {
+    generatedFunction()
+    common()
+}

Fixtures/SwiftMigrate/ExistentialAnyWithCommonPluginDependencyMigration/Sources/Tool/tool.swift

@@ -0,0 +1,13 @@
+import Foundation
+import CommonLibrary
+
+@main struct Entry {
+    public static func main() async throws {
+        common()
+        let outputPath = CommandLine.arguments[1]
+        let contents = """
+        func generatedFunction() {}
+        """
+        FileManager.default.createFile(atPath: outputPath, contents: contents.data(using: .utf8))
+    }
+}

Fixtures/SwiftMigrate/ExistentialAnyWithPluginMigration/Package.swift

@@ -0,0 +1,12 @@
+// swift-tools-version:5.8
+
+import PackageDescription
+
+let package = Package(
+    name: "ExistentialAnyMigration",
+    targets: [
+        .target(name: "Library", plugins: [.plugin(name: "Plugin")]),
+        .plugin(name: "Plugin", capability: .buildTool, dependencies: ["Tool"]),
+        .executableTarget(name: "Tool"),
+    ]
+)

Fixtures/SwiftMigrate/ExistentialAnyWithPluginMigration/Plugins/Plugin/Plugin.swift

@@ -0,0 +1,17 @@
+import PackagePlugin
+import Foundation
+
+@main struct Plugin: BuildToolPlugin {
+    func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
+        let tool = try context.tool(named: "Tool")
+        let output = context.pluginWorkDirectory.appending(["generated.swift"])
+        return [
+            .buildCommand(
+                displayName: "Plugin",
+                executable: tool.path,
+                arguments: [output],
+                inputFiles: [],
+                outputFiles: [output])
+        ]
+    }
+}

Fixtures/SwiftMigrate/ExistentialAnyWithPluginMigration/Sources/Library/Test.swift

@@ -0,0 +1,20 @@
+protocol P {
+}
+
+func test1(_: P) {
+}
+
+func test2(_: P.Protocol) {
+}
+
+func test3() {
+    let _: [P?] = []
+}
+
+func test4() {
+    var x = 42
+}
+
+func bar() {
+    generatedFunction()
+}

Fixtures/SwiftMigrate/ExistentialAnyWithPluginMigration/Sources/Tool/tool.swift

@@ -0,0 +1,12 @@
+import Foundation
+
+@main struct Entry {
+    public static func main() async throws {
+        let outputPath = CommandLine.arguments[1]
+        let contents = """
+        func generatedFunction() {}
+        func dontmodifyme(_: P) {}
+        """
+        FileManager.default.createFile(atPath: outputPath, contents: contents.data(using: .utf8))
+    }
+}

Sources/Commands/PackageCommands/Migrate.swift

@@ -113,11 +113,11 @@ extension SwiftPackageCommand {
             // Determine all of the targets we need up update.
             let buildPlan = try buildSystem.buildPlan
 
-            var modules: [any ModuleBuildDescription] = []
+            var modules = [String: [AbsolutePath]]()
             if !targets.isEmpty {
                 for buildDescription in buildPlan.buildModules
                     where targets.contains(buildDescription.module.name) {
-                    modules.append(buildDescription)
+                    modules[buildDescription.module.name, default: []].append(contentsOf: buildDescription.diagnosticFiles)
                 }
             } else {
                 let graph = try await buildSystem.getPackageGraph()
@@ -128,7 +128,7 @@ extension SwiftPackageCommand {
                     guard module.type != .plugin, !module.implicit else {
                         continue
                     }
-                    modules.append(buildDescription)
+                    modules[buildDescription.module.name, default: []].append(contentsOf: buildDescription.diagnosticFiles)
                 }
             }
 
@@ -139,10 +139,11 @@ extension SwiftPackageCommand {
 
             var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
             let fixItDuration = try ContinuousClock().measure {
-                for module in modules {
+                for (_, diagnosticFiles) in modules {
                     let fixit = try SwiftFixIt(
-                        diagnosticFiles: module.diagnosticFiles,
+                        diagnosticFiles: diagnosticFiles,
                         categories: Set(features.flatMap(\.categories)),
+                        excludedSourceDirectories: [swiftCommandState.scratchDirectory],
                         fileSystem: swiftCommandState.fileSystem
                     )
                     summary += try fixit.applyFixIts()
@@ -176,10 +177,10 @@ extension SwiftPackageCommand {
             // manifest with newly adopted feature settings.
 
             print("> Updating manifest")
-            for module in modules.map(\.module) {
-                swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'")
+            for (name, _) in modules {
+                swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(name)'")
                 try self.updateManifest(
-                    for: module.name,
+                    for: name,
                     add: features,
                     using: swiftCommandState
                 )

Sources/SwiftFixIt/SwiftFixIt.swift

@@ -124,9 +124,11 @@ private struct PrimaryDiagnosticFilter<Diagnostic: AnyDiagnostic>: ~Copyable {
     private var uniquePrimaryDiagnostics: Set<DiagnosticID> = []
 
     let categories: Set<String>
+    let excludedSourceDirectories: Set<AbsolutePath>
 
-    init(categories: Set<String>) {
+    init(categories: Set<String>, excludedSourceDirectories: Set<AbsolutePath>) {
         self.categories = categories
+        self.excludedSourceDirectories = excludedSourceDirectories
     }
 
     /// Returns a Boolean value indicating whether to skip the given primary
@@ -153,6 +155,13 @@ private struct PrimaryDiagnosticFilter<Diagnostic: AnyDiagnostic>: ~Copyable {
             }
         }
 
+        // Skip if the source file the diagnostic appears in is in an excluded directory.
+        if let sourceFilePath = try? diagnostic.location.map({ try AbsolutePath(validating: $0.filename) }) {
+            guard !self.excludedSourceDirectories.contains(where: { $0.isAncestor(of: sourceFilePath) }) else {
+                return true
+            }
+        }
+
         let notes = primaryDiagnosticWithNotes.dropFirst()
         precondition(notes.allSatisfy(\.isNote))
 
@@ -201,6 +210,7 @@ package struct SwiftFixIt /*: ~Copyable */ { // TODO: Crashes with ~Copyable
     package init(
         diagnosticFiles: [AbsolutePath],
         categories: Set<String> = [],
+        excludedSourceDirectories: Set<AbsolutePath> = [],
         fileSystem: any FileSystem
     ) throws {
         // Deserialize the diagnostics.
@@ -212,18 +222,23 @@ package struct SwiftFixIt /*: ~Copyable */ { // TODO: Crashes with ~Copyable
         self = try SwiftFixIt(
             diagnostics: diagnostics,
             categories: categories,
+            excludedSourceDirectories: excludedSourceDirectories,
             fileSystem: fileSystem
         )
     }
 
     init<Diagnostic: AnyDiagnostic>(
         diagnostics: some Collection<Diagnostic>,
         categories: Set<String>,
+        excludedSourceDirectories: Set<AbsolutePath>,
         fileSystem: any FileSystem
     ) throws {
         self.fileSystem = fileSystem
 
-        var filter = PrimaryDiagnosticFilter<Diagnostic>(categories: categories)
+        var filter = PrimaryDiagnosticFilter<Diagnostic>(
+            categories: categories,
+            excludedSourceDirectories: excludedSourceDirectories
+        )
         _ = consume categories
 
         // Build a map from source files to `SwiftDiagnostics` diagnostics.

Tests/CommandsTests/PackageCommandTests.swift

@@ -2158,6 +2158,44 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
         try await doMigration(featureName: "InferIsolatedConformances", expectedSummary: "Applied 3 fix-its in 2 files")
     }
 
+    func testMigrateCommandWithBuildToolPlugins() async throws {
+        try XCTSkipIf(
+            !UserToolchain.default.supportesSupportedFeatures,
+            "skipping because test environment compiler doesn't support `-print-supported-features`"
+        )
+
+        try await fixture(name: "SwiftMigrate/ExistentialAnyWithPluginMigration") { fixturePath in
+            let (stdout, _) = try await self.execute(
+                ["migrate", "--to-feature", "ExistentialAny"],
+                packagePath: fixturePath
+            )
+
+            // Check the plugin target in the manifest wasn't updated
+            let manifestContent = try localFileSystem.readFileContents(fixturePath.appending(component: "Package.swift")).description
+            XCTAssertTrue(manifestContent.contains(".plugin(name: \"Plugin\", capability: .buildTool, dependencies: [\"Tool\"]),"))
+
+            // Building the package produces migration fix-its in both an authored and generated source file. Check we only applied fix-its to the hand-authored one.
+            XCTAssertMatch(stdout, .regex("> \("Applied 3 fix-its in 1 file")" + #" \([0-9]\.[0-9]{1,3}s\)"#))
+        }
+    }
+
+    func testMigrateCommandWhenDependencyBuildsForHostAndTarget() async throws {
+        try XCTSkipIf(
+            !UserToolchain.default.supportesSupportedFeatures,
+            "skipping because test environment compiler doesn't support `-print-supported-features`"
+        )
+
+        try await fixture(name: "SwiftMigrate/ExistentialAnyWithCommonPluginDependencyMigration") { fixturePath in
+            let (stdout, _) = try await self.execute(
+                ["migrate", "--to-feature", "ExistentialAny"],
+                packagePath: fixturePath
+            )
+
+            // Even though the CommonLibrary dependency built for both the host and destination, we should only apply a single fix-it once to its sources.
+            XCTAssertMatch(stdout, .regex("> \("Applied 1 fix-it in 1 file")" + #" \([0-9]\.[0-9]{1,3}s\)"#))
+        }
+    }
+
     func testBuildToolPlugin() async throws {
         try await testBuildToolPlugin(staticStdlib: false)
     }
@@ -4108,6 +4146,14 @@ class PackageCommandSwiftBuildTests: PackageCommandTestCase {
         throw XCTSkip("SWBINTTODO: Build plan is not currently supported")
     }
 
+    override func testMigrateCommandWithBuildToolPlugins() async throws {
+        throw XCTSkip("SWBINTTODO: Build plan is not currently supported")
+    }
+
+    override func testMigrateCommandWhenDependencyBuildsForHostAndTarget() async throws {
+        throw XCTSkip("SWBINTTODO: Build plan is not currently supported")
+    }
+
     override func testCommandPluginTestingCallbacks() async throws {
         throw XCTSkip("SWBINTTODO: Requires PIF generation to adopt new test runner product type")
         try XCTSkipOnWindows(because: "TSCBasic/Path.swift:969: Assertion failed, https://github.com/swiftlang/swift-package-manager/issues/8602")

Tests/SwiftFixItTests/Utilities.swift

@@ -233,6 +233,7 @@ private func _testAPI(
     let swiftFixIt = try SwiftFixIt(
         diagnostics: flatDiagnostics,
         categories: categories,
+        excludedSourceDirectories: [],
         fileSystem: localFileSystem
     )
     let actualSummary = try swiftFixIt.applyFixIts()