Closure end indentation rule

[marcelofabri]

Fixes #326

This still have some issues:

• It should be opt-in. It's not currently to make it easier to test.
• Some false positives are being trigged, mainly related to opening brace on different line than the method call. For example:

return file.matchPattern(pattern, excludingSyntaxKinds: kinds,
                                          excludingPattern: excludingPattern).map {
    return StyleViolation(ruleDescription: type(of: self).description,
                          severity: configuration.severity,
                          location: Location(file: file, characterOffset: $0.location))
}

• It should check (and skip) if the closure begins and ends on the same line, not the body offset. This should prevent this false violation:

 return file.matchPattern(pattern, excludingSyntaxKinds: kinds,
                                 excludingPattern: excludingPattern) { ↓$0.rangeAt(1) }.flatMap {
            let parensRegex = NSRegularExpression.forcePattern(parensPattern)
            return parensRegex.firstMatch(in: file.contents, options: [], range: $0)?.range
}

The main issue here is that I'd need the starting point of the method related to the closure, not the full chained one (which is what SourceKits gives me). Any ideas on how to get it?

[jpsim]

I'm not sure I stylistically agree with the current semantics (the style changes you've already made). Would need to write down more explicit rules just in sentences/point-form explaining when to place closing braces and where.

[marcelofabri]

So, my interpretation of the comments in #326 was:

• If the method call is chained breaking lines on each method (e.g. . is on a new line), the closing brace should be in the same indentation as the ..
• Else, the closing brace should be in the same indentation as the beginning of the statement (in the first line).

[jpsim]

Yeah, that sounds good. It was hurting my head a bit to try to think of when you'd apply which rules... this is why we need machines to decide this for us! 😅

[marcelofabri]

🤖 > 👨‍💻

[jpsim]

🤖 > 👨‍💻

Is it too late to rename SwiftLint to this? 😂

But seriously, this is the exact opposite of the point I was trying to make in my SwiftLint presentation from last week (slides), that you shouldn't bend to the will of the machines and humans know better...

But there are times when the machines pay closer attention to details than we can.

[marcelofabri]

Maybe the issue is that it's harder to come up with logic rules to describe what "feels right". Our intuition is much more complex than I thought! I've learned the hard way when developing #847.

[jpsim]

Our intuition is much more complex than I thought

This. Though it is definable.

[codecov-io]

Current coverage is 82.28% (diff: 96.87%)

Merging #1004 into master will increase coverage by 0.12%

@@             master      #1004   diff @@
==========================================
  Files           137        138     +1   
  Lines          6530       6587    +57   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5365       5420    +55   
- Misses         1165       1167     +2   
  Partials          0          0          

Powered by Codecov. Last update 7abbd84...ed5fdd8

[jpsim]

🎉 thanks Marcelo!