nicklockwood / SwiftFormat

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

`hoistAwait` removes necessary `await` for operators/functions with async autoclosures. #1531

Closed lhunath closed 10 months ago

lhunath commented 11 months ago

The following code should be perfectly fine:

// TODO: Remove me when https://github.com/apple/swift/pull/36830 merges.
infix operator ???: NilCoalescingPrecedence
public func ??? <T>(optional: T?, defaultValue: @autoclosure () async throws -> T) async rethrows -> T {
    switch optional {
        case let .some(value):
            return value
        case .none:
            return try await defaultValue()
    }
}
func onLoad() async {
    let organization = await self.sessionInteractor.stage.values.first??.organization ???
        (await self.projectsInteractor.global.organizations().map(\.?.last).removeNil().values.first)
}

But it triggers:

(hoistAwait) Move inline await keyword(s) to start of expression.

The format operation also removes the second await. The result of the format operation is:

let organization = await self.sessionInteractor.stage.values.first??.organization ???
    (self.projectsInteractor.global.organizations().map(\.?.last).removeNil().values.first)
    // Expression is 'async' but is not marked with 'await'
nicklockwood commented 11 months ago

Unfortunately SwiftFormat isn't smart enough to recognize that an operator could introduce a new async scope within an expression. I'm not sure what I can do to easily fix this.

If the operator is declared in the same file then SwiftFormat could in theory parse it and handle this case, but in most cases I'd guess the operator would probably be defined elsewhere.

I'll think about it. In the meantime, I suggest either disabling the rule globally for your project, or adding the following comment directive wherever you use this operator:

// swiftformat:disable:next hoistAwait
lhunath commented 11 months ago

Thanks. Will this situation change when Swift decides to merge the linked PR and make the default operator ?? support async operations?

nicklockwood commented 11 months ago

I'm not sure. I guess it depends on whether reasync requires the explicit await keyword after the operator as it does currently? If so I'll probably just start having to assume that ?? may be async and not elide the await in that case.

(Note that the PR you referenced is from 2021 though, and there's been no activity for a while - I suspect it isn't landing any time soon)

nicklockwood commented 10 months ago

For now I've added the ability to add operators like ??? to the list of async-capturing functions specified with the --asynccapturing option. That will at least can you can work around the issue without need to disable the rule completely. If/when Apple adds reasync then I'll add ?? to the list by default.

nicklockwood commented 10 months ago

@lhunath support for --asynccapturing ??? has landed in 0.52.5