Skip to content

swift-package-migrate: Report a fix-it application summary #8684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 25, 2025
Merged

swift-package-migrate: Report a fix-it application summary #8684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55531e6
[test] SwiftFixIt: Avoid duplicate fix-its in test that does not expe…
AnthonyLatsis May 22, 2025
a61fe0e
swift-package-migrate: Report a fix-it application summary
AnthonyLatsis May 16, 2025
b4e0d87
swift-package-migrate: Don't end logging messages with a period
AnthonyLatsis May 20, 2025
0f97b3e
SwiftFixIt: Slightly optimize fix-it application
AnthonyLatsis May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 46 additions & 11 deletions Sources/Commands/PackageCommands/Migrate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ extension SwiftPackageCommand {
// Next, let's build all of the individual targets or the
// whole project to get diagnostic files.

print("> Starting the build.")
print("> Starting the build")
if let targets = self.options.targets {
for target in targets {
try await buildSystem.build(subset: .target(target))
Expand Down Expand Up @@ -123,22 +123,49 @@ extension SwiftPackageCommand {
// If the build suceeded, let's extract all of the diagnostic
// files from build plan and feed them to the fix-it tool.

print("> Applying fix-its.")
for module in modules {
let fixit = try SwiftFixIt(
diagnosticFiles: module.diagnosticFiles,
categories: Set(features.flatMap(\.categories)),
fileSystem: swiftCommandState.fileSystem
print("> Applying fix-its")

var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
let fixItDuration = try ContinuousClock().measure {
for module in modules {
let fixit = try SwiftFixIt(
diagnosticFiles: module.diagnosticFiles,
categories: Set(features.flatMap(\.categories)),
fileSystem: swiftCommandState.fileSystem
)
summary += try fixit.applyFixIts()
}
}

// Report the changes.
do {
var message = "> Applied \(summary.numberOfFixItsApplied) fix-it"
if summary.numberOfFixItsApplied != 1 {
message += "s"
}
message += " in \(summary.numberOfFilesChanged) file"
if summary.numberOfFilesChanged != 1 {
message += "s"
}
message += " ("
message += fixItDuration.formatted(
.units(
allowed: [.seconds],
width: .narrow,
fractionalPart: .init(lengthLimits: 0 ... 3, roundingRule: .up)
)
)
try fixit.applyFixIts()
message += ")"

print(message)
}

// Once the fix-its were applied, it's time to update the
// manifest with newly adopted feature settings.

print("> Updating manifest.")
print("> Updating manifest")
for module in modules.map(\.module) {
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'.")
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'")
try self.updateManifest(
for: module.name,
add: features,
Expand Down Expand Up @@ -208,7 +235,15 @@ extension SwiftPackageCommand {
verbose: !self.globalOptions.logging.quiet
)
} catch {
swiftCommandState.observabilityScope.emit(error: "Could not update manifest for '\(target)' (\(error)). Please enable '\(try features.map { try $0.swiftSettingDescription }.joined(separator: ", "))' features manually.")
try swiftCommandState.observabilityScope.emit(
error: """
Could not update manifest for '\(target)' (\(error)). \
Please enable '\(
features.map { try $0.swiftSettingDescription }
.joined(separator: ", ")
)' features manually
"""
)
}
}

Expand Down
50 changes: 44 additions & 6 deletions Sources/SwiftFixIt/SwiftFixIt.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import enum SwiftIDEUtils.FixItApplier
import struct SwiftParser.Parser

import struct SwiftSyntax.AbsolutePosition
import struct SwiftSyntax.SourceEdit
import struct SwiftSyntax.SourceFileSyntax
import class SwiftSyntax.SourceLocationConverter
import struct SwiftSyntax.Syntax
Expand Down Expand Up @@ -264,18 +265,55 @@ package struct SwiftFixIt /*: ~Copyable */ { // TODO: Crashes with ~Copyable

self.diagnosticsPerFile = diagnosticsPerFile
}
}

extension SwiftFixIt {
package struct Summary: Equatable {
package var numberOfFixItsApplied: Int
package var numberOfFilesChanged: Int

package init(numberOfFixItsApplied: Int, numberOfFilesChanged: Int) {
self.numberOfFixItsApplied = numberOfFixItsApplied
self.numberOfFilesChanged = numberOfFilesChanged
}

package static func + (lhs: consuming Self, rhs: Self) -> Self {
lhs += rhs
return lhs
}

package static func += (lhs: inout Self, rhs: Self) {
lhs.numberOfFixItsApplied += rhs.numberOfFixItsApplied
lhs.numberOfFilesChanged += rhs.numberOfFilesChanged
}
}

package func applyFixIts() throws -> Summary {
var numberOfFixItsApplied = 0

package func applyFixIts() throws {
// Bulk-apply fix-its to each file and write the results back.
for (sourceFile, diagnostics) in self.diagnosticsPerFile {
let result = SwiftIDEUtils.FixItApplier.applyFixes(
from: diagnostics,
filterByMessages: nil,
to: sourceFile.syntax
)
numberOfFixItsApplied += diagnostics.count

var edits = [SwiftSyntax.SourceEdit]()
edits.reserveCapacity(diagnostics.count)
for diagnostic in diagnostics {
for fixIt in diagnostic.fixIts {
for edit in fixIt.edits {
edits.append(edit)
}
}
}

let result = SwiftIDEUtils.FixItApplier.apply(edits: consume edits, to: sourceFile.syntax)

try self.fileSystem.writeFileContents(sourceFile.path, string: consume result)
}

return Summary(
numberOfFixItsApplied: numberOfFixItsApplied,
numberOfFilesChanged: self.diagnosticsPerFile.keys.count
)
}
}

Expand Down
12 changes: 7 additions & 5 deletions Tests/CommandsTests/PackageCommandTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2112,7 +2112,7 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
"skipping because test environment compiler doesn't support `-print-supported-features`"
)

func doMigration(featureName: String) async throws {
func doMigration(featureName: String, expectedSummary: String) async throws {
try await fixture(name: "SwiftMigrate/\(featureName)Migration") { fixturePath in
let sourcePaths: [AbsolutePath]
let fixedSourcePaths: [AbsolutePath]
Expand All @@ -2133,7 +2133,7 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
}
}

_ = try await self.execute(
let (stdout, _) = try await self.execute(
["migrate", "--to-feature", featureName],
packagePath: fixturePath
)
Expand All @@ -2146,12 +2146,14 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
localFileSystem.readFileContents(fixedSourcePath)
)
}

XCTAssertMatch(stdout, .regex("> \(expectedSummary)" + #" \([0-9]\.[0-9]{1,3}s\)"#))
}
}

try await doMigration(featureName: "ExistentialAny")
try await doMigration(featureName: "StrictMemorySafety")
try await doMigration(featureName: "InferIsolatedConformances")
try await doMigration(featureName: "ExistentialAny", expectedSummary: "Applied 3 fix-its in 1 file")
try await doMigration(featureName: "StrictMemorySafety", expectedSummary: "Applied 1 fix-it in 1 file")
try await doMigration(featureName: "InferIsolatedConformances", expectedSummary: "Applied 1 fix-it in 1 file")
}

func testBuildToolPlugin() async throws {
Expand Down
34 changes: 24 additions & 10 deletions Tests/SwiftFixItTests/BasicTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct BasicTests {
try testAPI1File { _ in
.init(
edits: .init(input: "var x = 1", result: "var x = 1"),
summary: .init(numberOfFixItsApplied: 0, numberOfFilesChanged: 0),
diagnostics: []
)
}
Expand All @@ -29,6 +30,7 @@ struct BasicTests {
try testAPI1File { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "let x = 1"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand All @@ -52,6 +54,7 @@ struct BasicTests {
try testAPI1File { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "let x = 1"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -81,6 +84,7 @@ struct BasicTests {
try testAPI1File { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "let x = 22"),
summary: .init(numberOfFixItsApplied: 2, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -142,6 +146,7 @@ struct BasicTests {
w = fooo(1, 233)
"""
),
summary: .init(numberOfFixItsApplied: 2, numberOfFilesChanged: 1),
diagnostics: [
// Different lines.
PrimaryDiagnostic(
Expand Down Expand Up @@ -205,6 +210,7 @@ struct BasicTests {
try testAPI1File { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "_ = 1"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -235,6 +241,11 @@ struct BasicTests {
try testAPI1File { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "_ = 1"),
summary: .init(
// 2 because skipped by SwiftIDEUtils.FixItApplier, not SwiftFixIt.
numberOfFixItsApplied: 2 /**/,
numberOfFilesChanged: 1
),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -272,9 +283,10 @@ struct BasicTests {
try testAPI2Files { (filename1: String, filename2: String) in
.init(
edits: (
.init(input: "var x = 1", result: "let x = 1"),
.init(input: "var x = 1", result: "let x = 1")
.init(input: "var x = 1", result: "let _ = 1"),
.init(input: "var x = 1", result: "let _ = 1")
),
summary: .init(numberOfFixItsApplied: 4, numberOfFilesChanged: 2),
diagnostics: [
// filename1
PrimaryDiagnostic(
Expand All @@ -292,25 +304,25 @@ struct BasicTests {
PrimaryDiagnostic(
level: .error,
text: "error1",
location: .init(filename: filename1, line: 1, column: 1, offset: 0),
location: .init(filename: filename1, line: 1, column: 5, offset: 0),
fixIts: [
.init(
start: .init(filename: filename1, line: 1, column: 1, offset: 0),
end: .init(filename: filename1, line: 1, column: 4, offset: 0),
text: "let"
start: .init(filename: filename1, line: 1, column: 5, offset: 0),
end: .init(filename: filename1, line: 1, column: 6, offset: 0),
text: "_"
),
]
),
// filename2
PrimaryDiagnostic(
level: .warning,
text: "warning2",
location: .init(filename: filename2, line: 1, column: 1, offset: 0),
location: .init(filename: filename2, line: 1, column: 5, offset: 0),
fixIts: [
.init(
start: .init(filename: filename2, line: 1, column: 1, offset: 0),
end: .init(filename: filename2, line: 1, column: 4, offset: 0),
text: "let"
start: .init(filename: filename2, line: 1, column: 5, offset: 0),
end: .init(filename: filename2, line: 1, column: 6, offset: 0),
text: "_"
),
]
),
Expand Down Expand Up @@ -339,6 +351,7 @@ struct BasicTests {
.init(input: "var x = 1", result: "let x = 1"),
.init(input: "var x = 1", result: "var x = 1")
),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -372,6 +385,7 @@ struct BasicTests {
.init(input: "var x = 1", result: "let x = 1"),
.init(input: "", result: "")
),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down
6 changes: 6 additions & 0 deletions Tests/SwiftFixItTests/CategoryTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct CategoryTests {
try testAPI1File(categories: ["Other", "Test"]) { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "let _ = 1"),
summary: .init(numberOfFixItsApplied: 2, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -57,6 +58,7 @@ struct CategoryTests {
try testAPI1File(categories: ["Other", "Test"]) { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "let _ = 22"),
summary: .init(numberOfFixItsApplied: 3, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -137,6 +139,7 @@ struct CategoryTests {
try testAPI1File(categories: ["Test"]) { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "var x = 22"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -175,6 +178,7 @@ struct CategoryTests {
try testAPI1File(categories: ["Test"]) { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "var x = 22"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -247,6 +251,7 @@ struct CategoryTests {
try testAPI1File(categories: ["Test"]) { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "var x = 22"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down Expand Up @@ -286,6 +291,7 @@ struct CategoryTests {
try testAPI1File(categories: ["Test"]) { (filename: String) in
.init(
edits: .init(input: "var x = 1", result: "var x = 22"),
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
diagnostics: [
PrimaryDiagnostic(
level: .error,
Expand Down
Loading