Skip to content

Commit dcab689

Browse files
AnthonyLatsisAnthonyLatsis
authored
swift-package-migrate: Report a fix-it application summary (#8684)
[test] SwiftFixIt: Avoid duplicate fix-its in test that does not expect them --- swift-package-migrate: Report a fix-it application summary E.g. `> Applied 3 fix-its in 1 file (0.001s)` --- swift-package-migrate: Don't end logging messages with a period Other commands don't do this. Be consistent. --- SwiftFixIt: Slightly optimize fix-it application `SwiftIDEUtils.FixItApplier.applyFixes` performs some O(n^2) filtering based on `SwiftDiagnostics.FixItMessage` messages, which we don't need. Use the overload that directly takes an array of source edits instead.
1 parent 8156d48 commit dcab689

File tree

7 files changed

+167
-37
lines changed

7 files changed

+167
-37
lines changed

‎Sources/Commands/PackageCommands/Migrate.swift

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ extension SwiftPackageCommand {
9494
// Next, let's build all of the individual targets or the
9595
// whole project to get diagnostic files.
9696

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

126-
print("> Applying fix-its.")
127-
for module in modules {
128-
let fixit = try SwiftFixIt(
129-
diagnosticFiles: module.diagnosticFiles,
130-
categories: Set(features.flatMap(\.categories)),
131-
fileSystem: swiftCommandState.fileSystem
126+
print("> Applying fix-its")
127+
128+
var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
129+
let fixItDuration = try ContinuousClock().measure {
130+
for module in modules {
131+
let fixit = try SwiftFixIt(
132+
diagnosticFiles: module.diagnosticFiles,
133+
categories: Set(features.flatMap(\.categories)),
134+
fileSystem: swiftCommandState.fileSystem
135+
)
136+
summary += try fixit.applyFixIts()
137+
}
138+
}
139+
140+
// Report the changes.
141+
do {
142+
var message = "> Applied \(summary.numberOfFixItsApplied) fix-it"
143+
if summary.numberOfFixItsApplied != 1 {
144+
message += "s"
145+
}
146+
message += " in \(summary.numberOfFilesChanged) file"
147+
if summary.numberOfFilesChanged != 1 {
148+
message += "s"
149+
}
150+
message += " ("
151+
message += fixItDuration.formatted(
152+
.units(
153+
allowed: [.seconds],
154+
width: .narrow,
155+
fractionalPart: .init(lengthLimits: 0 ... 3, roundingRule: .up)
156+
)
132157
)
133-
try fixit.applyFixIts()
158+
message += ")"
159+
160+
print(message)
134161
}
135162

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

139-
print("> Updating manifest.")
166+
print("> Updating manifest")
140167
for module in modules.map(\.module) {
141-
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'.")
168+
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'")
142169
try self.updateManifest(
143170
for: module.name,
144171
add: features,
@@ -208,7 +235,15 @@ extension SwiftPackageCommand {
208235
verbose: !self.globalOptions.logging.quiet
209236
)
210237
} catch {
211-
swiftCommandState.observabilityScope.emit(error: "Could not update manifest for '\(target)' (\(error)). Please enable '\(try features.map { try $0.swiftSettingDescription }.joined(separator: ", "))' features manually.")
238+
try swiftCommandState.observabilityScope.emit(
239+
error: """
240+
Could not update manifest for '\(target)' (\(error)). \
241+
Please enable '\(
242+
features.map { try $0.swiftSettingDescription }
243+
.joined(separator: ", ")
244+
)' features manually
245+
"""
246+
)
212247
}
213248
}
214249

‎Sources/SwiftFixIt/SwiftFixIt.swift

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import enum SwiftIDEUtils.FixItApplier
2929
import struct SwiftParser.Parser
3030

3131
import struct SwiftSyntax.AbsolutePosition
32+
import struct SwiftSyntax.SourceEdit
3233
import struct SwiftSyntax.SourceFileSyntax
3334
import class SwiftSyntax.SourceLocationConverter
3435
import struct SwiftSyntax.Syntax
@@ -264,18 +265,55 @@ package struct SwiftFixIt /*: ~Copyable */ { // TODO: Crashes with ~Copyable
264265

265266
self.diagnosticsPerFile = diagnosticsPerFile
266267
}
268+
}
269+
270+
extension SwiftFixIt {
271+
package struct Summary: Equatable {
272+
package var numberOfFixItsApplied: Int
273+
package var numberOfFilesChanged: Int
274+
275+
package init(numberOfFixItsApplied: Int, numberOfFilesChanged: Int) {
276+
self.numberOfFixItsApplied = numberOfFixItsApplied
277+
self.numberOfFilesChanged = numberOfFilesChanged
278+
}
279+
280+
package static func + (lhs: consuming Self, rhs: Self) -> Self {
281+
lhs += rhs
282+
return lhs
283+
}
284+
285+
package static func += (lhs: inout Self, rhs: Self) {
286+
lhs.numberOfFixItsApplied += rhs.numberOfFixItsApplied
287+
lhs.numberOfFilesChanged += rhs.numberOfFilesChanged
288+
}
289+
}
290+
291+
package func applyFixIts() throws -> Summary {
292+
var numberOfFixItsApplied = 0
267293

268-
package func applyFixIts() throws {
269294
// Bulk-apply fix-its to each file and write the results back.
270295
for (sourceFile, diagnostics) in self.diagnosticsPerFile {
271-
let result = SwiftIDEUtils.FixItApplier.applyFixes(
272-
from: diagnostics,
273-
filterByMessages: nil,
274-
to: sourceFile.syntax
275-
)
296+
numberOfFixItsApplied += diagnostics.count
297+
298+
var edits = [SwiftSyntax.SourceEdit]()
299+
edits.reserveCapacity(diagnostics.count)
300+
for diagnostic in diagnostics {
301+
for fixIt in diagnostic.fixIts {
302+
for edit in fixIt.edits {
303+
edits.append(edit)
304+
}
305+
}
306+
}
307+
308+
let result = SwiftIDEUtils.FixItApplier.apply(edits: consume edits, to: sourceFile.syntax)
276309

277310
try self.fileSystem.writeFileContents(sourceFile.path, string: consume result)
278311
}
312+
313+
return Summary(
314+
numberOfFixItsApplied: numberOfFixItsApplied,
315+
numberOfFilesChanged: self.diagnosticsPerFile.keys.count
316+
)
279317
}
280318
}
281319

‎Tests/CommandsTests/PackageCommandTests.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,7 +2112,7 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
21122112
"skipping because test environment compiler doesn't support `-print-supported-features`"
21132113
)
21142114

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

2136-
_ = try await self.execute(
2136+
let (stdout, _) = try await self.execute(
21372137
["migrate", "--to-feature", featureName],
21382138
packagePath: fixturePath
21392139
)
@@ -2146,12 +2146,14 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
21462146
localFileSystem.readFileContents(fixedSourcePath)
21472147
)
21482148
}
2149+
2150+
XCTAssertMatch(stdout, .regex("> \(expectedSummary)" + #" \([0-9]\.[0-9]{1,3}s\)"#))
21492151
}
21502152
}
21512153

2152-
try await doMigration(featureName: "ExistentialAny")
2153-
try await doMigration(featureName: "StrictMemorySafety")
2154-
try await doMigration(featureName: "InferIsolatedConformances")
2154+
try await doMigration(featureName: "ExistentialAny", expectedSummary: "Applied 3 fix-its in 1 file")
2155+
try await doMigration(featureName: "StrictMemorySafety", expectedSummary: "Applied 1 fix-it in 1 file")
2156+
try await doMigration(featureName: "InferIsolatedConformances", expectedSummary: "Applied 1 fix-it in 1 file")
21552157
}
21562158

21572159
func testBuildToolPlugin() async throws {

‎Tests/SwiftFixItTests/BasicTests.swift

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ struct BasicTests {
1919
try testAPI1File { _ in
2020
.init(
2121
edits: .init(input: "var x = 1", result: "var x = 1"),
22+
summary: .init(numberOfFixItsApplied: 0, numberOfFilesChanged: 0),
2223
diagnostics: []
2324
)
2425
}
@@ -29,6 +30,7 @@ struct BasicTests {
2930
try testAPI1File { (filename: String) in
3031
.init(
3132
edits: .init(input: "var x = 1", result: "let x = 1"),
33+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
3234
diagnostics: [
3335
PrimaryDiagnostic(
3436
level: .error,
@@ -52,6 +54,7 @@ struct BasicTests {
5254
try testAPI1File { (filename: String) in
5355
.init(
5456
edits: .init(input: "var x = 1", result: "let x = 1"),
57+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
5558
diagnostics: [
5659
PrimaryDiagnostic(
5760
level: .error,
@@ -81,6 +84,7 @@ struct BasicTests {
8184
try testAPI1File { (filename: String) in
8285
.init(
8386
edits: .init(input: "var x = 1", result: "let x = 22"),
87+
summary: .init(numberOfFixItsApplied: 2, numberOfFilesChanged: 1),
8488
diagnostics: [
8589
PrimaryDiagnostic(
8690
level: .error,
@@ -142,6 +146,7 @@ struct BasicTests {
142146
w = fooo(1, 233)
143147
"""
144148
),
149+
summary: .init(numberOfFixItsApplied: 2, numberOfFilesChanged: 1),
145150
diagnostics: [
146151
// Different lines.
147152
PrimaryDiagnostic(
@@ -205,6 +210,7 @@ struct BasicTests {
205210
try testAPI1File { (filename: String) in
206211
.init(
207212
edits: .init(input: "var x = 1", result: "_ = 1"),
213+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
208214
diagnostics: [
209215
PrimaryDiagnostic(
210216
level: .error,
@@ -235,6 +241,11 @@ struct BasicTests {
235241
try testAPI1File { (filename: String) in
236242
.init(
237243
edits: .init(input: "var x = 1", result: "_ = 1"),
244+
summary: .init(
245+
// 2 because skipped by SwiftIDEUtils.FixItApplier, not SwiftFixIt.
246+
numberOfFixItsApplied: 2 /**/,
247+
numberOfFilesChanged: 1
248+
),
238249
diagnostics: [
239250
PrimaryDiagnostic(
240251
level: .error,
@@ -272,9 +283,10 @@ struct BasicTests {
272283
try testAPI2Files { (filename1: String, filename2: String) in
273284
.init(
274285
edits: (
275-
.init(input: "var x = 1", result: "let x = 1"),
276-
.init(input: "var x = 1", result: "let x = 1")
286+
.init(input: "var x = 1", result: "let _ = 1"),
287+
.init(input: "var x = 1", result: "let _ = 1")
277288
),
289+
summary: .init(numberOfFixItsApplied: 4, numberOfFilesChanged: 2),
278290
diagnostics: [
279291
// filename1
280292
PrimaryDiagnostic(
@@ -292,25 +304,25 @@ struct BasicTests {
292304
PrimaryDiagnostic(
293305
level: .error,
294306
text: "error1",
295-
location: .init(filename: filename1, line: 1, column: 1, offset: 0),
307+
location: .init(filename: filename1, line: 1, column: 5, offset: 0),
296308
fixIts: [
297309
.init(
298-
start: .init(filename: filename1, line: 1, column: 1, offset: 0),
299-
end: .init(filename: filename1, line: 1, column: 4, offset: 0),
300-
text: "let"
310+
start: .init(filename: filename1, line: 1, column: 5, offset: 0),
311+
end: .init(filename: filename1, line: 1, column: 6, offset: 0),
312+
text: "_"
301313
),
302314
]
303315
),
304316
// filename2
305317
PrimaryDiagnostic(
306318
level: .warning,
307319
text: "warning2",
308-
location: .init(filename: filename2, line: 1, column: 1, offset: 0),
320+
location: .init(filename: filename2, line: 1, column: 5, offset: 0),
309321
fixIts: [
310322
.init(
311-
start: .init(filename: filename2, line: 1, column: 1, offset: 0),
312-
end: .init(filename: filename2, line: 1, column: 4, offset: 0),
313-
text: "let"
323+
start: .init(filename: filename2, line: 1, column: 5, offset: 0),
324+
end: .init(filename: filename2, line: 1, column: 6, offset: 0),
325+
text: "_"
314326
),
315327
]
316328
),
@@ -339,6 +351,7 @@ struct BasicTests {
339351
.init(input: "var x = 1", result: "let x = 1"),
340352
.init(input: "var x = 1", result: "var x = 1")
341353
),
354+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
342355
diagnostics: [
343356
PrimaryDiagnostic(
344357
level: .error,
@@ -372,6 +385,7 @@ struct BasicTests {
372385
.init(input: "var x = 1", result: "let x = 1"),
373386
.init(input: "", result: "")
374387
),
388+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
375389
diagnostics: [
376390
PrimaryDiagnostic(
377391
level: .error,

‎Tests/SwiftFixItTests/CategoryTests.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ struct CategoryTests {
1818
try testAPI1File(categories: ["Other", "Test"]) { (filename: String) in
1919
.init(
2020
edits: .init(input: "var x = 1", result: "let _ = 1"),
21+
summary: .init(numberOfFixItsApplied: 2, numberOfFilesChanged: 1),
2122
diagnostics: [
2223
PrimaryDiagnostic(
2324
level: .error,
@@ -57,6 +58,7 @@ struct CategoryTests {
5758
try testAPI1File(categories: ["Other", "Test"]) { (filename: String) in
5859
.init(
5960
edits: .init(input: "var x = 1", result: "let _ = 22"),
61+
summary: .init(numberOfFixItsApplied: 3, numberOfFilesChanged: 1),
6062
diagnostics: [
6163
PrimaryDiagnostic(
6264
level: .error,
@@ -137,6 +139,7 @@ struct CategoryTests {
137139
try testAPI1File(categories: ["Test"]) { (filename: String) in
138140
.init(
139141
edits: .init(input: "var x = 1", result: "var x = 22"),
142+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
140143
diagnostics: [
141144
PrimaryDiagnostic(
142145
level: .error,
@@ -175,6 +178,7 @@ struct CategoryTests {
175178
try testAPI1File(categories: ["Test"]) { (filename: String) in
176179
.init(
177180
edits: .init(input: "var x = 1", result: "var x = 22"),
181+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
178182
diagnostics: [
179183
PrimaryDiagnostic(
180184
level: .error,
@@ -247,6 +251,7 @@ struct CategoryTests {
247251
try testAPI1File(categories: ["Test"]) { (filename: String) in
248252
.init(
249253
edits: .init(input: "var x = 1", result: "var x = 22"),
254+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
250255
diagnostics: [
251256
PrimaryDiagnostic(
252257
level: .error,
@@ -286,6 +291,7 @@ struct CategoryTests {
286291
try testAPI1File(categories: ["Test"]) { (filename: String) in
287292
.init(
288293
edits: .init(input: "var x = 1", result: "var x = 22"),
294+
summary: .init(numberOfFixItsApplied: 1, numberOfFilesChanged: 1),
289295
diagnostics: [
290296
PrimaryDiagnostic(
291297
level: .error,

0 commit comments

Comments
 (0)