nicklockwood / SwiftFormat

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

Redundant self throwing compiler errors #1571

Open shkhaliq opened 8 months ago

shkhaliq commented 8 months ago

I am seeing compiler errors across my codebase after bumping the swift-version to 5.9.

SwiftFormat: 0.52.6 Swift Version 5.9 Xcode 14.3.1

An example

        }.didSelect { [weak self] _, someService in
            guard !isPlaceholder, let self else {
                return
            }
---       self.selectionHandler(someService.retailer, self)
+++     selectionHandler(someService.retailer, self)
        }
calda commented 8 months ago

If this because the closure is inside a result builder?

There's a known issue in the Swift 5.8 and Swift 5.9 compilers where the implicit weak self functionality doesn't work correctly for closures inside a result builder. This is fixed in Swift 5.10. In the meantime, the only solution would be to either disable the reundantSelf rule for your codebase, or disable the redundantSelf rule for that specific line:

// swiftformat:disable:next redundantSelf
self.selectionHandler(someService.retailer, self)

I fixed this compiler bug in https://github.com/apple/swift/pull/64786, which was merged in April. Unfortunately it just missed the Swift 5.9 branch cut and didn't end up getting cherry-picked into 5.9, but is included in 5.10. The compiler has really, really long release cycles!

nicklockwood commented 8 months ago

@calda there are already other special cases in SwiftFormat for things that are disallowed inside result builders (although the detection method is a bit crude). Do you think it's worth adding a special case for this too (gated to Swift 5.9)?

calda commented 8 months ago

It’s not possible to detect being inside a result builder scope in the general case, especially with how any closure can be a result builder scope, but we could conceivably handle common cases like the SwiftUI var body: some View and other @ViewBuilders.

calda commented 8 months ago

Although I think this is actually less likely to come up with SwiftUI, since everything is a value type and you’re less likely to use weak self in the first place.

@shkhaliq, could you share some full code examples of this that include the result builder scope that contains the weak self closure?

nicklockwood commented 8 months ago

we could conceivably handle common cases like the SwiftUI var body: some View and other @ViewBuilders.

Yep, that's exactly what the current heuristic does.