Support use of an optional label for the first trailing closure

@xwu, thank you for putting together this proposal. In addition to being incredibly well-written, it does a fantastic job of laying out the motivations for the optional first label.

I especially appreciated the motivation for the ability to disambiguate the intended matching parameter as this was one of the rough edges of SE-0279 for me (and for others that I've talked with.) Being able to provide the first label will reduce the cognitive load on the programmer by allowing them to declare what they expect rather than stepping through the rules of backwards scanning to deduce what the compiler will choose.


The illustrations laid out in the above section were extremely helpful in visualizing how this form might be preferred over others. Calling it out as a possibility was a good move, but I might even consider increasing its prominence in the actual proposal.

I think the future directions are interesting, though I worry that concise single trailing closure APIs such as DispatchQueue.async { ... } would suffer under mandatory labels.

Again, many thanks for putting this together!

2 Likes

Thanks for this, it's extremely well-written and for the most part reflects perfectly the ideas and concerns that were manifested in the SE-279 review and acceptance threads. Here's my two cents (probably just one).

I would accept the proposal as-is, but a small improvement I can see is a stronger story for signaling the elision of the first argument label at call site when the label is not _:. I honestly would go as far as making the compiler emit a warning: if the label for the first closure is not _: then it should be written explicitly, and it will become an error in a future version of Swift. I know that this will cause warnings in many codebases where APIs were used as suggested by the guidelines (including my codebases), but I don't care: we need to fix this, and we might as well start now. I really don't think one should worry about improvements to the language that cause warnings in valid codebases, but maybe it's just me.

Another option would be, as @anandabits suggested, to add an annotation to require the label, that will cause an error if it's elided: it's fine, but we already have a tool for eliding parameter labels, that is, _:, so I would simply leverage that. Also, my future self, while writing new shining APIs that use multiple closures and take advantage of SE-279, will probably not want to add that annotation over and over again.

Anyway, can't wait for this to be accepted and implemented :smiley:

One minor quirk is that, because SE-279 has been already implemented in Swift 5.3, there will be a period of "limbo" when I will probably write APIs ready to be deprecated when Swift 5.4 is released (or something like that) :smiley:

6 Likes

To be clear, this is my preference as well. The attribute I propose is in anticipation of the community at large and Core Team in particular not being willing to accept the source churn a warning for all misaligned call sites would cause.

1 Like

Maybe (probably) this is a completely horrible idea, but what about combining the first parameter label with the function name?

func count(where: (T) -> Bool) -> Int {
   ...
}

could be called like this:

let result = foos.countWhere { $0 > 5 }

or this

let result = foos.count { $0 > 5 } //assuming there weren't a typechecker problem with count(where:)

but not this

let result = foos.count where: { $0 > 5 } 

That's essentially what the API naming guideline revision would have authors do in terms of API naming.

As noted in the text, it doesn't address the problem of what to do here:

func foo(bar: Int, baz: Int, boo: () -> Void) { ... }
foo(bar: 42, baz: 42) /* now how do we label 'boo'? */ { ... }

...nor does it solve the issue of what to do when there is more than one possible first trailing closure:

func foo(bar: (() -> Int)? = nil, baz: (() -> Int)? = nil) { ... }
// let's rename to:
func fooBar(_: (() -> Int?) = nil, baz: (() -> Int)? = nil { ... }
fooBar { 42 } // oops, that closure actually goes with 'baz'
3 Likes

This has been brought up previously. The consensus is that names like countWhere is anti-swifty.

7 Likes

To that point, though, the revised API naming guidelines as a result of SE-0279 would encourage that name.

I really like this proposal, it gives flexibility to the user whether to expose the label for the closure or not, and makes sense with the fact that _ arguments are meant to be omitted in the Swift APIs. Also I'm feeling that this won't go through revision due you the firm decision over how labels are treated, which I'd love that they think again about it. The semantics makes sense and the resilience with the APIs as well, so it's a +1 for me.
Great job btw.

Really good proposal. It reflects my ideas on the topic quite accurately. I’m in favor of the push towards the new syntax without being too aggressive (warning or even failing to compile when “non-preferred” syntax is used in the current version of Swift). I would, though, like to see the non-preferred syntax slowly phased out in future versions. Overall great work!

-Filip

This looks like a huge improvement for consistency, call-site clarity, and swiftiness.

Strongly in favor!

NB. I’d also be in favor of actually requiring the first label if the function defines one. We already have a mechanism for omitting labels: func frob(_ f: () -> Void) can be written as frob { … }.

8 Likes

Strongly +1 to this, but I may be in the minority that would welcome mandatory labels where defined by the API authors.

Swift already supports label elision at the call site as declared by the API author using _. I believe that the sooner we move to enforcing consistency with this general syntax rule, the sooner API authors will move towards designs which clarify the call-site.

Conversely, the more leeway given via optional “suggested” practices, the longer we’ll be living with inconsistent APIs across first- and third-party code.

As a consumer of APIs, I really don’t want different ways of calling said APIs. I want a single canonical spelling that is clear and consistent at all points.

15 Likes

Thank you @xwu for writing this up! I'm probably in the minority in being ok with SE-0279 going through as it was, but I do think first parameter names ought to be an option at some point.

I'm against always requiring the label as-is mainly because I don't like the functionName parameterName syntax. It looks confusing, non Swift-like, and AFAIK "name otherName" isn't a syntax that exists anywhere in the language. Trailing closures are already confusing for new users, and I would rather not add to that noise by adding new syntax they have to visually parse.

I'm unsure whether this has been considered, but what about something similar to function-reference syntax? functionName(parameterLabel:) { closure } Assuming it wouldn't be too complex to parse (big assumption :innocent:) , it seems a bit more like code and would still be source compatible.

But IMO the biggest advantage is that syntax already exists in the language for a directly related subject.

Comparison:

// My preferred formatting for short closures with SE-0279, but at the cost of losing the clarifying label.
thing.bind { otherThing }
    set: { otherThing = $0 }

// Doesn't look right to me for Swift/Code
thing.bind get: { otherThing }
    set: { otherThing = $0 }

// Looks semi-right, but takes an extra line which isn't always preferable
thing.bind
    get: { otherThing }
    set: { otherThing = $0 }

// IMO This gets the best of both worlds
thing.bind(get:) { otherThing }
    set: { otherThing = $0 }

// Additional formats:

thing.bind(get:) { 
        otherThing 
    } set: { 
        otherThing = $0 
    }

thing.bind(get:) 
    { otherThing } 
    set: { otherThing = $0 }

In terms of the migration story, if/when it was made a requirement or preference to use the first parameter name this seems like a gentler transition from the existing trailing closure syntax. It would be an easy fixit with familiar and consistent syntax compared to the floating label which is new and should be put different places depending on the situation.

2 Likes

I would expect the asymmetry of requiring parentheses around the first closure label but none of the others (in fact, they are forbidden to have them) would be far more likely to cause confusion than the mere juxtaposition of a function's basename and its first trailing closure label.

Among all of the examples that you pasted, I find this to be the most readable (by far) due to the symmetry of its labeling and its indentation, and all of the examples requiring parentheses and the various wrapping/indentation used to try to adjust for that made them harder to parse mentally:

thing.bind
    get: { otherThing }
    set: { otherThing = $0 }

So, I don't buy the argument that this particular juxtaposition isn't "Swift-like". We've already accepted SE-0279, which juxtaposes multiple trailing closures separated by only their label, but even without that, this is a natural and clean extension of that which removes special cases from the language. That, to me, is a more Swift-like characteristic (and the goal to strive for), not strictly limiting ourselves to syntactic neighboring pairs that already existed.

11 Likes

I agree for some multi-closure cases, but definitely not for single closures:

myArray.first 
    where: { $0 < 5 }
// or
myArray.first where: { $0 < 5 }

// v.s.
myArray.first(where:) { 
    $0 < 5 
}
// or
myArray.first(where:) { $0 < 5 }

That syntax would naturally be supported if/we generalize support for compound names. However, when there are other arguments to pass, the result will look like this, which I don't think you'll particularly find very elegant:

func f(bar: Int, baz: Int, boo: () -> Void) { ... }

f(bar:baz:boo:)(42, 42) { ... }

Likewise, the example of bind(get:set:) would look like this (and, yes, the logic of compound names would be such that you would need to use an empty label):

bind(get:set:)
  { /* getter */ }
  _: { /* setter */ }

...and if we have a function both with non-closure arguments and multiple closure arguments, then you get a very unfortunate call site indeed:

func g(bar: Int, baz: () -> Void, boo: () -> Void) { ... }

g(bar:baz:boo:)(42) { ... } _: { ... } /* yikes */

It should also be noted that, so long as the first trailing closure is unlabeled, we likely cannot make the syntax work inside a statement condition (such as if), which is one of the principal motivating use cases here.

2 Likes

I dont like the function reference with out the parens. To me the parens r important since they indicate that I am calling the func.

thing.bind()
    get: { otherThing }
    set: { otherThing = $0 }

thing.bind() get: { 
      otherThing 
    } 

thing.bind get: { // error; add ()
      otherThing 
    } 



That's really an orthogonal issue: Swift has supported [1, 2, 3].first { ... } instead of [1, 2, 3].first() { ... } since the very beginning.

Changing that rule would be out of the scope of this pitch, but if you don't like this style it's certainly something that a linter can enforce (I imagine there are tools out there already that can do this).

6 Likes

I would expect that to be calling the function reference, rather than the function directly, but that is admittedly a potential confusing / problematic issue.

Anyway what I was thinking of is something that would only involve the first closure label

func g(bar: 42, baz:) {} boo {}

Though similar to @masters3d I would be comfortable with the label being around after parens:

func g(bar: 42) baz: {} boo: {}

I guess my primary concern is to me this looks too much like plain text:

thing.makeCall completion: { $0 < 4 }

Although that's normal in some languages, (Python) it's abnormal in Swift. I believe Swift is consistently formatted as

sigil(s) name||value, sigil(s)

A floating label breaks that pattern and IMO should only be added very intentionally in basic syntax (excluding something like DSL support)

1 Like

Enforcing parens at call sites, if that makes you more comfortable, is a straightforward thing for a linter to do, as I mentioned. It's not an unreasonable thing to want, and it applies to the existing trailing closure syntax also. Go for it :)

Ultimately, I think you're among the group who simply find the proposed spelling "unsettling" (as described in the proposal text), but I think once the novelty wears off it'll seem quite natural, since it's a generalization of an existing rule.

This is also why, instead of charging full on to making the compiler warn about using the syntax we already have, I'm proposing that we give the community time to coalesce around a set of best practices and styles; actual hands-on experience over time will guide us here. As you say, there are some situations in which the proposed spelling here is a clear win, and others where some people are on the fence; as with other aspects of closure syntax, not one style will be best for every use site: experience shows that we can trust users to pick what works best for them when we give them the option (which currently we do not in the case of first trailing closures).

(Incidentally, as we generalize other features, you are bound to see more instances where words are juxtaposed with each other. Off the top of my head, when we generalized opaque types, you'll start seeing spellings such as 42 as some FloatingPoint. These things will just fall naturally out of the existing rules. It's also rather amusing that we are now worried about call sites reading too well, when the API naming guidelines specifically recommend call sites that read fluently: "Prefer [to] make use sites form grammatical English phrases.")

I agree that generalizing support for compound names will give another way in which we can call first(where:), and that it's perfectly reasonable in that case; it doesn't work well for more complicated callers as I showed you. Therefore, I don't think it addresses the same problems as would generalizing support for labels to the first trailing closure. (I do support generalizing support for compound names though!)

If you're arguing that instead of generalizing any rules, we should invent a third kind of syntax specifically for the first trailing closure, I simply don't see how that could be justified.

10 Likes