New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule Request: Multiple Closure Arguments with Trailing Closure #1801

Closed
erikstrottmann opened this Issue Aug 26, 2017 · 3 comments

Comments

Projects
None yet
5 participants
@erikstrottmann
Contributor

erikstrottmann commented Aug 26, 2017

New Issue Checklist

Rule Request

In many cases, the trailing closure syntax can make code clearer by reducing boilerplate. When passing a single closure argument to a function, that closure’s role is often clear even without its parameter label. When passing two closure arguments, however, the parameter labels crucially distinguish the role of the two closures. With trailing closure syntax, it becomes less clear that the second argument to UIView.animate is a completion handler:

UIView.animate(withDuration: 1.0, animations: {
    someView.alpha = 0.0
}) { _ in
    someView.removeFromSuperview()
}

I propose a rule that would warn when the trailing closure syntax is used when passing two closure arguments to a function. The principle behind this rule is supported by the Ray Wenderlich Swift style guide and Erica Sadun’s blog post “Swift: The Problem with Trailing Closure Syntax”. SwiftLint’s existing Trailing Closure Rule already makes an exception when passing multiple closure arguments to a function (foo.something(param1: { $0 }, param2: { $0 + 1 }) is a non-triggering example for that rule).

The rule’s severity should be configurable to warning or error, defaulting to warning; there are no other obvious configurable parameters. The rule’s kind should be style. The rule should be enabled by default, because this style guideline has community support and generally improves code readability.

Additional examples of how this rule should behave:

Non-triggering examples:

foo.map { $0 + 1 }
foo.reduce(0) { $0 + $1 }
if let foo = bar.map({ $0 + 1 }) {

}
foo.something(param1: { $0 }, param2: { $0 + 1 })
UIView.animate(withDuration: 1.0) {
    someView.alpha = 0.0
}

Triggering examples:

foo.something(param1: { $0 }) { $0 + 1 }
UIView.animate(withDuration: 1.0, animations: {
    someView.alpha = 0.0
}) { _ in
    someView.removeFromSuperview()
}

erikstrottmann added a commit to erikstrottmann/SwiftLint that referenced this issue Aug 27, 2017

Add `multiple_closures_with_trailing_closure` rule
Multiple Closures with Trailing Closure rule disallows trailing closure
syntax when passing more than one closure argument to a function.

Fixes realm#1801.

erikstrottmann added a commit to erikstrottmann/SwiftLint that referenced this issue Aug 28, 2017

Add `multiple_closures_with_trailing_closure` rule
Multiple Closures with Trailing Closure rule disallows trailing closure
syntax when passing more than one closure argument to a function.

Fixes realm#1801.

erikstrottmann added a commit to erikstrottmann/SwiftLint that referenced this issue Aug 28, 2017

Add `multiple_closures_with_trailing_closure` rule
Multiple Closures with Trailing Closure rule disallows trailing closure
syntax when passing more than one closure argument to a function.

Fixes realm#1801.
@madordie

This comment has been minimized.

Show comment
Hide comment
@madordie

madordie Sep 18, 2017

This will conflict with the Xcode default style and must be manually added to the last parameter.
😂

madordie commented Sep 18, 2017

This will conflict with the Xcode default style and must be manually added to the last parameter.
😂

@tadija

This comment has been minimized.

Show comment
Hide comment
@tadija

tadija Apr 23, 2018

For anyone searching for a non-triggering example of the animation with a completion:

    UIView.animate(withDuration: 1.0, animations: {
        self.view.alpha = 0.0
    }, completion: { _ in
        self.view.removeFromSuperview()
    })

Although as stated in the previous comment, this doesn't play nice with Xcode.

tadija commented Apr 23, 2018

For anyone searching for a non-triggering example of the animation with a completion:

    UIView.animate(withDuration: 1.0, animations: {
        self.view.alpha = 0.0
    }, completion: { _ in
        self.view.removeFromSuperview()
    })

Although as stated in the previous comment, this doesn't play nice with Xcode.

@ivampir

This comment has been minimized.

Show comment
Hide comment
@ivampir

ivampir commented Jul 25, 2018

me too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment