Support use of an optional label for the first trailing closure

I am, not sure how you could interpret something else. ;)

Also keep in mind we're talking about a single optional label, not multiple optional labels. This proposal would allow the first trailing closure to explicitly specify a label, but does not force the user to do so. Any further trailing closures are required to provide a label.

I think @mattrips has pointed out an issue that should be discussed, so as to not make any regrettable decisions.

In my opinion, your example is the behavior that should be exhibited. I find the label a way to clarify the initialization call. Especially in member-wise initializers, which are usually used once in private code as @DevAndArtist pointed out, clarity is key. Without the clarity that labels provide the user may have to go back to the declaration to see what the unlabeled trailing closure means. Yes, Xcode usually shows the label before it transforms it into to a trailing closure, but still when reading old code it can be hard to understand and, thus, maintain.

I think @karim's logic in this post is sound and is a desirable way to approach the problem. (I suggest reading the whole post.)

Whether or not the label should be required should be left to the API author.

If the author of a SwiftUI view feels that an unlabeled trailing closure (for the content or the action or something else entirely) is the "desirable" way to construct their view, they should write an initializer with an _ external label. I realize that this would take away from some of the magic of member-wise initializers in your own module but in the case for SwiftUI (and any other dependency) the member-wise initializer must be explicitly written to expose it publicly anyways.

I know there has been talk of allowing for the scope of a synthesized member-wise initializer to be specified, and if that ever happens, that would be an excellent time to address that issue.

13 Likes

It's been a while, is this on hold? Or did I miss a new thread that starts the broader discussion?

It's not on hold, and you haven't missed anything. But we want to set up that broader discussion with some guidance from the core team. Putting together guidance like that takes a little time, in part because it needs to come from the core team as a unit rather than as individuals who can more rapidly put out their own thoughts, and in part because we want to be clear in that guidance.

15 Likes

