nicklockwood / SwiftFormat

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

`preferKeyPath`: fails to handle non-trailing parameters & optional type incompatibilities. #1197

Closed lhunath closed 2 years ago

lhunath commented 2 years ago

Considering the following utility:

public extension Binding {
    /// Get a binding for a collection of items of a different value type that's based on the collection values in this binding, silently dropping from the collections any values that are not supported by the other type.
    func compactMap<C>(_ transform: @escaping (Value.Element) -> C.Element?, reverse: @escaping (C.Element) -> Value.Element?) -> Binding<C>
        where Value: Collection & ExpressibleBySequence, C: Collection & ExpressibleBySequence {
        Binding<C>(
            get: { C(self.wrappedValue.compactMap(transform)) },
            set: { self.wrappedValue = Value($0.compactMap(reverse)) }
        )
    }
}

Utilized like this:

self.$model.selection.compactMap { $0.id } reverse: { id in
    self.model.members.values.first { $0.id ==  id }
}

swiftformat reformats the usage point to:

self.$model.selection.compactMap(\.id) reverse: { id in
    self.model.members.values.first { $0.id ==  id }
}

Which is illegal.

  1. SwiftFormat should properly understand block arguments that are not trailing blocks.
  2. SwiftFormat should ensure that the return value of the keypath is equivalent to the return value of the block parameter, since certain type translations do not happen.

For instance, the following reformatting may seem to be a valid fix:

self.$model.selection.compactMap(\.id, reverse: { id in
    self.model.members.values.first { $0.id ==  id }
})

However, since \.id is a non-optionally typed property and compactMap's first block takes a block with an optionally-typed return value, the two are not in fact compatible (even though an explicit block allows transparent type conversion to take place, this does not happen for under the keypath short-hand syntax).

nicklockwood commented 2 years ago

@lhunath the first issue is now fixed in SwiftFormat 0.49.10. The second is a known issue that isn't easily solved (see known issues in README.md).

lhunath commented 2 years ago

Is there any mechanism for SwiftFormat to check whether its output is syntactically legal? Though I suppose for detecting [2] we would require a type checker, perhaps the requirements machine is relevant.