Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager. When emailing the review manager directly, please keep the proposal link at the top of the message.
Trying it out
To try this feature out, add a dependency to the task-local-test-trait branch of the pointfreeco fork of swift-testing to your package:
Finally, import Swift Testing using @_spi(Experimental) import Testing.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.
When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift Testing?
Does this proposal fit well with the feel and direction of Swift Testing?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at
I think that we do need a label on one argument or the other (probably the second) to make it easier to read/write it without needing to refer back to the documentation.
We are open to adding an label to the second argument, but do want to point out that there is quite a bit precedence in the Swift ecosystem for methods that take two unlabeled arguments where the second argument "binds" to the first:
Personally, I feel that a label here would add more ceremony than clarity. Consider that the $ already does a decent job of semantically[1] disambiguating the two arguments.
it's often associated with some "meta" representation of a concrete value ↩︎
Kind of disagree, not sure it is a reason to deviate from the API guidelines. Not sure why SwiftUI DSL was allowed to to be honest, but that is a whole other topic hehe :).
I kind of see this as a constraint of the language, that we can't use the = operator for an indirect assignment like this one. In general Swift does like parameter labels though, and while the words "with value" don't add a lot of semantic meaning, they do a) make clear which argument is the value, and b) rhyme with the existing taskLocal.withValue() {} API.
Note that TaskLocal instances are not always referred to with a leading dollar sign, and it's also possible for a task local to refer to another task local (thus resulting in both arguments having leading dollar signs). These are edge cases, but real ones.
If we were more adventurous, we could overload some operator like <= (the COMEFROM operator, of course) and write:
@Test(
.taskLocal($logger <= .inMemory)
)
I don't think this is a good idea, but for the sake of completeness, I mention it.
I personally don’t mind the lack of a label and like the current spelling, but for the sake of discussion what about .withTaskLocal(_: as:)? Or just .taskLocal(_: as:).
I wouldn't support "as" because, in Swift, "as" is associated with casting, which isn't what we're doing here; we're binding a value, not changing it to some other type.
I was mulling over other spellings the other day and wondered about as but for the same reason thought it wasn’t right.
The other one I pondered was to but felt like it wasn’t markedly better than withValue but it would look like this:
@Test(
.taskLocal($featureEnabled, to: true),
.taskLocal($theme, to: .minimal),
.taskLocal($logger, to: .inMemory),
.taskLocal($analytics, to: .inMemory)
)
For what it’s worth, I don’t strongly feel a label is needed but marginally prefer there being one.
While I started out mostly ambivalent to a label, the more I exercised it in practice the less necessary it seems, and I'm having a hard time making an argument for it outside a very strict, localized reading of the Argument Labels section of the API Design Guidelines. In isolation it does seem like everything falls through to:
Label all other arguments.
…but even functions in the Standard Library fail this test.
assert(_:_:)
reduce(_:_:)
zip(_:_:)
If you're willing to zoom out to the rest of the API Design Guidelines you might say that these are terms of art of sorts, but then I think we should also consider this guideline:
Omit needless words. Every word in a name should convey salient information at the use site.
This could justify not only SwiftUI's use of environment(_:_:), but also SwiftSyntax's use of with(_:_:), both of which describe a binding–value relationship. I can't really think of a label that adds information to the call site: the call site is a binding to some Value and then the Value itself. The first argument even kind of acts as a label to the second:
.environment(\.colorScheme, .light)
// vs.
.environment(colorScheme: .light)
.with(\.trailingTrivia, .space)
// vs.
.with(trailingTrivia: .space)
.taskLocal($isEnabled, true)
// vs.
.taskLocal(isEnabled: true)
Omit needless words is unfortunately not a very precise guideline. Because what may be 'needless' to one, might not be to another.
You provide a few examples from the standard library. I would like to note that for zip(_:_:) the sequence of collections passed in mostly does not matter. And for reduce(_:_:) I am that guy that keeps looking up what the sequence of arguments was again: accumulator first or initial result first? assert(_:_:) is probably the closest example and there too I check the autocomplete template to see if I am using it right.
So I wouldn't say the labels are needed, but they are very helpful to me. I would therefore strongly encourage labelling at least one of the arguments.
If you mean they don't matter in the same way as this guideline:
Omit all labels when arguments can’t be usefully distinguished, e.g. min(number1, number2), zip(sequence1, sequence2).
Don't they matter in the same ways? The order manifests in the types of the zipped collections. And arguably this same argument could apply to .environment/.with/.taskLocal, where the context provides enough information as is.
I guess this is the rub. Both reduce and assert made it through the API guidelines audit, and you do still have this autocomplete template guiding you. Would the autocomplete template of .taskLocal(<#TaskLocal<Value>#>, <#Value#>) be enough to guide you towards the correct usage? If you want to argue against that, would you want reduce and assert to be revised?
I'd say they are not because for .taskLocal sequence matters: the position in the argument list determines what it means, whereas for symmetric functions like min the sequence doesn't matter. Here again the API guidelines are not really specific as the zip case already demonstrates: zip(a, b) is distinct from zip(b, a). But whether that constitutes "usefully distinguished" sounds to me like it's up to interpretation or taste.
It would, but at higher friction cost than just typing out the labels. Note that the autocomplete template has taskLocal in it twice, has Value in it twice, has multiple # signs in it. This takes time in my brain to interpret.
I'd say we should learn from the past, not repeat it.
I'm not sure I understand, because for zip sequence also matters?
We are open to a label, but in practice I haven't found them to be less cumbersome than omission, and I haven't come up with a counterexample myself. Can you make a case for a .taskLocal test trait argument label that adds clarity, especially when chained?
I do concede that $logger, withValue: .inMemory is kinda weird, because .inMemory reads more like a kind of logger, not a value like true, 4 or "Hello!". But at least it's explicit about 'what holds a thing' (the TaskLocal itself) and 'the thing that's being held' (the value).
I'll jump in again and express support for argument labels (I still like withValue:).
I think that showing other examples that fail the API Design Guidelines is not a particularly strong argument for continuing to disregard the Design Guidelines. For historical reasons, they don't comply, and post-swift-3, no one has the appetite to force a migration to something compliant with the Design Guidelines. I think ignoring the API Design Guidelines without a very strong reason to do so is a mistake.
Especially in the current world we live in, the extra verbosity is very cheap (the main cost is our humanity). And even 4 years ago, that extra verbosity was only a few extra characters being typed. Code is read way more often than it is written, and the reason argument labels exist is to facilitate reading code.