SE-0279: Multiple Trailing Closures [Amended]

I’m not referring to the degree of implementation challenge, but the challenge of correctly using and teaching the rule.

1 Like

Well, good API does take a lot of iteration. So it'd be fair to say that old API won't play well with new syntax. Still, how about we start with:

func with(
  duration: Double,
  animate: @escaping () -> Void,
  onComplete: ((Bool) -> Void)? = nil)

view.with(duration: 0.0, animate: defeatEnemy, onComplete: updateScore)
view.with(duration: 0.0, animate: defeatEnemy)
  onComplete: { ... }
view.with(duration: 0.0)
  animate: { ... }
  onComplete: { ... }

I'd say the reason I'm advocating for requiring the first label is that the API guideline doesn't need to play guessing game regarding whether the label is being dropped. I think It's a big plus from design perspective.

4 Likes

I wasn't suggesting changing overload resolution. The compiler can continue matching parameters and overloaded functions exactly as this proposal suggests. Only if it ends up matching a function where the label is not allowed to be implicit, it'd emit a warning accompanied by a fix-it that makes the label explicit.

A parallel with `@discardableResult`

@discardableResult's only effect is that you can omit using the return value and the compiler won't complain. Here we want to be able to omit a label for the first trailing closure and not have the compiler complain. The main question is whether allowing the label to be omitted is opt-in or opt-out in the function declaration. The @discardableResult precedent would say that omissions should be opt-in, but to keep "warning-less" the current behavior for single trailing closures it'd have to be opt-out.

The parallel to @discardableResult goes further since we actually flipped the default behavior for this in Swift 3 with SE-0047. The justification was "Real world situations where it makes sense from a compiler point of view to discard non-Void results are rare." We could do the same and flip to an opt-in model for when a label can be omitted. That's certainly doable and it'd be my preferred behavior, but I'll admit the justification probably isn't there.

3 Likes

Okay, thanks.

This most cases there is, but consider the filter method:

myArray.filter { condition } 

Personally I don’t find this very clear to the purpose. Without an argument label is confusing as I see it.

myArray.filter() where: { condition }

This conveys to the developer what the closure does much better than just using an unlabeled trailing closure.

2 Likes

I'm -1 on this proposal.

I think it should be the developer's choice whether to use trailing closure syntax or not, and in case of UIView.animate the code becomes more clear when you don't use this syntax and just stick to passing lambdas as function parameters.

Personally I would prefer this (the currently valid syntax):

UIView.animate(withDuration: 0.3,
               animations: {
                   // ...
               },
               completion: { _ in
                   // ...
               })

over this (the proposed new syntax):

UIView.animate(withDuration: 0.3) {
    // ...
} completion: { _ in
    // ...
}

The difference isn't dramatic, however for me ( & ) make this code more readable by setting boundaries of the function call.

This syntax might also become problematic when a function returns a closure:

func foo(p1: () -> (), p2: () -> ()) -> (() -> ()) {
    return {}
}

// current syntax, works fine:
foo(p1: {},
    p2: {}) ()

// proposed syntax, unclear which closure is being invoked:
foo() p1: {}
      p2: {} ()
// now let's say there is a function with the same name defined like this:
func foo(p1: () -> (), p2: ())
// which `foo` should be called?

The actual issue seems to be that Xcode completion suggests to use trailing closure syntax for UIView.animate and similar methods. So I feel like this proposal is an attempt to solve a tooling issue with a fix to the language itself.

The @implicit / @explicit annotations discussed in the thread make this idea somewhat more clear, but at the same time they add unnecessary complexity to the language, and in my view this is a more serious issue than the one this proposal is aiming to resolve.

17 Likes

+0

I am not necessarily opposed to the syntax as shown in the examples. However, the explicitness is dependent on this

UIView.animate(withDuration: 0.3) {
    self.view.alpha = 0
} completion: { _ in
    self.view.removeFromSuperview()
}

not turning into this

UIView.animate(withDuration: 0.3) {
    self.view.alpha = 0
}