I found another use case for supporting a label for the first trailing closure, that is, interaction with callAsFunction (I don't remember if it was already brought up). Consider the following:

func foo(bar: (Int) -> Void) {
  bar(42)
}

func foo(baz: (Int) -> Void) {
  baz(24)
}

func frobulate() {
  /// Fails: Ambiguous use of 'foo'
  foo {
    print($0)
  }

  /// The following works, I can disambiguate by writing the full function name.

  foo(bar:) {
    print($0)
  }

  foo(baz:) {
    print($0)
  }
}

struct Foo {
  func callAsFunction(bar: (Int) -> Void) {
    bar(42)
  }

  func callAsFunction(baz: (Int) -> Void) {
    baz(24)
  }
}

func frobnicate() {
  let aFoo = Foo()

  /// Fails: Ambiguous use of 'callAsFunction'
  aFoo {
    print($0)
  }

  /// I can't disambiguate here without removing trailing closures.

  /// Fails: Use of unresolved identifier 'aFoo(bar:)'
  aFoo(bar:) {
    print($0)
  }

  /// Fails: Use of unresolved identifier 'aFoo(bar:)'
  aFoo(baz:) {
    print($0)
  }
}

I can't disambiguate by including the parameter label in the base identifier, but with the suggested feature I could simply write:

aFoo bar: {
  print($0)
}

aFoo baz: {
  print($0)
}
12 Likes

Just to add another concrete case here.

I'm adding a new withAllRequests API to Alamofire to operate on all active Requests, similar to URLSession's getAllTasks. To make it compatible with the new syntax I designed it like this:

func withAllRequests(completingOn queue: DispatchQueue = .main,
                     perform action: @escaping (Request) -> Void,
                     completion: (() -> Void)? = nil)

When using the full syntax, it works pretty well:

session.withAllRequests { (request) in
    // Do something with all requests.
} completion: {
    // Do something on completion.
}

However, if I only want to use the action closure, trailing syntax can't be used all. The old syntax (which is completed incorrectly, I've filed SR-12880) only supports the last closure, so to use the action closure I'd need the full function syntax. This both awkward because I could've supported trailing syntax before with a single closure (though the full function usage wouldn't have been great) and because there's no autocomplete for just the action parameter directly. I have to select the full completion, delete the queue and completion parameters, select the action closure, and manually enter my closure (trying to complete just the action closure after deleting the other parameters triggers the same completion bug).

Being able to label the first closure would seem to solve these issues.

(As an aside, I think the new syntax will lead to issues where users complete the full series of closures, delete the closures after the first, and run into a compiler error because that closure can't be used as a trailer on its own.)

5 Likes

Update from the Core Team

Hi everyone – thank you for bearing with us. I have posted some thoughts from the core team in this thread and we’d appreciate your contributions there.

Thanks!
Ben

2 Likes

So the feature is out in the wild now and I tried it out with SwiftUI.

Here is my small report:

// now we can write this
Button {
  ...
} label: {
  ...
}.modifier()

// we could correct the alignment
Button
  .init {
    ...
  }
  label: {
    ...
  }
  .modifier()

// however, the ideal version should rather look like this 
Button
  action: {
    ...
  }
  label: {
    ...
  }
  .modifier()
17 Likes

I spoke to some Xcode engineers at WWDC labs about Xcode's autoindentation behavior. We largely covered . indentation in chained code and markdown code blocks, but we also spoke about automatic vertical . alignment and SwiftUI's }. behavior. Hopefully something comes of that soon (nothing new in today's b2).

Having just started to learn SwiftUI, I see this format as significantly easier to wrap my head around :upside_down_face:

6 Likes

Yes, this is Swift's "clarity at point of use" principle in practice. For trailing closures though, it was judged that the unlabeled first closure's symmetry with the single closure's unlabeled syntax was more desirable, for better or worse.

1 Like

Yet it breaks code alignment for vertical chains. I hope we can move the optional first label pitch forward.

2 Likes

Totally agree.

It should be moved forward into review.

2 Likes

FWIW, I think that the API of the Button initializer may fall into the camp of "would have been designed differently in Swift 5.3" as @beccadax notes re: sheet here. Using the same transformation that Brent does for sheet, the API might have been designed as:

// Declared as
init(label: () -> Label, action: @escaping () -> Void)

// Called as
Button {
  Text("My button")
} action: {
  print("Tapped")
}.modifier()

// Or...
Button 
  .init {
    Text("My button")
  } 
  action: {
    print("Tapped")
  }
  .modifier()

I actually prefer the existing Button API and would advocate for its current design (as well as that of sheet) even in light of SE-0279, since it has the important property that all @ViewBuilder closures come at the end of the parameter list.

Specifically in the SwiftUI context (though the logic extends to other eDSL contexts as well), my preferred approach is to place only the @ViewBuilder closures in trailing position and move imperative code out-of-line. Thus, the 'ideal' for me is actually:

Button(action: self.buttonTapped) {
  Text("My button")
}

IMHO, mixing eDSL closures indiscriminately with 'normal' closures, especially in trailing position) is bound to cause readability difficulties, regardless of what labels you apply at the call site.

1 Like

I see your point and the design flaw before SE-0279, yet it does not solve the ergonomics for chained calls.

Sure you could wrap and indent properly using init:

Button
  .init(action: self.buttonTapped) {
    Text("My button")
  }
  .modifier()

But who said that you should always be forced to off-load the action closure?
Even if you'd flip the parameters you'd still end up in a weird situation.

Button
  .init(action: { ... }) {
    Text("My button")
  }
  .modifier()

Button
  .init {
    ...
  } 
  label: {
    Text("My button")
  }
  .modifier()

Which brings me back to the optional first label:

Button
  action: {
    ...
  } 
  label: {
    Text("My button")
  }
  .modifier()

There is literally no improvement achieved by swapping the action and label parameters. This of course is my personal opinion in regard of my preferred call style.

2 Likes

I'm not arguing anyone should be forced to, but IMO if you have code that is interleaving imperative closures with eDSL closures, there's going to be readability issues that will not be addressed by nudging the indentation around. I.e., if the language design pushes users towards defining their 'action' closures out-of-line, I would consider that a feature rather than a bug.

If I understand correctly, your preferred form of the out-of-line version would actually be:

Button(action: self.buttonTapped) 
  label: {
    Text("My button")
  }
  .modifier()

?

I'm not sure I agree that that's better than:

Button(action: self.buttonTapped) {
  Text("My button")
}
.modifier()
1 Like

I would prefer the former example yes. If it fits my line width, which is 80, then I'd use that style. The latter example is totally busted in terms of readability and proper code indentation.

So I would pick one the following styles:

Button(action: self.buttonTapped) 
  label: {
    Text("My button")
  }
  .modifier()

// this one is strange though.
Button
  .init(action: self.buttonTapped) 
  label: {
    Text("My button")
  }
  .modifier()

Button
  .init(action: self.buttonTapped) {
    Text("My button")
  }
  .modifier()

// preferred because of the clarity provided
// by the labels
Button
  .init(
    action: self.buttonTapped,
    label: {
      Text("My button")
    }
  ) 
  .modifier()

Do we have an estimate about when this could be moved into the release process again ?

5 Likes

The support of an optional label for the first trailing closure would make the "incorrect argument label" fix-its more consistent and simple. As an example:

func filter(by: () -> (), until: () -> ()) {}

// error: Incorrect argument labels in call (have '_:by:', expected 'by:until:')
// fix-it: Replace '{} by' with 'by: {} until'
filter {} by: {}

The proposed fix-it suggests

filter by: {} until: {}

which would be valid under this proposed pitch.