nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.62k stars 623 forks source link

Fix closure argument list parsing bug in preferForLoop rule #1712

Closed calda closed 4 weeks ago

calda commented 1 month ago

This PR fixes #1709.

This code was being parsed incorrectly:

dict.forEach { (header: (key: String, value: String)) in
    print(header.key)
    print(header.value)
}

so was unexpectedly converted to:

for (header, value) in dict {
    print(header.key)
    print(header.value)
}

instead of:

for header in dict {
    print(header.key)
    print(header.value)
}
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.20%. Comparing base (f487996) to head (adea102).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1712 +/- ## =========================================== + Coverage 95.13% 95.20% +0.07% =========================================== Files 20 20 Lines 23124 23135 +11 =========================================== + Hits 21998 22025 +27 + Misses 1126 1110 -16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nicklockwood commented 1 month ago

I think maybe this only works by coincidence? The key/value labels happen to match the defaults in Dictionary, but what if different labels had been used instead?

calda commented 1 month ago

Just checked -- if you reassign the labels, the compiler emits a warning suggesting you not do this:

let dict = ["foo": "bar"]

// ⚠️ warning: tuple conversion from '(key: String, value: String)' to '(a: String, b: String)' mismatches labels
dict.forEach { (header: (a: String, b: String)) in
    print(header.a)
    print(header.b)
}

So while renaming tuple keys in that way is technically possible, it seems like an anti-pattern that I think is ok for us to not support.

alistra commented 1 month ago

LGTM