diff --git a/CHANGELOG.md b/CHANGELOG.md index bdc5afee3..fbd785d77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/Source/SwiftLintFramework/Extensions/SwiftExpressionKind.swift b/Source/SwiftLintFramework/Extensions/SwiftExpressionKind.swift new file mode 100644 index 000000000..67a6dba25 --- /dev/null +++ b/Source/SwiftLintFramework/Extensions/SwiftExpressionKind.swift @@ -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 + } + } +} diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 08ca1bcb1..97c2498b9 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -43,6 +43,7 @@ public struct RuleList { public let masterRuleList = RuleList(rules: AttributesRule.self, ClosingBraceRule.self, + ClosureParameterPositionRule.self, ClosureSpacingRule.self, ColonRule.self, CommaRule.self, diff --git a/Source/SwiftLintFramework/Rules/ClosureParameterPositionRule.swift b/Source/SwiftLintFramework/Rules/ClosureParameterPositionRule.swift new file mode 100644 index 000000000..d6f94a64f --- /dev/null +++ b/Source/SwiftLintFramework/Rules/ClosureParameterPositionRule.swift @@ -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 [] + } + } +} diff --git a/Source/SwiftLintFramework/Rules/EmptyParenthesesWithTrailingClosureRule.swift b/Source/SwiftLintFramework/Rules/EmptyParenthesesWithTrailingClosureRule.swift index 104c9e721..35a7afb9b 100644 --- a/Source/SwiftLintFramework/Rules/EmptyParenthesesWithTrailingClosureRule.swift +++ b/Source/SwiftLintFramework/Rules/EmptyParenthesesWithTrailingClosureRule.swift @@ -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 [] } diff --git a/Source/SwiftLintFramework/Rules/ExplicitInitRule.swift b/Source/SwiftLintFramework/Rules/ExplicitInitRule.swift index 565a9d193..939967428 100644 --- a/Source/SwiftLintFramework/Rules/ExplicitInitRule.swift +++ b/Source/SwiftLintFramework/Rules/ExplicitInitRule.swift @@ -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) + diff --git a/Source/SwiftLintFramework/Rules/OverriddenSuperCallRule.swift b/Source/SwiftLintFramework/Rules/OverriddenSuperCallRule.swift index 019b2d411..7b74f0462 100644 --- a/Source/SwiftLintFramework/Rules/OverriddenSuperCallRule.swift +++ b/Source/SwiftLintFramework/Rules/OverriddenSuperCallRule.swift @@ -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 } diff --git a/Source/SwiftLintFramework/Rules/TrailingCommaRule.swift b/Source/SwiftLintFramework/Rules/TrailingCommaRule.swift index ddebc8190..ee99e5228 100644 --- a/Source/SwiftLintFramework/Rules/TrailingCommaRule.swift +++ b/Source/SwiftLintFramework/Rules/TrailingCommaRule.swift @@ -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 - } - } -} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index c07dfa01f..cd184a7c9 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -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 = ""; }; D46E041C1DE3712C00728374 /* TrailingCommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaRule.swift; sourceTree = ""; }; D47079A61DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EmptyParenthesesWithTrailingClosureRule.swift; sourceTree = ""; }; + D47079A81DFDBED000027086 /* ClosureParameterPositionRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureParameterPositionRule.swift; sourceTree = ""; }; + D47079AA1DFDCF7A00027086 /* SwiftExpressionKind.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwiftExpressionKind.swift; sourceTree = ""; }; D47A510D1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwitchCaseOnNewlineRule.swift; sourceTree = ""; }; D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributesRule.swift; sourceTree = ""; }; D48AE2CB1DFB58C5001C6A4A /* AttributesRulesExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributesRulesExamples.swift; sourceTree = ""; }; @@ -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 */, diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift index 37a94769b..f92ecd373 100644 --- a/Tests/SwiftLintFrameworkTests/RulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift @@ -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),