completion: { _ in
    self.view.removeFromSuperview()
}

which makes the trailing closure completion: {} only one character different than a function call with a single trailing closure (completion {})

This may seem like an obvious "just keep your formatting straight and follow the style guide", but beginners are not formatting experts and are often off by one critical syntax character.

(I brought this up last review, but things were a bit; shall we say, lively :wink:)

3 Likes

Thanks for the feedback. I agree about splitting trailing closures being a bad idea, and there are certainly things we could do here. For example, we already have a warning about empty lines between the start of a call and the first closure, which we could definitely generalize to all of the gaps between trailing closures.

I'd be interested in knowing what you (and anyone else) think about a couple cases. What if completion: is on a line by itself, but that line is immediately after the previous closure?

UIView.animate(withDuration: 0.3) {
    self.view.alpha = 0
}
completion: { _ in
    self.view.removeFromSuperview()
}

Is separating closures vertically by comments okay? (All the intervening lines have to have comments.)

UIView.animate(withDuration: 0.3) {
    self.view.alpha = 0
}
// As soon as the animation is over, we can remove the view.
completion: { _ in
    self.view.removeFromSuperview()
}
3 Likes

I think we should maintain the stance that Swift is (largely) agnostic to the choice of whitespace. Esp. that this is still valid:

// well
func
// umm
foo1
// anyway
(
// this is fine
a:
//
Int) { }

We can agree that no one writes like this. Still, fixing that is probably way out of scope of this proposal. It's more important to figure out a structure that we can encourage, and enforced via style guide.

I want to added my thoughts on this... too. Since function-with-multiple-closure is a largely untouched api space and all. Which ever syntax we go with, the apis will start optimising toward the syntax, and likely away from similar syntaxes. While of course no one can predict the future, we probably want to be more careful than letting it out, then adjust it later.

14 Likes

Personally, I would say no to both comments and labels on new lines.

There does not appear to be a precedent for the compiler restricting types of comments based on context, and anything more than a single line comment gets in the way of reading the statement. For example:

UIView.animate(withDuration: 0.3) {
    self.view.alpha = 0
}
/*
As soon as the animation is over, we can remove the view.
We also need to stop the mouse before it eats all our cheese.

TODO: Stop *all* mice from eating cheese if more than one mouse is present.
*/
completion: { _ in
    self.view.removeFromSuperview()
    self.mouse.stopEatingCheese()
}

As for labels on new lines, I believe it adds ambiguity because it puts the label at the same indentation/level-of-importance as the main statement (making it much easier to confuse with an independent statement). That said, just making sure there are no empty lines between closures is a vast improvement.

1 Like

I completely agree that (as a general rule) the compiler should not try to enforce white space style.

However, in this instance a single colon could be the only differential between two vastly different outcomes.

It's like the stereotypical finicky programming language that fails to compile because you missed one semi-colon, except it might compile and then happily do something unexpected.

3 Likes

This is all interesting, thank you. (And feel free to keep it coming!)

I do add comments from time to time for similar calls or between chained calls. I also would separate multiple trailing closures on their own lines as shown upthread in several examples such as with fold or Binding.

Since this syntax requires a colon on closure labels, I personally might avoid the classic if else look and rather go with wrapped and indented labels on their own lines. That‘s my personal preference I hope we would support.

1 Like

I often put comments between chained calls too. I don't think I would ever do it with trailing closures in standard library or combine. However rethinking this a little, I could could see these "outer" comments as interesting with SwiftUI. It feels wrong to me in normal control-flow style trailing closures, but I could see it in conjunction with function builders. I think "outer" comments works better in a declarative style.

Having read through the proposal and the discussion in this thread, I want to echo some of the concerns others have expressed about complexity creep. I hope that in our efforts to improve the language, we don’t end up adding syntax that’s harder to understand and use correctly than what we have now.

In my mind, one of the most important things about the current training closure syntax is its simplicity. The rules can be understood quickly and are easy to pattern-match when reading code. This is especially valuable for newcomers who may not be familiar with this kind of language feature.

