Skip to content
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

Include named closure parameters in closure end indentation rule #2132

Merged
merged 6 commits into from
May 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
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
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
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
}
}