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

Closure end indentation rule #1004

Merged
merged 7 commits into from
Dec 20, 2016
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
1 change: 1 addition & 0 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ opt_in_rules:
- nimble_operator
- attributes
- operator_usage_whitespace
- closure_end_indentation

file_header:
required_pattern: |
Expand Down
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,16 @@

##### Enhancements

* None.
* Add `closure_end_indentation` opt-in rule that validates closure closing
braces according to these rules:
* If the method call has chained breaking lines on each method
(`.` is on a new line), the closing brace should be vertically aligned
with the `.`.
* Otherwise, the closing brace should be vertically aligned with
the beginning of the statement in the first line.

[Marcelo Fabri](https://github.com/marcelofabri)
[#326](https://github.com/realm/SwiftLint/issues/326)

##### Bug Fixes

Expand Down
10 changes: 5 additions & 5 deletions Source/SwiftLintFramework/Extensions/File+SwiftLint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ extension File {
return []
}
let contents = self.contents.bridge()
return matchPattern("swiftlint:(enable|disable)(:previous|:this|:next)?\\ [^\\n]+",
withSyntaxKinds: [.comment]).flatMap { range in
return Command(string: contents, range: range)
}.flatMap { command in
return command.expand()
let pattern = "swiftlint:(enable|disable)(:previous|:this|:next)?\\ [^\\n]+"
return matchPattern(pattern, withSyntaxKinds: [.comment]).flatMap { range in
return Command(string: contents, range: range)
}.flatMap { command in
return command.expand()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ extension FileManager {
return try subpathsOfDirectory(atPath: absolutePath)
.map(absolutePath.bridge().appendingPathComponent).filter {
$0.bridge().isSwiftFile()
}
}
} catch {
fatalError("Couldn't find files in \(absolutePath): \(error)")
}
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public struct RuleList {
public let masterRuleList = RuleList(rules:
AttributesRule.self,
ClosingBraceRule.self,
ClosureEndIndentationRule.self,
ClosureParameterPositionRule.self,
ClosureSpacingRule.self,
ColonRule.self,
Expand Down
111 changes: 111 additions & 0 deletions Source/SwiftLintFramework/Rules/ClosureEndIndentationRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
//
// ClosureEndIndentationRule.swift
// SwiftLint
//
// Created by Marcelo Fabri on 12/18/16.
// Copyright © 2016 Realm. All rights reserved.
//

import Foundation
import SourceKittenFramework

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

public init() {}

public static let description = RuleDescription(
identifier: "closure_end_indentation",
name: "Closure End Indentation",
description: "Closure end should have the same indentation as the line that started it.",
nonTriggeringExamples: [
"SignalProducer(values: [1, 2, 3])\n" +
" .startWithNext { number in\n" +
" print(number)\n" +
" }\n",
"[1, 2].map { $0 + 1 }\n",
"return matchPattern(pattern, withSyntaxKinds: [.comment]).flatMap { range in\n" +
" return Command(string: contents, range: range)\n" +
"}.flatMap { command in\n" +
" return command.expand()\n" +
"}\n"
],
triggeringExamples: [
"SignalProducer(values: [1, 2, 3])\n" +
" .startWithNext { number in\n" +
" print(number)\n" +
"↓}\n",
"return matchPattern(pattern, withSyntaxKinds: [.comment]).flatMap { range in\n" +
" return Command(string: contents, range: range)\n" +
" ↓}.flatMap { command in\n" +
" return command.expand()\n" +
"↓}\n"
]
)

private static let notWhitespace = regex("[^\\s]")

public func validateFile(_ file: File,
kind: SwiftExpressionKind,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
guard kind == .call else {
return []
}

let contents = file.contents.bridge()
guard let offset = (dictionary["key.offset"] as? Int64).flatMap({ Int($0) }),
let length = (dictionary["key.length"] as? Int64).flatMap({ Int($0) }),
let bodyLength = (dictionary["key.bodylength"] as? Int64).flatMap({ Int($0) }),
let nameOffset = (dictionary["key.nameoffset"] as? Int64).flatMap({ Int($0) }),
let nameLength = (dictionary["key.namelength"] as? Int64).flatMap({ Int($0) }),
bodyLength > 0,
case let endOffset = offset + length - 1,
contents.substringWithByteRange(start: endOffset, length: 1) == "}",
let startOffset = startOffsetFor(dictionary: dictionary, file: file),
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 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 startOffsetFor(dictionary: [String: SourceKitRepresentable],
file: File) -> Int? {
guard let nameOffset = (dictionary["key.nameoffset"] as? Int64).flatMap({ Int($0) }),
let nameLength = (dictionary["key.namelength"] as? Int64).flatMap({ Int($0) }) else {
return nil
}

let newLineRegex = regex("\n(\\s*\\}?\\.)")
let contents = file.contents.bridge()
guard let range = contents.byteRangeToNSRange(start: nameOffset, length: nameLength),
let match = newLineRegex.matches(in: file.contents, options: [],
range: range).last?.rangeAt(1),
let methodByteRange = contents.NSRangeToByteRange(start: match.location,
length: match.length) else {
return nameOffset
}

return methodByteRange.location
}
}
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Rules/CommaRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,6 @@ public struct CommaRule: CorrectableRule, ConfigurationProviderRule {

// return first captured range
return firstRange
}
}
}
}
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Rules/ForceUnwrappingRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public struct ForceUnwrappingRule: OptInRule, ConfigurationProviderRule {
} else {
return nil
}
}
}
}

