Skip to content

Commit 2da2fa9

Browse files
dongdong867ahoppen
authored andcommitted
Fix type mismatch error in reflowMultilineStringLiterals (swiftlang#979)
1 parent 58807b9 commit 2da2fa9

File tree

6 files changed

+217
-16
lines changed

6 files changed

+217
-16
lines changed

.github/workflows/pull_request.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,9 @@ jobs:
2020
with:
2121
license_header_check_project_name: "Swift.org"
2222
api_breakage_check_allowlist_path: "api-breakages.txt"
23+
perf:
24+
name: Test
25+
uses: ahoppen/github-workflows/.github/workflows/performance_test.yml@performance-test
26+
with:
27+
command: "echo Test: 1"
28+
sensitivity: 0
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import Foundation
2+
3+
func printToStderr(_ value: String) {
4+
fputs(value, stderr)
5+
}
6+
7+
func extractMeasurements(output: String) -> [String: Double] {
8+
var measurements: [String: Double] = [:]
9+
for line in output.split(separator: "\n") {
10+
guard let colonPosition = line.lastIndex(of: ":") else {
11+
printToStderr("Ignoring following measurement line because it doesn't contain a colon: \(line)")
12+
continue
13+
}
14+
let beforeColon = String(line[..<colonPosition]).trimmingCharacters(in: .whitespacesAndNewlines)
15+
let afterColon = String(line[line.index(after: colonPosition)...]).trimmingCharacters(
16+
in: .whitespacesAndNewlines
17+
)
18+
guard let value = Double(afterColon) else {
19+
printToStderr("Ignoring following measurement line because the value can't be parsed as a Double: \(line)")
20+
continue
21+
}
22+
measurements[beforeColon] = value
23+
}
24+
return measurements
25+
}
26+
27+
extension Double {
28+
func round(toDecimalDigits decimalDigits: Int) -> Double {
29+
return (self * pow(10, Double(decimalDigits))).rounded() / pow(10, Double(decimalDigits))
30+
}
31+
}
32+
33+
func run(
34+
baselinePerformanceOutput: String,
35+
changedPerformanceOutput: String,
36+
sensitivityPercentage: Double
37+
) -> (output: String, hasDetectedSignificantChange: Bool) {
38+
let baselineMeasurements = extractMeasurements(output: baselinePerformanceOutput)
39+
let changedMeasurements = extractMeasurements(output: changedPerformanceOutput)
40+
41+
var hasDetectedSignificantChange = false
42+
var output = ""
43+
for (measurementName, baselineValue) in baselineMeasurements.sorted(by: { $0.key < $1.key }) {
44+
guard let changedValue = changedMeasurements[measurementName] else {
45+
output += "🛑 \(measurementName) not present after changes\n"
46+
continue
47+
}
48+
let differencePercentage = (changedValue - baselineValue) / baselineValue * 100
49+
let rawMeasurementsText = "(baseline: \(baselineValue), after changes: \(changedValue))"
50+
if differencePercentage < -sensitivityPercentage {
51+
output +=
52+
"🎉 \(measurementName) improved by \(-differencePercentage.round(toDecimalDigits: 3))% \(rawMeasurementsText)\n"
53+
hasDetectedSignificantChange = true
54+
} else if differencePercentage > sensitivityPercentage {
55+
output +=
56+
"⚠️ \(measurementName) regressed by \(differencePercentage.round(toDecimalDigits: 3))% \(rawMeasurementsText)\n"
57+
hasDetectedSignificantChange = true
58+
} else {
59+
output +=
60+
"➡️ \(measurementName) did not change significantly with \(differencePercentage.round(toDecimalDigits: 3))% \(rawMeasurementsText)\n"
61+
}
62+
}
63+
return (output, hasDetectedSignificantChange)
64+
}
65+
66+
guard CommandLine.arguments.count > 2 else {
67+
print("Expected at least two parameters: The baseline performance output and the changed performance output")
68+
exit(1)
69+
}
70+
71+
let baselinePerformanceOutput = CommandLine.arguments[1]
72+
let changedPerformanceOutput = CommandLine.arguments[2]
73+
74+
let sensitivityPercentage =
75+
if CommandLine.arguments.count > 3, let percentage = Double(CommandLine.arguments[3]) {
76+
percentage
77+
} else {
78+
1.0 /* percent */
79+
}
80+
81+
let (output, hasDetectedSignificantChange) = run(
82+
baselinePerformanceOutput: baselinePerformanceOutput,
83+
changedPerformanceOutput: changedPerformanceOutput,
84+
sensitivityPercentage: sensitivityPercentage
85+
)
86+
87+
print(output)
88+
if hasDetectedSignificantChange {
89+
exit(1)
90+
}

Documentation/Configuration.md

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,30 @@ switch someValue {
204204
---
205205

206206
### `reflowMultilineStringLiterals`
207-
**type:** `string`
207+
208+
> [!NOTE]
209+
> This setting should be specified as a string value (e.g. `"never"`)
210+
> For backward compatibility with swift-format version 601.0.0, the configuration also accepts the legacy object format where the setting is specified as an object with a single key (e.g., ⁠`{ "never": {} }`).
211+
212+
**type:** `string` or `object` (legacy)
213+
214+
**example:**
215+
216+
For all versions above 601.0.0, the configuration should be specified as a string, for example:
217+
```json
218+
{
219+
"reflowMultilineStringLiterals": "never"
220+
}
221+
```
222+
223+
For version 601.0.0, the configuration should be specified as an object, for example:
224+
```json
225+
{
226+
"reflowMultilineStringLiterals": {
227+
"never": {}
228+
}
229+
}
230+
```
208231

209232
**description:** Determines how multiline string literals should reflow when formatted.
210233

Sources/SwiftFormat/API/Configuration.swift

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public struct Configuration: Codable, Equatable {
197197
public var multiElementCollectionTrailingCommas: Bool
198198

199199
/// Determines how multiline string literals should reflow when formatted.
200-
public enum MultilineStringReflowBehavior: Codable {
200+
public enum MultilineStringReflowBehavior: String, Codable {
201201
/// Never reflow multiline string literals.
202202
case never
203203
/// Reflow lines in string literal that exceed the maximum line length. For example with a line length of 10:
@@ -241,20 +241,36 @@ public struct Configuration: Codable, Equatable {
241241
case always
242242

243243
var isNever: Bool {
244-
switch self {
245-
case .never:
246-
return true
247-
default:
248-
return false
249-
}
244+
self == .never
250245
}
251246

252247
var isAlways: Bool {
248+
self == .always
249+
}
250+
}
251+
252+
/// A private enum created to maintain backward compatibility with swift-format version 601.0.0,
253+
/// which had a `MultilineStringReflowBehavior` enum without a String raw type.
254+
///
255+
/// In version 601.0.0, the `reflowMultilineStringLiterals` configuration was encoded as an object
256+
/// with a single key (e.g., `{ "never": {} }`) rather than as a string (e.g., `"never"`). This
257+
/// enum allows decoding from both formats:
258+
/// - First, we attempt to decode as a String using `MultilineStringReflowBehavior`
259+
/// - If that fails, we fall back to this legacy format
260+
/// - If both attempts fail, an error will be thrown
261+
///
262+
/// This approach preserves compatibility without requiring a configuration version bump.
263+
private enum LegacyMultilineStringReflowBehavior: Codable {
264+
case never
265+
case onlyLinesOverLength
266+
case always
267+
268+
/// Converts this legacy enum to the corresponding `MultilineStringReflowBehavior` value.
269+
func toMultilineStringReflowBehavior() -> MultilineStringReflowBehavior {
253270
switch self {
254-
case .always:
255-
return true
256-
default:
257-
return false
271+
case .never: .never
272+
case .always: .always
273+
case .onlyLinesOverLength: .onlyLinesOverLength
258274
}
259275
}
260276
}
@@ -371,9 +387,31 @@ public struct Configuration: Codable, Equatable {
371387
)
372388
?? defaults.multiElementCollectionTrailingCommas
373389

374-
self.reflowMultilineStringLiterals =
375-
try container.decodeIfPresent(MultilineStringReflowBehavior.self, forKey: .reflowMultilineStringLiterals)
376-
?? defaults.reflowMultilineStringLiterals
390+
self.reflowMultilineStringLiterals = try {
391+
// Try to decode `reflowMultilineStringLiterals` as a string
392+
// This handles configurations using the String raw value format (e.g. "never").
393+
// If an error occurs, we'll silently bypass it and fall back to the legacy behavior.
394+
if let behavior = try? container.decodeIfPresent(
395+
MultilineStringReflowBehavior.self,
396+
forKey: .reflowMultilineStringLiterals
397+
) {
398+
return behavior
399+
}
400+
401+
// If the modern format fails, try to decode as an object with a single key.
402+
// This handles configurations from swift-format v601.0.0 (e.g. { "never": {} }).
403+
// If an error occurs in this step, we'll propagate it to the caller.
404+
if let legacyBehavior = try container.decodeIfPresent(
405+
LegacyMultilineStringReflowBehavior.self,
406+
forKey: .reflowMultilineStringLiterals
407+
) {
408+
return legacyBehavior.toMultilineStringReflowBehavior()
409+
}
410+
411+
// If the key is not present in the configuration at all, use the default value.
412+
return defaults.reflowMultilineStringLiterals
413+
}()
414+
377415
self.indentBlankLines =
378416
try container.decodeIfPresent(
379417
Bool.self,

Tests/SwiftFormatTests/API/ConfigurationTests.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,44 @@ final class ConfigurationTests: XCTestCase {
6464
let path = #"\\mount\test.swift"#
6565
XCTAssertNil(Configuration.url(forConfigurationFileApplyingTo: URL(fileURLWithPath: path)))
6666
}
67+
68+
func testDecodingReflowMultilineStringLiteralsAsString() throws {
69+
let testCases: [String: Configuration.MultilineStringReflowBehavior] = [
70+
"never": .never,
71+
"always": .always,
72+
"onlyLinesOverLength": .onlyLinesOverLength,
73+
]
74+
75+
for (jsonString, expectedBehavior) in testCases {
76+
let jsonData = """
77+
{
78+
"reflowMultilineStringLiterals": "\(jsonString)"
79+
}
80+
""".data(using: .utf8)!
81+
82+
let config = try JSONDecoder().decode(Configuration.self, from: jsonData)
83+
XCTAssertEqual(config.reflowMultilineStringLiterals, expectedBehavior)
84+
}
85+
}
86+
87+
func testDecodingReflowMultilineStringLiteralsAsObject() throws {
88+
89+
let testCases: [String: Configuration.MultilineStringReflowBehavior] = [
90+
"{ \"never\": {} }": .never,
91+
"{ \"always\": {} }": .always,
92+
"{ \"onlyLinesOverLength\": {} }": .onlyLinesOverLength,
93+
]
94+
95+
for (jsonString, expectedBehavior) in testCases {
96+
let jsonData = """
97+
{
98+
"reflowMultilineStringLiterals": \(jsonString)
99+
}
100+
""".data(using: .utf8)!
101+
102+
let config = try JSONDecoder().decode(Configuration.self, from: jsonData)
103+
XCTAssertEqual(config.reflowMultilineStringLiterals, expectedBehavior)
104+
}
105+
}
106+
67107
}

api-breakages.txt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,8 @@ API breakage: enum Finding.Severity has been removed
1313
API breakage: var Finding.severity has been removed
1414
API breakage: var FindingCategorizing.defaultSeverity has been removed
1515
API breakage: var FindingCategorizing.defaultSeverity has been removed
16-
API breakage: func Rule.diagnose(_:on:severity:anchor:notes:) has been renamed to func diagnose(_:on:anchor:notes:)
16+
API breakage: func Rule.diagnose(_:on:severity:anchor:notes:) has been renamed to func diagnose(_:on:anchor:notes:)
17+
API breakage: func Configuration.MultilineStringReflowBehavior.hash(into:) has been removed
18+
API breakage: func Configuration.MultilineStringReflowBehavior.encode(to:) has been removed
19+
API breakage: var Configuration.MultilineStringReflowBehavior.hashValue has been removed
20+
API breakage: constructor Configuration.MultilineStringReflowBehavior.init(from:) has been removed

0 commit comments

Comments
 (0)