Fix preferLazyMap rewrites that were not behavior-preserving - #2661
Merged
calda merged 1 commit intoAug 20, 2026
Conversation
Making a `map` lazy changes more than whether an intermediate array is
allocated: the closure becomes escaping, the resulting sequence participates in
overload resolution differently, and the receiver now has to be a `Sequence`.
Each produced broken rewrites when the rule was run over a large codebase.
**Implicit `self` in the closure.** `lazy.map` stores its transform, so its
closure is escaping while eager `map`'s is not. Inside a reference type that
difference is load-bearing:
final class Generator {
func render(_ value: Int) -> String { "\(value)" }
func eager() -> String { values.map { render($0) }.joined(separator: ",") }
// error: call to method 'render' in closure requires explicit
// use of 'self' to make capture semantics explicit
func lazyOne() -> String { values.lazy.map { render($0) }.joined(separator: ",") }
}
Whether a bare name is a member of `self`, a global function, or a type can't be
resolved from tokens, so the rule now rewrites only when the closure body
references nothing but its own arguments. Member accesses reached through a `.`,
argument labels, keywords, and literals still qualify, so the projections this
rule targets (`$0.foo`, `$0.a.b`, `$0.name(x: true)`) are unaffected. This is
deliberately conservative: `hypot($0.x, $0.y)` is skipped even though it captures
no self, being indistinguishable from a member reference here.
**`joined()` with no separator.** On a lazy sequence this resolves to the
sequence-flattening overload rather than the `StringProtocol` one, silently
changing the result type:
parts.map { $0 + "!" }.joined() // String
parts.lazy.map { $0 + "!" }.joined() // LazySequence<FlattenSequence<...>>
That is worse than a compile error when it lands in a string interpolation, which
then emits the sequence's description instead of the joined text — it corrupted
the output of a macro that string-builds source. `joined` now requires a
separator; `joined(separator:)` returns `String` either way.
**Identity maps.** `map { $0 }` transforms nothing, so laziness saves no work,
while `lazy` still has to exist on the receiver. Since `map` is defined on plenty
of non-`Sequence` types (`Optional`, `Result`, publishers, custom iterators) and
the receiver's type isn't knowable here, these are now left alone — no upside,
and it removes a way for the rewrite to stop compiling.
Found by enabling the rule across a ~50k-file first-party codebase: the checks
together skip 189 of 500 candidate sites, and the remainder now builds clean.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2661 +/- ##
===========================================
+ Coverage 95.45% 95.46% +0.01%
===========================================
Files 178 178
Lines 27137 27186 +49
===========================================
+ Hits 25904 25954 +50
+ Misses 1233 1232 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
calda
approved these changes
Aug 20, 2026
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2656. Enabling
preferLazyMapacross a ~50k-file first-party codebase surfaced three ways the rewrite was not behavior-preserving. This tightens the rule so the same sweep now builds clean.Making a
maplazy changes more than whether an intermediate array is allocated:selfin a reference typejoined()resolves to a different overloadStringbecomes a lazy sequence.lazyrequiresSequencemap-providing types1. Implicit
selfin the closurelazy.mapstores its transform, so its closure is escaping while eagermap's is not. Inside a reference type that difference is load-bearing:Whether a bare name is a member of
self, a global function, or a type isn't resolvable from tokens, so the rule now rewrites only when the closure body references nothing but its own arguments. Member accesses through a., argument labels, keywords and literals still qualify, so the projections this rule targets ($0.foo,$0.a.b,$0.name(x: true)) are unaffected. Deliberately conservative:hypot($0.x, $0.y)is skipped even though it captures no self, being indistinguishable from a member reference here.2.
joined()with no separatorOn a lazy sequence this resolves to the sequence-flattening overload instead of the
StringProtocolone:This is the one I'd flag hardest, because it doesn't fail at the rewritten line. In a string interpolation it emits the sequence's description, which corrupted the generated output of a macro that string-builds source.
joinednow requires a separator argument;joined(separator:)returnsStringeither way.3. Identity maps
map { $0 }transforms nothing, so laziness saves no work whilelazystill has to exist on the receiver.mapis defined on plenty of non-Sequencetypes (Optional,Result, publishers, custom iterators), so these are now left alone: no upside, and it removes a way for the rewrite to stop compiling.A caveat worth naming
(3) only removes the instances we hit. In general a token-based rule cannot know that the receiver of
.mapis aSequence, so a rewrite can still fail to compile on a non-Sequencereceiver. That seems acceptable for a disabled-by-default rule, but I'm happy to document it in the rule's help text, or narrow the rule further.