Merge pull request #960 from marcelofabri/closure_parameter_position

Add closure_parameter_position rule
2026-05-07 20:12:49 +00:00 · 2016-12-13 01:33:00 -08:00
parent 5dfed04b6b bbf594dd08
commit 11bfa938ae
10 changed files with 183 additions and 53 deletions
@@ -66,6 +66,11 @@
  empty parentheses after method call when using trailing closures.  
  [Marcelo Fabri](https://github.com/marcelofabri)
  [#885](https://github.com/realm/SwiftLint/issues/885)
+  
+* Add `closure_parameter_position` rule that validates that closure
+  parameters are in the same line as the opening brace.  
+  [Marcelo Fabri](https://github.com/marcelofabri)
+  [#931](https://github.com/realm/SwiftLint/issues/931)

 * `type_name` rule now validates `typealias` and `associatedtype` too.  
  [Marcelo Fabri](https://github.com/marcelofabri)
@@ -0,0 +1,35 @@
+//
+//  SwiftExpressionKind.swift
+//  SwiftLint
+//
+//  Created by Marcelo Fabri on 12/11/16.
+//  Copyright © 2016 Realm. All rights reserved.
+//
+
+import Foundation
+
+public enum SwiftExpressionKind: String {
+    case call = "source.lang.swift.expr.call"
+    case argument = "source.lang.swift.expr.argument"
+    case array = "source.lang.swift.expr.array"
+    case dictionary = "source.lang.swift.expr.dictionary"
+    case objectLiteral = "source.lang.swift.expr.object_literal"
+    case other
+
+    public init?(rawValue: String) {
+        switch rawValue {
+        case SwiftExpressionKind.call.rawValue:
+            self = .call
+        case SwiftExpressionKind.argument.rawValue:
+            self = .argument
+        case SwiftExpressionKind.array.rawValue:
+            self = .array
+        case SwiftExpressionKind.dictionary.rawValue:
+            self = .dictionary
+        case SwiftExpressionKind.objectLiteral.rawValue:
+            self = .objectLiteral
+        default:
+            self = .other
+        }
+    }
+}
@@ -43,6 +43,7 @@ public struct RuleList {
 public let masterRuleList = RuleList(rules:
    AttributesRule.self,
    ClosingBraceRule.self,
+    ClosureParameterPositionRule.self,
    ClosureSpacingRule.self,
    ColonRule.self,
    CommaRule.self,
@@ -0,0 +1,117 @@
+//
+//  ClosureParameterPositionRule.swift
+//  SwiftLint
+//
+//  Created by Marcelo Fabri on 12/11/16.
+//  Copyright © 2016 Realm. All rights reserved.
+//
+
+import Foundation
+import SourceKittenFramework
+
+public struct ClosureParameterPositionRule: ASTRule, ConfigurationProviderRule {
+    public var configuration = SeverityConfiguration(.warning)
+
+    public init() {}
+
+    public static let description = RuleDescription(
+        identifier: "closure_parameter_position",
+        name: "Closure Parameter Position",
+        description: "Closure parameters should be on the same line as opening brace.",
+        nonTriggeringExamples: [
+            "[1, 2].map { $0 + 1 }\n",
+            "[1, 2].map({ $0 + 1 })\n",
+            "[1, 2].map { number in\n number + 1 \n}\n",
+            "[1, 2].map { number -> Int in\n number + 1 \n}\n",
+            "[1, 2].map { (number: Int) -> Int in\n number + 1 \n}\n",
+            "[1, 2].map { [weak self] number in\n number + 1 \n}\n",
+            "[1, 2].something(closure: { number in\n number + 1 \n})\n",
+            "let isEmpty = [1, 2].isEmpty()\n",
+            "rlmConfiguration.migrationBlock.map { rlmMigration in\n" +
+                "return { migration, schemaVersion in\n" +
+                    "rlmMigration(migration.rlmMigration, schemaVersion)\n" +
+                "}\n" +
+            "}"
+        ],
+        triggeringExamples: [
+            "[1, 2].map {\n ↓number in\n number + 1 \n}\n",
+            "[1, 2].map {\n ↓number -> Int in\n number + 1 \n}\n",
+            "[1, 2].map {\n (↓number: Int) -> Int in\n number + 1 \n}\n",
+            "[1, 2].map {\n [weak self] ↓number in\n number + 1 \n}\n",
+            "[1, 2].map { [weak self]\n ↓number in\n number + 1 \n}\n",
+            "[1, 2].map({\n ↓number in\n number + 1 \n})\n",
+            "[1, 2].something(closure: {\n ↓number in\n number + 1 \n})\n",
+            "[1, 2].reduce(0) {\n ↓sum, ↓number in\n number + sum \n}\n"
+        ]
+    )
+
+    private static let openBraceRegex = regex("\\{")
+
+    public func validateFile(_ file: File,
+                             kind: SwiftExpressionKind,
+                             dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
+        guard kind == .call else {
+            return []
+        }
+
+        guard let nameOffset = (dictionary["key.nameoffset"] as? Int64).flatMap({ Int($0) }),
+            let nameLength = (dictionary["key.namelength"] as? Int64).flatMap({ Int($0) }),
+            let bodyLength = (dictionary["key.bodylength"] as? Int64).flatMap({ Int($0) }),
+            bodyLength > 0 else {
+                return []
+        }
+
+        let parameters = filterByKind(dictionary: dictionary, kind: .varParameter)
+        let rangeStart = nameOffset + nameLength
+        let regex = ClosureParameterPositionRule.openBraceRegex
+
+        // parameters from inner closures are reported on the top-level one, so we can't just
+        // use the first and last parameters to check, we need to check all of them
+        return parameters.flatMap { param -> StyleViolation? in
+            guard let paramOffset = (param["key.offset"] as? Int64).flatMap({ Int($0) }) else {
+                return nil
+            }
+
+            let rangeLength = paramOffset - rangeStart
+            let contents = file.contents.bridge()
+
+            guard let range = contents.byteRangeToNSRange(start: rangeStart,
+                                                          length: rangeLength),
+                let match = regex.matches(in: file.contents, options: [], range: range).last?.range,
+                match.location != NSNotFound,
+                let braceOffset = contents.NSRangeToByteRange(start: match.location,
+                                                              length: match.length)?.location,
+                let (braceLine, _) = contents.lineAndCharacter(forByteOffset: braceOffset),
+                let (paramLine, _) = contents.lineAndCharacter(forByteOffset: paramOffset),
+                braceLine != paramLine else {
+                    return nil
+            }
+
+            return StyleViolation(ruleDescription: type(of: self).description,
+                                  severity: configuration.severity,
+                                  location: Location(file: file, byteOffset: paramOffset))
+        }
+    }
+
+    private func filterByKind(dictionary: [String: SourceKitRepresentable],
+                              kind: SwiftDeclarationKind) -> [[String: SourceKitRepresentable]] {
+
+        let substructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? []
+        return substructure.flatMap { subItem -> [[String: SourceKitRepresentable]] in
+            guard let subDict = subItem as? [String: SourceKitRepresentable],
+                let kindString = subDict["key.kind"] as? String else {
+                    return []
+            }
+
+            if SwiftDeclarationKind(rawValue: kindString) == kind {
+                return [subDict]
+            }
+
+            if SwiftExpressionKind(rawValue: kindString) == .argument {
+                return filterByKind(dictionary: subDict, kind: kind)
+            }
+
+            return []
+        }
+    }
+}
@@ -24,7 +24,7 @@ public struct EmptyParenthesesWithTrailingClosureRule: ASTRule, ConfigurationPro
            "[1, 2].map({ $0 + 1 })\n",
            "[1, 2].reduce(0) { $0 + $1 }",
            "[1, 2].map { number in\n number + 1 \n}\n",
-            "let isEmpty = [1, 2].map.isEmpty()\n"
+            "let isEmpty = [1, 2].isEmpty()\n"
        ],
        triggeringExamples: [
            "[1, 2].map↓() { $0 + 1 }",
@@ -34,25 +34,12 @@ public struct EmptyParenthesesWithTrailingClosureRule: ASTRule, ConfigurationPro
        ]
    )

-    public enum Kind: String {
-        case exprCall = "source.lang.swift.expr.call"
-        case other
-        public init?(rawValue: String) {
-            switch rawValue {
-            case Kind.exprCall.rawValue:
-                self = .exprCall
-            default:
-                self = .other
-            }
-        }
-    }
-
    private static let emptyParenthesesRegex = regex("^\\s*\\(\\s*\\)")

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

@@ -35,21 +35,9 @@ public struct ExplicitInitRule: ASTRule, ConfigurationProviderRule, CorrectableR
        ]
    )

-    public enum Kind: String {
-        case expr_call = "source.lang.swift.expr.call"
-        case other
-        public init?(rawValue: String) {
-            switch rawValue {
-            case Kind.expr_call.rawValue: self = .expr_call
-            default: self = .other
-            }
-        }
-    }
-
-    public func validateFile(
-        _ file: File,
-        kind: ExplicitInitRule.Kind,
-        dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
+    public func validateFile(_ file: File,
+                             kind: SwiftExpressionKind,
+                             dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
        return violationRangesInFile(file, kind: kind, dictionary: dictionary).map {
            StyleViolation(ruleDescription: type(of: self).description,
                severity: configuration.severity,
@@ -59,7 +47,7 @@ public struct ExplicitInitRule: ASTRule, ConfigurationProviderRule, CorrectableR

    private let initializerWithType = regex("^[A-Z].*\\.init$")

-    private func violationRangesInFile(_ file: File, kind: ExplicitInitRule.Kind,
+    private func violationRangesInFile(_ file: File, kind: SwiftExpressionKind,
                                       dictionary: [String: SourceKitRepresentable]) -> [NSRange] {
        func isExpected(_ name: String) -> Bool {
            let range = NSRange(location: 0, length: name.utf16.count)
@@ -69,7 +57,7 @@ public struct ExplicitInitRule: ASTRule, ConfigurationProviderRule, CorrectableR

        let length = ".init".utf8.count

-        guard kind == .expr_call,
+        guard kind == .call,
            let name = dictionary["key.name"] as? String, isExpected(name),
            let nameOffset = dictionary["key.nameoffset"] as? Int64,
            let nameLength = dictionary["key.namelength"] as? Int64,
@@ -85,7 +73,7 @@ public struct ExplicitInitRule: ASTRule, ConfigurationProviderRule, CorrectableR
        return substructure.flatMap { subItem -> [NSRange] in
            guard let subDict = subItem as? [String: SourceKitRepresentable],
                let kindString = subDict["key.kind"] as? String,
-                let kind = ExplicitInitRule.Kind(rawValue: kindString) else {
+                let kind = SwiftExpressionKind(rawValue: kindString) else {
                    return []
            }
            return violationRangesInFile(file, dictionary: subDict) +
@@ -94,9 +94,10 @@ public struct OverriddenSuperCallRule: ConfigurationProviderRule, ASTRule, OptIn
        let superCall = "super.\(name)"
        return substructure.flatMap {
            guard let elems = $0 as? [String: SourceKitRepresentable],
-                let type = elems["key.kind"] as? String,
+                let type = (elems["key.kind"] as? String)
+                    .flatMap({ SwiftExpressionKind(rawValue: $0) }),
                let name = elems["key.name"] as? String,
-                type == "source.lang.swift.expr.call" && superCall.contains(name)
+                type == .call && superCall.contains(name)
                else { return nil }
            return name
        }
@@ -123,20 +123,3 @@ public struct TrailingCommaRule: ASTRule, ConfigurationProviderRule {
        }?.location
    }
 }
-
-public enum SwiftExpressionKind: String {
-    case array = "source.lang.swift.expr.array"
-    case dictionary = "source.lang.swift.expr.dictionary"
-    case other
-
-    public init?(rawValue: String) {
-        switch rawValue {
-        case SwiftExpressionKind.array.rawValue:
-            self = .array
-        case SwiftExpressionKind.dictionary.rawValue:
-            self = .dictionary
-        default:
-            self = .other
-        }
-    }
-}
@@ -85,6 +85,8 @@
 		D46252541DF63FB200BE2CA1 /* NumberSeparatorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46252531DF63FB200BE2CA1 /* NumberSeparatorRule.swift */; };
 		D46E041D1DE3712C00728374 /* TrailingCommaRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46E041C1DE3712C00728374 /* TrailingCommaRule.swift */; };
 		D47079A71DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47079A61DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift */; };
+		D47079A91DFDBED000027086 /* ClosureParameterPositionRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47079A81DFDBED000027086 /* ClosureParameterPositionRule.swift */; };
+		D47079AB1DFDCF7A00027086 /* SwiftExpressionKind.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47079AA1DFDCF7A00027086 /* SwiftExpressionKind.swift */; };
 		D47A510E1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47A510D1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift */; };
 		D47A51101DB2DD4800A4CC21 /* AttributesRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */; };
 		D48AE2CC1DFB58C5001C6A4A /* AttributesRulesExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D48AE2CB1DFB58C5001C6A4A /* AttributesRulesExamples.swift */; };
@@ -302,6 +304,8 @@
 		D46252531DF63FB200BE2CA1 /* NumberSeparatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorRule.swift; sourceTree = "<group>"; };
 		D46E041C1DE3712C00728374 /* TrailingCommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaRule.swift; sourceTree = "<group>"; };
 		D47079A61DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EmptyParenthesesWithTrailingClosureRule.swift; sourceTree = "<group>"; };
+		D47079A81DFDBED000027086 /* ClosureParameterPositionRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureParameterPositionRule.swift; sourceTree = "<group>"; };
+		D47079AA1DFDCF7A00027086 /* SwiftExpressionKind.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwiftExpressionKind.swift; sourceTree = "<group>"; };
 		D47A510D1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwitchCaseOnNewlineRule.swift; sourceTree = "<group>"; };
 		D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributesRule.swift; sourceTree = "<group>"; };
 		D48AE2CB1DFB58C5001C6A4A /* AttributesRulesExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributesRulesExamples.swift; sourceTree = "<group>"; };
@@ -664,6 +668,7 @@
 				D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */,
 				D48AE2CB1DFB58C5001C6A4A /* AttributesRulesExamples.swift */,
 				1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */,
+				D47079A81DFDBED000027086 /* ClosureParameterPositionRule.swift */,
 				1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */,
 				E88DEA831B0990F500A66CB0 /* ColonRule.swift */,
 				695BE9CE1BDFD92B0071E985 /* CommaRule.swift */,
@@ -765,6 +770,7 @@
 				E88DEA721B0984C400A66CB0 /* String+SwiftLint.swift */,
 				6CB514E81C760C6900FA02C4 /* Structure+SwiftLint.swift */,
 				E816194B1BFBF35D00946723 /* SwiftDeclarationKind+SwiftLint.swift */,
+				D47079AA1DFDCF7A00027086 /* SwiftExpressionKind.swift */,
 				E87E4A081BFB9CAE00FCFE46 /* SyntaxKind+SwiftLint.swift */,
 				6CC4259A1C77046200AEA885 /* SyntaxMap+SwiftLint.swift */,
 				3B1150C91C31FC3F00D83B1E /* Yaml+SwiftLint.swift */,
@@ -1014,6 +1020,7 @@
 				E86396CB1BADB519002C9E88 /* CSVReporter.swift in Sources */,
 				37B3FA8B1DFD45A700AD30D2 /* Dictionary+SwiftLint.swift in Sources */,
 				E88198561BEA94D800333A11 /* FileLengthRule.swift in Sources */,
+				D47079A91DFDBED000027086 /* ClosureParameterPositionRule.swift in Sources */,
 				E8B67C3E1C095E6300FDED8E /* Correction.swift in Sources */,
 				E88198531BEA944400333A11 /* LineLengthRule.swift in Sources */,
 				E847F0A91BFBBABD00EA9363 /* EmptyCountRule.swift in Sources */,
@@ -1054,6 +1061,7 @@
 				D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */,
 				7C0C2E7A1D2866CB0076435A /* ExplicitInitRule.swift in Sources */,
 				E88DEA771B098D0C00A66CB0 /* Rule.swift in Sources */,
+				D47079AB1DFDCF7A00027086 /* SwiftExpressionKind.swift in Sources */,
 				24B4DF0D1D6DFDE90097803B /* RedundantNilCoalescingRule.swift in Sources */,
 				7250948A1D0859260039B353 /* StatementPositionConfiguration.swift in Sources */,
 				E81619531BFC162C00946723 /* QueuedPrint.swift in Sources */,
@@ -94,6 +94,10 @@ class RulesTests: XCTestCase {
        verifyRule(CommaRule.description)
    }

+    func testClosureParameterPosition() {
+        verifyRule(ClosureParameterPositionRule.description)
+    }
+
    func testClosureSpacingRule() {
        verifyRule(ClosureSpacingRule.description)
    }
@@ -387,6 +391,7 @@ extension RulesTests {
            ("testClosingBrace", testClosingBrace),
            ("testColon", testColon),
            ("testComma", testComma),
+            ("testClosureParameterPosition", testClosureParameterPosition),
            ("testClosureSpacingRule", testClosureSpacingRule),
            ("testConditionalReturnsOnNewline", testConditionalReturnsOnNewline),
            ("testControlStatement", testControlStatement),