Skip to content

Commit

Permalink
Merge pull request #2132 from erichoracek/eh-close-end-indentation-pa…
Browse files Browse the repository at this point in the history 
…rams

Include named closure parameters in closure end indentation rule
  • Loading branch information
@jpsim
jpsim authored May 6, 2018
2 parents d4b1aa6 + 26c36d4 commit 9a340a2
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 31 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#2127](https://github.com/realm/SwiftLint/issues/2127)

* Fixes a case where the `closure_end_indentation` rule wouldn't lint the end
indentation of non-trailing closure parameters.
[Eric Horacek](https://github.com/erichoracek)
[#2121](https://github.com/realm/SwiftLint/issues/2121)]

* Add `redundant_set_access_control` rule to warn against using redundant
setter ACLs on variable declarations.
[Marcelo Fabri](https://github.com/marcelofabri)
Expand Down
36 changes: 36 additions & 0 deletions Rules.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,32 @@ foo(abc, 123)

```

```swift
function(
closure: { x in
print(x)
},
anotherClosure: { y in
print(y)
})
```

```swift
function(parameter: param,
closure: { x in
print(x)
})
```

```swift
function(parameter: param, closure: { x in
print(x)
},
anotherClosure: { y in
print(y)
})
```

</details>
<details>
<summary>Triggering Examples</summary>
Expand All @@ -827,6 +853,16 @@ return match(pattern: pattern, with: [.comment]).flatMap { range in

```

```swift
function(
closure: { x in
print(x)
↓},
anotherClosure: { y in
print(y)
↓})
```

</details>


Expand Down
201 changes: 170 additions & 31 deletions Source/SwiftLintFramework/Rules/ClosureEndIndentationRule.swift
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,64 @@
import Foundation
import SourceKittenFramework

internal struct ClosureEndIndentationRuleExamples {

static let nonTriggeringExamples = [
"SignalProducer(values: [1, 2, 3])\n" +
" .startWithNext { number in\n" +
" print(number)\n" +
" }\n",
"[1, 2].map { $0 + 1 }\n",
"return match(pattern: pattern, with: [.comment]).flatMap { range in\n" +
" return Command(string: contents, range: range)\n" +
"}.flatMap { command in\n" +
" return command.expand()\n" +
"}\n",
"foo(foo: bar,\n" +
" options: baz) { _ in }\n",
"someReallyLongProperty.chainingWithAnotherProperty\n" +
" .foo { _ in }",
"foo(abc, 123)\n" +
"{ _ in }\n",
"function(\n" +
" closure: { x in\n" +
" print(x)\n" +
" },\n" +
" anotherClosure: { y in\n" +
" print(y)\n" +
" })",
"function(parameter: param,\n" +
" closure: { x in\n" +
" print(x)\n" +
"})",
"function(parameter: param, closure: { x in\n" +
" print(x)\n" +
" },\n" +
" anotherClosure: { y in\n" +
" print(y)\n" +
" })"
]

static let triggeringExamples = [
"SignalProducer(values: [1, 2, 3])\n" +
" .startWithNext { number in\n" +
" print(number)\n" +
"↓}\n",
"return match(pattern: pattern, with: [.comment]).flatMap { range in\n" +
" return Command(string: contents, range: range)\n" +
" ↓}.flatMap { command in\n" +
" return command.expand()\n" +
"↓}\n",
"function(\n" +
" closure: { x in\n" +
" print(x)\n" +
"↓},\n" +
" anotherClosure: { y in\n" +
" print(y)\n" +
"↓})"
]
}

public struct ClosureEndIndentationRule: ASTRule, OptInRule, ConfigurationProviderRule {
public var configuration = SeverityConfiguration(.warning)

Expand All @@ -11,35 +69,8 @@ public struct ClosureEndIndentationRule: ASTRule, OptInRule, ConfigurationProvid
name: "Closure End Indentation",
description: "Closure end should have the same indentation as the line that started it.",
kind: .style,
nonTriggeringExamples: [
"SignalProducer(values: [1, 2, 3])\n" +
" .startWithNext { number in\n" +
" print(number)\n" +
" }\n",
"[1, 2].map { $0 + 1 }\n",
"return match(pattern: pattern, with: [.comment]).flatMap { range in\n" +
" return Command(string: contents, range: range)\n" +
"}.flatMap { command in\n" +
" return command.expand()\n" +
"}\n",
"foo(foo: bar,\n" +
" options: baz) { _ in }\n",
"someReallyLongProperty.chainingWithAnotherProperty\n" +
" .foo { _ in }",
"foo(abc, 123)\n" +
"{ _ in }\n"
],
triggeringExamples: [
"SignalProducer(values: [1, 2, 3])\n" +
" .startWithNext { number in\n" +
" print(number)\n" +
"↓}\n",
"return match(pattern: pattern, with: [.comment]).flatMap { range in\n" +
" return Command(string: contents, range: range)\n" +
" ↓}.flatMap { command in\n" +
" return command.expand()\n" +
"↓}\n"
]
nonTriggeringExamples: ClosureEndIndentationRuleExamples.nonTriggeringExamples,
triggeringExamples: ClosureEndIndentationRuleExamples.triggeringExamples
)

private static let notWhitespace = regex("[^\\s]")
Expand All @@ -50,6 +81,25 @@ public struct ClosureEndIndentationRule: ASTRule, OptInRule, ConfigurationProvid
return []
}

return validateArguments(in: file, dictionary: dictionary) +
validateCall(in: file, dictionary: dictionary)
}

private func hasTrailingClosure(in file: File,
dictionary: [String: SourceKitRepresentable]) -> Bool {
guard
let offset = dictionary.offset,
let length = dictionary.length,
let text = file.contents.bridge().substringWithByteRange(start: offset, length: length)
else {
return false
}

return !text.hasSuffix(")")
}

private func validateCall(in file: File,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
let contents = file.contents.bridge()
guard let offset = dictionary.offset,
let length = dictionary.length,
Expand Down Expand Up @@ -88,6 +138,65 @@ public struct ClosureEndIndentationRule: ASTRule, OptInRule, ConfigurationProvid
]
}

