- What is your evaluation of the proposal?
I think it's a step in a right direction. I'd like to discuss handling of cases where it's possible for forward matching to silently behave differently behavior from backward matching but still type-check.
Let's consider this example:
func executeQuery(
onSuccess: ((Result) -> Void)? = nil,
onError: ((Error) -> Void)? = nil,
onCompletion: ((Either<Result, Error>) -> Void)? = nil) {
...
}
executeQuery { log("Query: \(query)") }
Edit: onCompletion: parameter supposed to be onCompletion: ((Either<Result, Error>) -> Void)? = nil.
Based on the proposed rules forward matching would pick onSuccess: but backward one would pick onCompletion:, silently allowing such call to type-check would mean that in Swift today log message would be emitted for every query but in the future it would be emitted only for successful ones. Change in behavior like this could be a surprise for a developer working on that code that's why emitting at least a warning should be considered here to give users a clear indication that, because of the change in compiler, code might behave differently at runtime and they should provide a label to make it explicit for the future readers what is expected behavior in this case.
I think type-checker rule proposed here should be extended to attempt N possible positions where trailing closure is acceptable in "source compatibility mode" and, in this case, try to type-check all possibilities and if multiple match - emit a warning (or an error) if behavior of forward matching is different from "old" backward scanning and suggest the developer to add a label based on what is expected behavior here.
We could keep that until Swift 6 and afterwards switch to pure forward matching without attempting other arguments.
I'd like to add that IHMO executeQuery as written should be banned so instead of warning I think in cases like this it's more appropriate to produce an error asking developer to explicitly specify label (onSuccess: vs. onCompletion:) to define what behavior is expected from the call, but determining this would still require attempting to type-check trailing closure at all three positions with, in my opinion, is a valid trade-off to preserve correctness with added benefit if producing precise diagnostics.
Another example, case mentioned in the proposal:
init(startHandler: ((AOperation) -> Void)? = nil,
produceHandler: ((AOperation, Foundation.Operation) -> Void)? = nil,
finishHandler: ((AOperation, [NSError]) -> Void)? = nil) {
self.startHandler = startHandler
self.produceHandler = produceHandler
self.finishHandler = finishHandler
}
BlockObserver { (operation, errors) in
print("finishHandler!")
}
Type-checker could produce a warning or error - trailing closure matches both 'produceHandler:' and 'finishHandler:' parameters, explicitly specify label to disambiguate and give two fix-its.
Note that if closure has explicitly declared parameter or result type e.g. (operation: AOperation, error: [NSError]) then trailing closure would have only matched produceHandler: and diagnostic would have to point out that neither startProducer: nor finishProducer: would match and explicit label is always required here to form a correct call.
- Is the problem being addressed significant enough to warrant a change to Swift?
Yes, backward scanning rule is a source of complexity and surprising behavior with multiple significant downsides.
- Does this proposal fit well with the feel and direction of Swift?
Yes, it tries to make behavior at the call site consistent.
- If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
N/A
- How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
In-depth study.