Add multiline closure argument rule

[keith]

This rule forces users to name arguments for multiline closures.

This is the first draft of #70

See the first commit for the rule. The second is just fixes for it.

[segiddins]

if at all possible, it might be preferable to make this single statement closures instead of single-line

[jpsim]

Lol, of course the rule would violate itself:

/Users/travis/build/realm/SwiftLint/Source/SwiftLintFramework/Rules/MultilineClosureArgumentRule.swift:14:32: error: Force Try Violation: Force tries should be avoided. (force_try)
/Users/travis/build/realm/SwiftLint/Source/SwiftLintFramework/Rules/MultilineClosureArgumentRule.swift:25:1: warning: Multi-line closure argument Violation: Multi-line closures should not use $0 (closure_argument)
/Users/travis/build/realm/SwiftLint/Source/SwiftLintFramework/Rules/MultilineClosureArgumentRule.swift:32:1: warning: Multi-line closure argument Violation: Multi-line closures should not use $0 (closure_argument)
/Users/travis/build/realm/SwiftLint/Source/SwiftLintFramework/Rules/MultilineClosureArgumentRule.swift:41:1: warning: Multi-line closure argument Violation: Multi-line closures should not use $0 (closure_argument)
/Users/travis/build/realm/SwiftLint/Source/SwiftLintFramework/Rules/MultilineClosureArgumentRule.swift:42:1: warning: Multi-line closure argument Violation: Multi-line closures should not use $0 (closure_argument)

[keith]

Yea I can disable those if we want to move forward with this. And fix the description.

[jpsim]

I think we have a difference of stylistic opinion regarding when it's appropriate to use $0.

I'd be much more inclined to say it's for single statements like @segiddins suggested rather than requiring the closure's braces to be on the same line.

I actually consider all of SwiftLint's current uses of the dollar sign shorthand to be appropriate in that sense.

Unfortunately, I have no idea how to detect statements.

[jpsim]

By the way, it'd be trivial to update this rule to only apply in the appropriate context:

-    private static let regex = try! NSRegularExpression(pattern:
-        "(^[^\\{\\n]*\\$0|\\$0[^\\}\\n]*$)", options: .AnchorsMatchLines)
-
     public func validateFile(file: File) -> [StyleViolation] {
-        let range = NSRange(location: 0, length: file.contents.utf16.count)
-        let matches = MultilineClosureArgumentRule.regex.matchesInString(file.contents,
-            options: [], range: range)
-
-        return matches.map { match in
-            return StyleViolation(ruleDescription: self.dynamicType.description,
-                location: Location(file: file, offset: match.range.location))
+        let pattern = "(^[^\\{\\n]*(\\$0)|(\\$0)[^\\}\\n]*$)"
+        return file.matchPattern(pattern, withSyntaxKinds: [.Identifier]).map { match in
+            StyleViolation(ruleDescription: self.dynamicType.description,
+                           location: Location(file: file, offset: match.location))
         }
     }

[jpsim]

@keith would you like to revive this now that we have opt-in rules?

[keith]

Sure. I'll try and get to the feedback shortly.

[jpsim]

FWIW I wouldn't enable it for this project.

[scottrhoyt]

@keith do you have any updates here? Or should we close this PR out? Thanks.

[keith]

I'll reopen this when I have time to make the fixes!

[jpsim]

I'll actually take a stab at reviving this if you don't mind @keith. I think it's a great rule to have, even on by default, but keeping the default number of lines a bit higher (4-5?).

[keith]

Have at it! I think that a single line is a nice rule, but maybe that could be customizable?

[jpsim]

Have at it! I think that a single line is a nice rule, but maybe that could be customizable?

Yeah, if this goes in, it should certainly be customizable.

[jpsim]

Hmm, on second thought, I don't think I can do this as easily as I had expected... There's no SwiftDeclarationKind for closures, so they're a bit hard to detect.