func validateArguments(in file: File,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
guard isFirstArgumentOnNewline(dictionary, file: file) else {
return []
}

var closureArguments = filterClosureArguments(dictionary.enclosedArguments, file: file)

if hasTrailingClosure(in: file, dictionary: dictionary), !closureArguments.isEmpty {
closureArguments.removeLast()
}

let argumentViolations = closureArguments.flatMap { dictionary in
return validateClosureArgument(in: file, dictionary: dictionary)
}

return argumentViolations
}

private func validateClosureArgument(in file: File,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
let contents = file.contents.bridge()
guard let offset = dictionary.offset,
let length = dictionary.length,
let bodyLength = dictionary.bodyLength,
let nameOffset = dictionary.nameOffset,
let nameLength = dictionary.nameLength,
bodyLength > 0,
case let endOffset = offset + length - 1,
contents.substringWithByteRange(start: endOffset, length: 1) == "}",
let startOffset = dictionary.offset,
let (startLine, _) = contents.lineAndCharacter(forByteOffset: startOffset),
let (endLine, endPosition) = contents.lineAndCharacter(forByteOffset: endOffset),
case let nameEndPosition = nameOffset + nameLength,
let (bodyOffsetLine, _) = contents.lineAndCharacter(forByteOffset: nameEndPosition),
startLine != endLine, bodyOffsetLine != endLine,
!isSingleLineClosure(dictionary: dictionary, endPosition: endOffset, file: file) else {
return []
}

let range = file.lines[startLine - 1].range
let regex = ClosureEndIndentationRule.notWhitespace
let actual = endPosition - 1
guard let match = regex.firstMatch(in: file.contents, options: [], range: range)?.range,
case let expected = match.location - range.location,
expected != actual else {
return []
}

let reason = "Closure end should have the same indentation as the line that started it. " +
"Expected \(expected), got \(actual)."
return [
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, byteOffset: endOffset),
reason: reason)
]
}

private func startOffset(forDictionary dictionary: [String: SourceKitRepresentable], file: File) -> Int? {
guard let nameOffset = dictionary.nameOffset,
let nameLength = dictionary.nameLength else {
Expand All @@ -107,9 +216,21 @@ public struct ClosureEndIndentationRule: ASTRule, OptInRule, ConfigurationProvid
return methodByteRange.location
}

private func isSingleLineClosure(dictionary: [String: SourceKitRepresentable],
endPosition: Int, file: File) -> Bool {
let contents = file.contents.bridge()

guard let start = dictionary.bodyOffset,
let (startLine, _) = contents.lineAndCharacter(forByteOffset: start),
let (endLine, _) = contents.lineAndCharacter(forByteOffset: endPosition) else {
return false
}

return startLine == endLine
}

private func containsSingleLineClosure(dictionary: [String: SourceKitRepresentable],
endPosition: Int,
file: File) -> Bool {
endPosition: Int, file: File) -> Bool {
let contents = file.contents.bridge()

guard let closure = trailingClosure(dictionary: dictionary, file: file),
Expand Down Expand Up @@ -149,4 +270,22 @@ public struct ClosureEndIndentationRule: ASTRule, OptInRule, ConfigurationProvid
return true
}
}

private func isFirstArgumentOnNewline(_ dictionary: [String: SourceKitRepresentable],
file: File) -> Bool {
guard
let nameOffset = dictionary.nameOffset,
let nameLength = dictionary.nameLength,
let firstArgument = dictionary.enclosedArguments.first,
let firstArgumentOffset = firstArgument.offset,
case let offset = nameOffset + nameLength,
case let length = firstArgumentOffset - offset,
let range = file.contents.bridge().byteRangeToNSRange(start: offset, length: length),
let match = regex("\\(\\s*\\n\\s*").firstMatch(in: file.contents, options: [], range: range)?.range,
match.location == range.location else {
return false
}

return true
}
}

0 comments on commit 9a340a2

Please sign in to comment.