Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
I strongly agree with Ben. This is a style choice and highly context-dependent. We should not warn about it or produce a fix-it unless specifically requested, and we don’t have any place for such requests today.
(Motivation: Another case where I prefer not using a trailing closure is when there are other closure arguments in the call.)
Jordan
···
On Jul 5, 2016, at 17:19, Ben Langmuir via swift-dev <swift-dev@swift.org> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
Probably, the warning/fixit should be opt-out/in, like an attribute @trailing_closure_prefered on the function decl, thoughts?
Another benefit of adding such attribute is to govern code completion UI the way to expand a closure placeholder. Currently, we expand
closure unequivocally to trailing closures, even for lexicographicallyPrecedes(isOrderedBefore:)
···
On Jul 5, 2016, at 5:39 PM, Xi Ge via swift-dev <swift-dev@swift.org> wrote:
On Jul 5, 2016, at 5:19 PM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
I strongly agree with Ben. This is a style choice and highly context-dependent. We should not warn about it or produce a fix-it unless specifically requested, and we don’t have any place for such requests today.
(Motivation: Another case where I prefer not using a trailing closure is when there are other closure arguments in the call.)
I agree with you if by “context-sensitive”, you mean “function-signature-sensitive”; and we can solve that by adding a new attribute on function decls to categorize a function into either “trailing closure preferred” or “inline closure preferred”.
“lexicographicallyPrecedes” is a good example of the later while "DispatchQueue.async(execute:)” is of the former. Any other contexts in your mind?
Xi
···
On Jul 5, 2016, at 8:31 PM, Jordan Rose <jordan_rose@apple.com> wrote:
On Jul 5, 2016, at 17:19, Ben Langmuir via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
I prefer trailing closures for -> Void signatures and in-line closures for anything (notably sequences and collections) that is likely to be iterated through or chained functionally. I also prefer inline closures for items that have multiple states for callback (completion handlers, error handlers, etc, where there is going to be a test of some kind -- we don't have a Result type but if we did, it would fall here -- and contains error/value pairs) and trailing closures for no-state-will-execute such as GCD.
-- E
···
On Jul 5, 2016, at 7:42 PM, Xi Ge via swift-dev <swift-dev@swift.org> wrote:
On Jul 5, 2016, at 5:39 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 5:19 PM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
For once I don’t think that this is something that makes sense for the API author to control. It should always be legal to replace a named closure with an anonymous closure without any other changes.
Jordan
···
On Jul 5, 2016, at 21:29, Xi Ge <xi_ge@apple.com> wrote:
On Jul 5, 2016, at 8:31 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:
On Jul 5, 2016, at 17:19, Ben Langmuir via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
I strongly agree with Ben. This is a style choice and highly context-dependent. We should not warn about it or produce a fix-it unless specifically requested, and we don’t have any place for such requests today.
(Motivation: Another case where I prefer not using a trailing closure is when there are other closure arguments in the call.)
I agree with you if by “context-sensitive”, you mean “function-signature-sensitive”; and we can solve that by adding a new attribute on function decls to categorize a function into either “trailing closure preferred” or “inline closure preferred”.
“lexicographicallyPrecedes” is a good example of the later while "DispatchQueue.async(execute:)” is of the former. Any other contexts in your mind?
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
I prefer trailing closures for -> Void signatures and in-line closures for anything (notably sequences and collections) that is likely to be iterated through or chained functionally.
I’m not sure if this matches your personal preferences, but your example made me realize the decision can also easily be context-dependent. I would probably write a single map with a trailing closure,
I also prefer inline closures for items that have multiple states for callback (completion handlers, error handlers, etc, where there is going to be a test of some kind -- we don't have a Result type but if we did, it would fall here -- and contains error/value pairs) and trailing closures for no-state-will-execute such as GCD.
And these callbacks are a place I really like to use trailing closures, which reinforces to me that we shouldn’t try to force one style or the other.
···
On Jul 5, 2016, at 7:57 PM, Erica Sadun via swift-dev <swift-dev@swift.org> wrote:
On Jul 5, 2016, at 7:42 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 5:39 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 5:19 PM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
I strongly agree with Ben. This is a style choice and highly context-dependent. We should not warn about it or produce a fix-it unless specifically requested, and we don’t have any place for such requests today.
(Motivation: Another case where I prefer not using a trailing closure is when there are other closure arguments in the call.)
I agree with you if by “context-sensitive”, you mean “function-signature-sensitive”; and we can solve that by adding a new attribute on function decls to categorize a function into either “trailing closure preferred” or “inline closure preferred”.
“lexicographicallyPrecedes” is a good example of the later while "DispatchQueue.async(execute:)” is of the former. Any other contexts in your mind?
For once I don’t think that this is something that makes sense for the API author to control. It should always be legal to replace a named closure with an anonymous closure without any other changes.
Isn’t trailing closure more amenable to completion blocks/callbacks than predicates like “isOrderedBefore”. Aren’t API authors the best people to tell the role of the closure?
Xi
···
On Jul 6, 2016, at 9:39 AM, Jordan Rose <jordan_rose@apple.com> wrote:
On Jul 5, 2016, at 21:29, Xi Ge <xi_ge@apple.com <mailto:xi_ge@apple.com>> wrote:
On Jul 5, 2016, at 8:31 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:
On Jul 5, 2016, at 17:19, Ben Langmuir via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
I don't think that's a rule that's universally agreed on. I use trailing closures for pretty much everything except when there are multiple closure arguments. Erica and others use them for imperative code only. Some people don't use them at all, maybe because they want to preserve the argument label.
We could try to enforce a universal style here, but that's a direction we need to consciously decide to go in; it's not just clean-up of our existing model. Additionally, we'd then have to come up with an importer rule for deciding which closures are trailing in imported APIs, and allow API owners to control that.
I do see that it's not out of line for API authors to be able to control this, but I think we should actually design such a thing properly, and we can do that at any time as long as it's just warnings or a separate style checker.
Jordan
···
On Jul 6, 2016, at 10:31, Xi Ge <xi_ge@apple.com> wrote:
On Jul 6, 2016, at 9:39 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:
On Jul 5, 2016, at 21:29, Xi Ge <xi_ge@apple.com <mailto:xi_ge@apple.com>> wrote:
On Jul 5, 2016, at 8:31 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:
On Jul 5, 2016, at 17:19, Ben Langmuir via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
On Jul 5, 2016, at 4:34 PM, Xi Ge via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
Hi Swift-devs,
I have tried to add a fixit to help developers using trailing closures more, motivated by my observation during WWDC that some developers
do not even realize that we have such a feature. In my opinion, trailing closures are more concise, and once you get used to it, more readable;
therefore users should adopt trailing closures whenever doing so introduces no ambiguity.
Fixits can enhance the discoverability of trailing closures by identifying misuses and by transforming users’ code automatically. However, adding the fixit introduces new issues:
Issue 1: The fixit has to be associated with a warning. Adding the warning means we declare wars against convertible non-trailing closures, which is a valid syntax choice by users.
Issue 2: Ambiguity checking should be exhaustive. We have several known situations when non-trailing closures cannot be convert to trailing closures, including:
Trailing closures are followed by other brackets, e.g., “if foo({}) {}” cannot be converted to “if foo {} {}”.
Removing the label of the last closure causes ambiguous function references, e.g. “foo(v: {})” cannot be converted to “foo {}” when “foo(v1: {})” also exists.
So Swift-devs, is the warning worth adding? If yes, are there other situations of ambiguity that are not covered?
Thanks for your feedback!
Xi
-1, this feels like a really good tool to have in a code-linter, but not something that we should put in the compiler and prescribe to all our users.
My biggest concern is that it’s not always obvious what the closure is used for. The argument label can help with this:
I strongly agree with Ben. This is a style choice and highly context-dependent. We should not warn about it or produce a fix-it unless specifically requested, and we don’t have any place for such requests today.
(Motivation: Another case where I prefer not using a trailing closure is when there are other closure arguments in the call.)
I agree with you if by “context-sensitive”, you mean “function-signature-sensitive”; and we can solve that by adding a new attribute on function decls to categorize a function into either “trailing closure preferred” or “inline closure preferred”.
“lexicographicallyPrecedes” is a good example of the later while "DispatchQueue.async(execute:)” is of the former. Any other contexts in your mind?
For once I don’t think that this is something that makes sense for the API author to control. It should always be legal to replace a named closure with an anonymous closure without any other changes.
Isn’t trailing closure more amenable to completion blocks/callbacks than predicates like “isOrderedBefore”. Aren’t API authors the best people to tell the role of the closure?