Renaming Trailing-closure Functions in the Standard Library

I'll agree with you in part: @explicit is a patchy solution. If it was just me, I'd simply ban the trailing closure syntax when the last argument has a label. This actually makes the language simpler:

  • API designers don't have to ask themselves if the function is readable both when the parameter label is present and when it is omitted. If they say it has a label, it always has a label.
  • Call sites cannot create ambiguities by omitting labels. Function calls are spelled as the author of the function intended, always with the same label everywhere.

Forcing this everywhere would be a major headache however**. So @explicit is just an opt-in system to this world with less ambiguities. It should be used sparingly when the ambiguity actually causes problems.

Unlike a rename, there is also no ABI impact in adding the attribute to a function. It does not break source compatibility either if the effect is only a warning + a fixit at the call site. So whenever you discover a similar problem with other functions, it's an easy fix.

And if SE-0279 is accepted, this problem is bound to come up more often. Better be prepared.

** The problem if we were to make labels explicit as the default is that it'd create so many warnings to fix that nobody would want to put up with them. So not really source breaking because everything would still compile fine, but it'd be quite annoying to deal with. It also changes what people are used to.

7 Likes

If you think the current SE-0279 proposal can work on its own but you’d prefer it to go further in certain ways, that’s very interesting feedback. Likewise if you think it’s not a good idea unless it does go further. It’s not an up/down vote; you don’t have to try to distill your conclusions down to a single bit.

3 Likes

This is a topic I'm biased against. I always believed (and still do) that trailing closures should've been only an opt-in feature, for making some specific functions (e.g. control flow-like ones) read more naturally. Instead it's present everywhere, and since it is people tend to use it, and then the issues occurring due to it arise.
I believe that function labels are meant to have semantic meaning, and I see allowing for that to be removed as counter-productive.

The main question for me is: why do we have trailing closures at all? What reads different between foo { ... } and foo({ ... }) (apart from the lack of ())? Imho this distinction should have some semantic difference, but the fact that it can only be known by the designer of the function while the trailing closure is allowed everywhere kills this option.

Trailing closure syntax can also be interpreted as one of the various "simplification" sugars available (like omitting the signature), but imho it's not the same: it actively moves the closure from inside to outside the "domain" of the function call. This kind of movement will necessarily cause a difference in perception to the reader, even though by now we're trained to read it as equivalent due to it not being a selective thing.

Personally I believe it could still be made opt-in, with something like @trailing in the parameter, as long as a compiler option for ignoring it gets made (e.g. always_assume_trailing). In this case I wouldn't have strong thoughts about having to write or not the associated label anyway, as it would be in control of the designer of the function. Though I find the option of having label-prefixed trailing closures interesting (in the context where I find trailing closures appropriate).