From the perspective of an “average Swift user,” I‘m concerned when I hear suggestions for new attributes that govern argument label usage, or special-case rules around whitespace and comments. More complexity means more mental overhead when reading and writing code, and the more syntactical surface area exposed, the more difficult it becomes for users to grok the language. It may be obvious to us why the first label in a series of trailing closures would be implicit but not the others, or what @explicit means in a function definition, or that myArray.first where: { condition } is a function call rather than some other language construct. But it may not be obvious to a new developer who doesn’t have a body of prior experience to reason from. I worry that we’re raising the “barrier to entry” of baseline knowledge needed to use trailing closures correctly.

A while ago, someone commented in another thread that Swift is beginning to feel like “a grab bag of syntax.” I hope we can find a solution to this design problem that feels like a natural extension of the language, and not just a “grab bag” addition.

22 Likes

Has there been any though about using subscript syntax for multiple trailing closures?

when(2 < 3) [
then: { print("then") } 
else: { print("else") }
]

This should allows to apply different rules than regular trailing closures rules.

No more ambiguous than the use of angular brackets for generics and as operators

It is different. Generics have a whitespace rule and a matching pair to differentiate. The equivalent would be:


foo() 
<bar>{ provideBar() } 
<baz>{  provideBaz()  } 
<garply>{ provideGarply()   }

Note: No space between >{ and a matching <.

With the pipe, that could be a call to function foo() and three calls to functions with trailing closures with a logical or between them.

Beyond that, it was expressed in the original proposal that there is not the desire to use prefix symbols for this feature since the ones that will work are rare (such as ^) and better reserved for future features.

The subscript/obj-c message syntax and similar syntaxes was also brought up in the original proposal. This was generally rejected because there is not the desire to add another level of nesting. There is also not the desire to add what could be considered a third calling convention. In this case it also looks too much like a dictionary.

Alternative syntaxes were explored greatly in the original proposal. I’m sure if there was something better it would have already come to the surface.

Is there a particular issue you have with the proposed syntax? The most cited is issues in if/guard conditions, but any alternatives also have compromises. I personally find that it fits well with named arguments and honors the SmallTalk heritage of the language.

Has it been discussed to optionally allow a colon as in the following to disambiguate a single trailing call in if/guard/switch?

I know an empty tuple is against current style guidelines, but maybe it shouldn't be– at least in certain contexts.

// parentheses required before this optional `:`
if foo(): { ... } {
    ...
}

// or

foo(): { 
    ...
} bar: { 
    ...
} baz: {
    ...
}

Multiple trailing closures already feel SmallTalk inspired, this would make it more so.

Typing this colon could kick off auto-complete suggestions.

A little more out-there complementary idea

A new attribute to prevent overrides (or something to a similar effect such as a preferred override or declaring it as a statement-style trailing closure) could be designed for trailing closures without parentheses. This would be used to allow these trailing closures without parentheses (or with parentheses, but without the optional colon mentioned above) to work in if/guard/switch with the idea that the optional trailing colon would mostly be used outside the standard library. Trailing closures without parentheses have always been somewhat troublesome. This could make them more specialized to particular use cases. For backwards compatibility this behavior could be optional, but required to use the trailing closure inside if/guard/switch. Auto-complete could prefer dropping parentheses for functions with the attribute, otherwise add parentheses and optional colon. I think this could add consistency by allowing the statement-like form only in circumstances where it would not be ambiguous. I think this would be very unlikely to break existing code since it would be optional and functions that would get the attribute would be unlikely to be overridden in existing code. Further, the attribute could be completely ignored outside of ambiguous contexts to allow full backwards compatibility in the short term.

@disallowOverrides // or @preferredOverride or @statementClosure
func filter(_ isIncluded: (Self.Element) throws -> Bool) rethrows -> [Self.Element] { ... }

// Disable overrides to allow this to work inside if/guard/switch.
if let selected = users.filter {$0.characters.count == 2}.first {
   print(selected)
}

nice idea, I like it. :

1 Like