[6.2] swift-package-migrate: Call SwiftFixIt once for all diagnostic files

Sources/Commands/PackageCommands/Migrate.swift

@@ -125,6 +125,9 @@ extension SwiftPackageCommand {
                     where graph.isRootPackage(buildDescription.package)
                 {
                     let module = buildDescription.module
+                    // FIXME: Plugin target init does not have a Swift settings
+                    // parameter, so we won't be able to enable the feature.
+                    // Exclude plugins from migration.
                     guard module.type != .plugin, !module.implicit else {
                         continue
                     }
@@ -139,15 +142,13 @@ extension SwiftPackageCommand {
 
             var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
             let fixItDuration = try ContinuousClock().measure {
-                for (_, diagnosticFiles) in modules {
-                    let fixit = try SwiftFixIt(
-                        diagnosticFiles: diagnosticFiles,
-                        categories: Set(features.flatMap(\.categories)),
-                        excludedSourceDirectories: [swiftCommandState.scratchDirectory],
-                        fileSystem: swiftCommandState.fileSystem
-                    )
-                    summary += try fixit.applyFixIts()
-                }
+                let applier = try SwiftFixIt(
+                    diagnosticFiles: modules.values.joined(),
+                    categories: Set(features.flatMap(\.categories)),
+                    excludedSourceDirectories: [swiftCommandState.scratchDirectory],
+                    fileSystem: swiftCommandState.fileSystem
+                )
+                summary = try applier.applyFixIts()
             }
 
             // Report the changes.

Sources/SwiftFixIt/SwiftFixIt.swift

@@ -143,7 +143,7 @@ private struct PrimaryDiagnosticFilter<Diagnostic: AnyDiagnostic>: ~Copyable {
         }
 
         // Skip if no location.
-        if diagnostic.hasNoLocation {
+        guard let location = diagnostic.location else {
             return true
         }
 
@@ -155,9 +155,12 @@ 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 {
+        // Skip if excluded directories were given and the source file the
+        // diagnostic appears in is in any of them.
+        if !self.excludedSourceDirectories.isEmpty {
+            if let sourceFilePath = try? AbsolutePath(validating: location.filename),
+               self.excludedSourceDirectories.contains(where: sourceFilePath.isDescendant(of:))
+            {
                 return true
             }
         }
@@ -208,7 +211,7 @@ package struct SwiftFixIt /*: ~Copyable */ { // TODO: Crashes with ~Copyable
     private let diagnosticsPerFile: DiagnosticsPerFile
 
     package init(
-        diagnosticFiles: [AbsolutePath],
+        diagnosticFiles: some Collection<AbsolutePath>,
         categories: Set<String> = [],
         excludedSourceDirectories: Set<AbsolutePath> = [],
         fileSystem: any FileSystem

Tests/SwiftFixItTests/FilteringTests.swift

@@ -412,6 +412,7 @@ struct FilteringTests {
         }
     }
 
+    @Test
     func testDuplicateInsertionFixIts() throws {
         try testAPI1File { path in
             .init(
@@ -526,4 +527,91 @@ struct FilteringTests {
             )
         }
     }
+
+    @Test
+    func testExcludedSourceDirectories() throws {
+        // Each generated file has a distinct parent directory.
+        try testAPI2Files { path1, path2 in
+            .init(
+                edits: (
+                    .init(input: "var x = 1", result: "var x = 1"),
+                    .init(input: "var x = 1", result: "var y = 2")
+                ),
+                summary: .init(
+                    // 6 because skipped by SwiftIDEUtils.FixItApplier, not SwiftFixIt.
+                    numberOfFixItsApplied: 2,
+                    numberOfFilesChanged: 1
+                ),
+                excludedSourceDirectories: [path1.parentDirectory],
+                diagnostics: [
+                    // path1.
+                    PrimaryDiagnostic(
+                        level: .error,
+                        text: "error1_fixit1",
+                        location: .init(path: path1, line: 1, column: 5),
+                        fixIts: [
+                            // Skipped, excluded directory.
+                            .init(
+                                start: .init(path: path1, line: 1, column: 5),
+                                end: .init(path: path1, line: 1, column: 6),
+                                text: "y"
+                            ),
+                        ]
+                    ),
+                    PrimaryDiagnostic(
+                        level: .error,
+                        text: "error2_fixit2",
+                        location: .init(path: path1, line: 1, column: 9),
+                        notes: [
+                            Note(
+                                text: "error2_note1",
+                                location: .init(path: path1, line: 1, column: 9),
+                                fixIts: [
+                                    // Skipped, excluded directory.
+                                    .init(
+                                        start: .init(path: path1, line: 1, column: 9),
+                                        end: .init(path: path1, line: 1, column: 10),
+                                        text: "2"
+                                    ),
+                                ]
+                            ),
+                        ]
+                    ),
+                    // path2.
+                    PrimaryDiagnostic(
+                        level: .error,
+                        text: "error1_fixit1",
+                        location: .init(path: path2, line: 1, column: 5),
+                        fixIts: [
+                            // Applied.
+                            .init(
+                                start: .init(path: path2, line: 1, column: 5),
+                                end: .init(path: path2, line: 1, column: 6),
+                                text: "y"
+                            ),
+                        ]
+                    ),
+                    PrimaryDiagnostic(
+                        level: .error,
+                        text: "error2_fixit2",
+                        location: .init(path: path2, line: 1, column: 9),
+                        notes: [
+                            Note(
+                                text: "error2_note1",
+                                location: .init(path: path2, line: 1, column: 9),
+                                fixIts: [
+                                    // Applied.
+                                    .init(
+                                        start: .init(path: path2, line: 1, column: 9),
+                                        end: .init(path: path2, line: 1, column: 10),
+                                        text: "2"
+                                    ),
+                                ]
+                            ),
+                        ]
+                    ),
+                ]
+            )
+        }
+    }
 }

Tests/SwiftFixItTests/Utilities.swift

@@ -178,9 +178,10 @@ struct Summary {
 }
 
 struct TestCase<T> {
-    let edits: T
-    let summary: Summary
-    let diagnostics: [PrimaryDiagnostic]
+    var edits: T
+    var summary: Summary
+    var excludedSourceDirectories: Set<AbsolutePath> = []
+    var diagnostics: [PrimaryDiagnostic]
 }
 
 extension Testing.Issue {
@@ -211,6 +212,7 @@ private func _testAPI(
     _ expectedSummary: Summary,
     _ diagnostics: [PrimaryDiagnostic],
     _ categories: Set<String>,
+    _ excludedSourceDirectories: Set<AbsolutePath>,
 ) throws {
     for (path, edit) in sourceFilePathsAndEdits {
         try localFileSystem.writeFileContents(path, string: edit.input)
@@ -233,7 +235,7 @@ private func _testAPI(
     let swiftFixIt = try SwiftFixIt(
         diagnostics: flatDiagnostics,
         categories: categories,
-        excludedSourceDirectories: [],
+        excludedSourceDirectories: excludedSourceDirectories,
         fileSystem: localFileSystem
     )
     let actualSummary = try swiftFixIt.applyFixIts()
@@ -261,44 +263,46 @@ private func _testAPI(
     }
 }
 
-private func uniqueSwiftFileName() -> String {
-    "\(UUID().uuidString).swift"
-}
-
 // Cannot use variadic generics: crashes.
 func testAPI1File(
+    function: StaticString = #function,
     categories: Set<String> = [],
     _ getTestCase: (AbsolutePath) -> TestCase<SourceFileEdit>
 ) throws {
-    try testWithTemporaryDirectory { fixturePath in
-        let sourceFilePath = fixturePath.appending(uniqueSwiftFileName())
+    try testWithTemporaryDirectory(function: function) { fixturePath in
+        let sourceFilePath = fixturePath.appending("file.swift")
 
         let testCase = getTestCase(sourceFilePath)
 
         try _testAPI(
             [(sourceFilePath, testCase.edits)],
             testCase.summary,
             testCase.diagnostics,
-            categories
+            categories,
+            testCase.excludedSourceDirectories,
         )
     }
 }
 
 func testAPI2Files(
+    function: StaticString = #function,
     categories: Set<String> = [],
     _ getTestCase: (AbsolutePath, AbsolutePath) -> TestCase<(SourceFileEdit, SourceFileEdit)>,
 ) throws {
-    try testWithTemporaryDirectory { fixturePath in
-        let sourceFilePath1 = fixturePath.appending(uniqueSwiftFileName())
-        let sourceFilePath2 = fixturePath.appending(uniqueSwiftFileName())
+    try testWithTemporaryDirectory(function: function) { fixturePath in
+        // Create each file in a separate subdirectory so that we can test
+        // directory exclusion.
+        let sourceFilePath1 = fixturePath.appending(components: [UUID().uuidString, "file.swift"])
+        let sourceFilePath2 = fixturePath.appending(components: [UUID().uuidString, "file.swift"])
 
         let testCase = getTestCase(sourceFilePath1, sourceFilePath2)
 
         try _testAPI(
             [(sourceFilePath1, testCase.edits.0), (sourceFilePath2, testCase.edits.1)],
             testCase.summary,
             testCase.diagnostics,
-            categories
+            categories,
+            testCase.excludedSourceDirectories,
         )
     }
 }