But said all that, I understand that people seem to like trailing closures in general, and I mostly gave up on my interpretation of it :sweat_smile: though I always liked putting semantic information in the argument labels and this renaming seems to go against it :(

10 Likes

This thread is not the place to provide feedback on SE-0279. I appreciate the point that it’s hard to respond to Ben without interrogating the language design at all, but still, please try to recognize when your post is primarily a response to SE-0279 and say those points in the review thread. You can then build on that in here (by linking to it) to make a more direct point about renaming APIs.

Thanks for the answer, but I don't really agree: SE-0279 is primarily for multiple trailing closures, even though of course it has implications on the case of a single trailing closure. But I posted in this thread completely ignoring the multiple trailing closures case, and only considering the single one.
I realise that the proposal is for renaming of functions, so the trailing closure feature itself might seem OT, but since it's really "renaming because of trailing closures" I believe that a longer form of "I wouldn't want a rename because I think it's trailing closure's that it's at fault" is correctly posted here.

If you still think that it is fundamentally in the wrong place I can move it :+1:

(Edit: I appreciate that this whole discussion is getting hard to moderate)

3 Likes

My post wasn't meant exclusively as a response to you, sorry; it was something I'd meant to post as follow-up to an earlier post.

2 Likes

I find this mostly to be a straw man argument.

// A this reads well
array.first(where: { criteria })
// B but this is not at all clear
array.first { criteria }
// C Meh
array.firstWhere { criteria }

IMO A is better than B and A is better than C. Stick with A. You want to force me to use C because you say it's better than B. Just don't prevent me from using A.

C looks like Objective-C, which we all want to leave in our rear-view. If some people want to use B rather than A that's fine with me. Just not in my projects. This is a job for the linter.

I don't see this as a matter of safety or clarity. There are plenty of ways people can shoot themselves in the foot.

I personally use this design style in plenty of my own method names and I like it.

func myObject(at indexPath: IndexPath) -> MyObject?
func name(for offset: Int) -> String

and similar. If you change the API Guidelines then this good style will be replaced by this new (IMO Bad) style in general.

-1

19 Likes

I definitely understand the rationale for wanting to trailing closure read more naturally, but I don't support this proposal as it stands.

For one, I don't like the way it changes these functions when not using trailing closures.

arr.firstWhere({ $0 > 5 })

As has been mentioned previously, the Swift API guidelines strongly recommend moving the prepositional part ('where') into the first argument label. Not doing so makes this read awkwardly alongside more idiomatic Swift code. In making one use case better (trailing closures) at the expense of other use cases, I think the benefits of this change are largely negated.

Outside of simple style nits, the methods you're proposing to change are used quite heavily in most Swift codebases, so any changes to them have wide reaching effects. Is the effort involved in renaming all of these methods in every single Swift codebase going forward worth a slight increase in readability in certain use cases?


I think one under-discussed part of the issue here also has to do with the way Xcode aggressively inserts trailing closures wherever possible. I frequently run into cases where Xcode autocompletes some code to use a trailing closure, and then I have to manually revert that change for the sake of readability. For example if I'm in the middle of writing this Rx code:

If I then tab over to the argument placeholder for onError and press return, it expands to this:

Screen Shot 2020-04-21 at 2.56.02 pm

In this case Xcode has removed the argument label and made my code less readable in doing so.

I think a more pragmatic solution here might be to introduce annotations which can provide hints to text editors about when to use trailing closures, and where a label provides valuable context and shouldn't be removed.

7 Likes

-1

It makes the call site uglier when you aren’t using trailing closures. Better would be to adopt a trailing closure syntax that optionally keeps the argument label intact, which also allows for it to read the same way whether or not trailing closures are used but keeps the function name looking nice in the non-trailing closure scenario. (And has the bonus of not renaming functions and breaking source compatibility.)

6 Likes

As usual, I would strongly prefer a more conservative approach. For example, one or both of these:

  1. Fix the typechecker. Seriously, there is no good reason for this performance issue other than technical debt which needs to be paid off, and the longer we wait the more damage it causes
  2. Make hanging trailing closure argument labels optional, unless needed for disambiguation, in which case they become required.

IMO,

  • It helps to look at a complete real example. a.first { x in x < 0 } is perfectly clear to me as “the first negative element.” As you've often said, we can't expect every detail to be apparent without looking at documentation. As I've often said, it's most important that APIs are have recognizable meaning to those that know the library and don't encourage any easy misinterpretations for those who don't. Personally I don't see any other obvious ways to interpret that code and don't mind if some people have to look at the docs.
  • drop(while:) is obviously misnamed and should be dropFirst(while:) or maybe even better, dropPrefix(while:). We have "drop" slicing operations at both ends of a collection and this name simply fails to clarify which end is being dropped. Yes, to understand it in all its detail one needs to know that "dropXXX" is a contiguous slicing operation that goes with prefix and suffix, but I believe this does enough to address the issue without undertaking a large, disruptive renaming effort.
45 Likes

I agree with @dabrahams and could not have said it better.

3 Likes

making argument labels mandatory at the call site for trailing closures seems completely unacceptable to many people.

Isn't this what this proposal is about? Only instead of making it mandatory at the label, it is making it mandatory at the function name?

2 Likes

OK, I know that this is different because it is only affecting some standard library functions, but the motivation for this proposal serves for other APIs as well. It is part of a bigger issue that was already stated.

The decision of allowing trailing closures or not should be in the hands of the API designer!

One has complete control over function naming, but not complete control of the call site syntax because of the lack of control over trailing closure use. This is the real problem, I reckon. Instead of working around the problem by changing the function name, the language should empower the API designer. IMO, API designers should have complete control of the call site. I'm not sure if trailing closure is the only feature that would affect this principle though, but if there is anything else, we should look into it as well.

1 Like

I agree with @allevato and @DevAndArtist's comments.
This doesn't seem like a step in the right direction. It feels like a workaround, and not a solution.

This seems to create Objecive-C like naming for some of these methods, or more specifically how Swift APIs are bridged to Obj-C today.

15 Likes

-1
This is a step back towards objective c style function names. Moreover, if proposal SE-0279 does not end well and eventually we have changes in relation to the behaviour of the label of the first closure the damage in renaming functions would already be done. We should at least expect what will happen with multiple trailing closures in the future.

16 Likes

I cannot emphasize more than many of done it that it would be fatal to change the way the standard library works nowadays. Also, SE-0279 would have made more sense if when there's more than one closure that labels are obliged for all. But I'm late to the game for that, and now I'll follow SEs more closely.

6 Likes

As many have already stated, the correct solution to this issue should be to make all function argument labels mandatory at the call site, not to rename functions in ways that don't follow the Swift API naming guidelines. Both of these methods end up breaking source compatibility but the former doesn't break the naming guidelines.

You say that "making argument labels mandatory at the call site for trailing closures seems completely unacceptable to many people." But by renaming these functions, we'd be doing exactly that - except the label is now part of the function name instead of what it should be, a label. If the reason people feel that this is unacceptable is because some argument labels don't actually add meaning to that function call, then that label should just be omitted in the definition of the API.

You mention that, among the functions in the standard library, "not all argument labels are necessary for readability". These are the functions that should be renamed - to omit these unnecessary argument labels. This combined with a change to require all argument labels at the call site has the same effect as the proposed change - source compatibility breaks for functions where the closure argument label is required to provide meaning to the closure, and functions that currently have labels that don't provide meaning remain being able to be called without the label - without going against the naming guidelines.

I'll end with one more comment about a hypothetical function named first(_ number: Int, where predicate: (Element) throws -> Bool) rethrows -> [Element]. The proposed renaming doesn't work with this method. firstWhere(numberOfElementsToFind) { criteria } does not read well at all. Unless it becomes required to include the argument label for the closure at the call site, that meaningful piece of information will be, as you say, "left off 90% of the time", and there's no meaningful way to rename the function to prevent that.

8 Likes

When I see firstWhere I think JS, Dart, Kolin. What I always loved about Swift is the ability to use the same name for a family of functions, differentiated by their labels. Maybe this is not a strong argument, but first(where:) is what makes Swift Swift.

I agree that dropping the label in the given examples is ambiguous, but how about we solve that by doing the right thing, instead of going with the simple workaround.

Federighi said that Swift is the language we will use for the next 20 years, which is underestimated in my opinion, so how about we fix early mistakes now while Swift is still relatively young, instead of forever satisfying with mediocre solutions.

Arguments where and while are not unlabeled arguments so having a language feature that drops those labels seems to be a mistake in the first place. Are there no steps that we can make to fix this mistake in the long term?

30 Likes

This seems more like a regression to the days of Swift 2 and 3 than an improvement to me. It makes the distinction between function and parameter blurrier, and makes overloading patterns messy.

1 Like

drop(while:) is a poor example to argue over because it's not a particularly good name to begin with. It really ought to be dropPrefix(where:) (label to be discussed), which both removes any doubt about which elements are to be dropped and allows the same drop term to be used on the equally useful dropSuffix(where:). I don't think this is a good area to be inventing jargon-y assumptions like "of course drop means a prefix".

7 Likes