// Returns if range should generate violation
Expand Down
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Rules/NestingRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public struct NestingRule: ASTRule, ConfigurationProviderRule {
} + ["enum Enum0 { enum Enum1 { case Case } }"],
triggeringExamples: ["class", "struct", "enum"].map { kind in
"\(kind) A { \(kind) B { ↓\(kind) C {} } }\n"
} + [
} + [
"func func0() {\nfunc func1() {\nfunc func2() {\nfunc func3() {\nfunc func4() { " +
"func func5() {\n↓func func6() {\n}\n}\n}\n}\n}\n}\n}\n"
]
Expand Down
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Rules/SyntacticSugarRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public struct SyntacticSugarRule: Rule, ConfigurationProviderRule {
let pattern = "\\b(" + types.joined(separator: "|") + ")\\s*<.*?>"

return file.matchPattern(pattern,
excludingSyntaxKinds: SyntaxKind.commentAndStringKinds()).map {
excludingSyntaxKinds: SyntaxKind.commentAndStringKinds()).map {
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, characterOffset: $0.location))
Expand Down
3 changes: 2 additions & 1 deletion Source/SwiftLintFramework/Rules/VoidReturnRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public struct VoidReturnRule: ConfigurationProviderRule, CorrectableRule {
let excludingPattern = "(\(pattern))\\s*(throws\\s+)?->"

return file.matchPattern(pattern, excludingSyntaxKinds: kinds,
excludingPattern: excludingPattern) { $0.rangeAt(1) }.flatMap {
excludingPattern: excludingPattern,
exclusionMapping: { $0.rangeAt(1) }).flatMap {
let parensRegex = NSRegularExpression.forcePattern(parensPattern)
return parensRegex.firstMatch(in: file.contents, options: [], range: $0)?.range
}
Expand Down
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
D41E7E0B1DF9DABB0065259A /* RedundantStringEnumValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */; };
D4348EEA1C46122C007707FB /* FunctionBodyLengthRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */; };
D43B04641E0620AB004016AF /* UnusedEnumeratedRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43B04631E0620AB004016AF /* UnusedEnumeratedRule.swift */; };
D43B046B1E075905004016AF /* ClosureEndIndentationRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43B046A1E075905004016AF /* ClosureEndIndentationRule.swift */; };
D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43DB1071DC573DA00281215 /* ImplicitGetterRule.swift */; };
D44254201DB87CA200492EA4 /* ValidIBInspectableRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */; };
D44254271DB9C15C00492EA4 /* SyntacticSugarRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D44254251DB9C12300492EA4 /* SyntacticSugarRule.swift */; };
Expand Down Expand Up @@ -322,6 +323,7 @@
D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantStringEnumValueRule.swift; sourceTree = "<group>"; };
D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FunctionBodyLengthRuleTests.swift; sourceTree = "<group>"; };
D43B04631E0620AB004016AF /* UnusedEnumeratedRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedEnumeratedRule.swift; sourceTree = "<group>"; };
D43B046A1E075905004016AF /* ClosureEndIndentationRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureEndIndentationRule.swift; sourceTree = "<group>"; };
D43DB1071DC573DA00281215 /* ImplicitGetterRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImplicitGetterRule.swift; sourceTree = "<group>"; };
D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ValidIBInspectableRule.swift; sourceTree = "<group>"; };
D44254251DB9C12300492EA4 /* SyntacticSugarRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SyntacticSugarRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -705,6 +707,7 @@
D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */,
D48AE2CB1DFB58C5001C6A4A /* AttributesRulesExamples.swift */,
1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */,
D43B046A1E075905004016AF /* ClosureEndIndentationRule.swift */,
D47079A81DFDBED000027086 /* ClosureParameterPositionRule.swift */,
1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */,
E88DEA831B0990F500A66CB0 /* ColonRule.swift */,
Expand Down Expand Up @@ -1109,6 +1112,7 @@
D47A510E1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift in Sources */,
D48AE2CC1DFB58C5001C6A4A /* AttributesRulesExamples.swift in Sources */,
E88DEA6F1B09843F00A66CB0 /* Location.swift in Sources */,
D43B046B1E075905004016AF /* ClosureEndIndentationRule.swift in Sources */,
93E0C3CE1D67BD7F007FA25D /* ConditionalReturnsOnNewline.swift in Sources */,
D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */,
7C0C2E7A1D2866CB0076435A /* ExplicitInitRule.swift in Sources */,
Expand Down
5 changes: 5 additions & 0 deletions Tests/SwiftLintFrameworkTests/RulesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ class RulesTests: XCTestCase {
verifyRule(CommaRule.description)
}

func testClosureEndIndentation() {
verifyRule(ClosureEndIndentationRule.description)
}

func testClosureParameterPosition() {
verifyRule(ClosureParameterPositionRule.description)
}
Expand Down Expand Up @@ -414,6 +418,7 @@ extension RulesTests {
("testClosingBrace", testClosingBrace),
("testColon", testColon),
("testComma", testComma),
("testClosureEndIndentation", testClosureEndIndentation),
("testClosureParameterPosition", testClosureParameterPosition),
("testClosureSpacingRule", testClosureSpacingRule),
("testConditionalReturnsOnNewline", testConditionalReturnsOnNewline),
Expand Down