From 6fca21cdf6c66355d760721716a5a29174e91453 Mon Sep 17 00:00:00 2001 From: Erik Strottmann Date: Sat, 26 Aug 2017 19:37:33 -0700 Subject: [PATCH] Add `multiple_closures_with_trailing_closure` rule Multiple Closures with Trailing Closure rule disallows trailing closure syntax when passing more than one closure argument to a function. Fixes #1801. --- CHANGELOG.md | 5 + Rules.md | 62 ++++++++++++ .../Models/MasterRuleList.swift | 1 + ...tipleClosuresWithTrailingClosureRule.swift | 98 +++++++++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 4 + Tests/LinuxMain.swift | 1 + .../SwiftLintFrameworkTests/RulesTests.swift | 5 + 7 files changed, 176 insertions(+) create mode 100644 Source/SwiftLintFramework/Rules/MultipleClosuresWithTrailingClosureRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index e1a195344f..0f23921a16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,11 @@ [Marcelo Fabri](https://github.com/marcelofabri) [#1803](https://github.com/realm/SwiftLint/issues/1803) +* Add `multiple_closures_with_trailing_closure` rule that disallows trailing + closure syntax when passing more than one closure argument to a function. + [Erik Strottmann](https://github.com/erikstrottmann) + [#1801](https://github.com/realm/SwiftLint/issues/1801) + ##### Bug Fixes * Fix false positive on `force_unwrapping` rule when declaring diff --git a/Rules.md b/Rules.md index 6fea9b94b2..90bf0448f5 100644 --- a/Rules.md +++ b/Rules.md @@ -54,6 +54,7 @@ * [Line Length](#line-length) * [Mark](#mark) * [Multiline Parameters](#multiline-parameters) +* [Multiple Closures with Trailing Closure](#multiple-closures-with-trailing-closure) * [Nesting](#nesting) * [Nimble Operator](#nimble-operator) * [No Extension Access Modifier](#no-extension-access-modifier) @@ -6522,6 +6523,67 @@ class Foo { +## Multiple Closures with Trailing Closure + +Identifier | Enabled by default | Supports autocorrection | Kind +--- | --- | --- | --- +`multiple_closures_with_trailing_closure` | Enabled | No | style + +Trailing closure syntax should not be used when passing more than one closure argument. + +### Examples + +
+Non Triggering Examples + +```swift +foo.map { $0 + 1 } + +``` + +```swift +foo.reduce(0) { $0 + $1 } + +``` + +```swift +if let foo = bar.map({ $0 + 1 }) { + +} + +``` + +```swift +foo.something(param1: { $0 }, param2: { $0 + 1 }) + +``` + +```swift +UIView.animate(withDuration: 1.0) { + someView.alpha = 0.0 +} +``` + +
+
+Triggering Examples + +```swift +foo.something(param1: { $0 }) ↓{ $0 + 1 } +``` + +```swift +UIView.animate(withDuration: 1.0, animations: { + someView.alpha = 0.0 +}) ↓{ _ in + someView.removeFromSuperview() +} +``` + +
+ + + ## Nesting Identifier | Enabled by default | Supports autocorrection | Kind diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 0641748df9..41d0f14680 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -62,6 +62,7 @@ public let masterRuleList = RuleList(rules: [ LineLengthRule.self, MarkRule.self, MultilineParametersRule.self, + MultipleClosuresWithTrailingClosureRule.self, NestingRule.self, NimbleOperatorRule.self, NoExtensionAccessModifierRule.self, diff --git a/Source/SwiftLintFramework/Rules/MultipleClosuresWithTrailingClosureRule.swift b/Source/SwiftLintFramework/Rules/MultipleClosuresWithTrailingClosureRule.swift new file mode 100644 index 0000000000..f049575f48 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/MultipleClosuresWithTrailingClosureRule.swift @@ -0,0 +1,98 @@ +// +// MultipleClosuresWithTrailingClosureRule.swift +// SwiftLint +// +// Created by Erik Strottmann on 8/26/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public struct MultipleClosuresWithTrailingClosureRule: ASTRule, ConfigurationProviderRule { + public var configuration = SeverityConfiguration(.warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "multiple_closures_with_trailing_closure", + name: "Multiple Closures with Trailing Closure", + description: "Trailing closure syntax should not be used when passing more than one closure argument.", + kind: .style, + nonTriggeringExamples: [ + "foo.map { $0 + 1 }\n", + "foo.reduce(0) { $0 + $1 }\n", + "if let foo = bar.map({ $0 + 1 }) {\n\n}\n", + "foo.something(param1: { $0 }, param2: { $0 + 1 })\n", + "UIView.animate(withDuration: 1.0) {\n" + + " someView.alpha = 0.0\n" + + "}" + ], + triggeringExamples: [ + "foo.something(param1: { $0 }) ↓{ $0 + 1 }", + "UIView.animate(withDuration: 1.0, animations: {\n" + + " someView.alpha = 0.0\n" + + "}) ↓{ _ in\n" + + " someView.removeFromSuperview()\n" + + "}" + ] + ) + + public func validate(file: File, kind: SwiftExpressionKind, + dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { + + guard let call = Call(file: file, kind: kind, dictionary: dictionary), call.hasTrailingClosure else { + return [] + } + + let closureArguments = call.closureArguments + guard closureArguments.count > 1, let trailingClosureOffset = closureArguments.last?.offset else { + return [] + } + + return [ + StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: Location(file: file, byteOffset: trailingClosureOffset)) + ] + } +} + +private struct Call { + let file: File + let dictionary: [String: SourceKitRepresentable] + let offset: Int + + init?(file: File, kind: SwiftExpressionKind, dictionary: [String: SourceKitRepresentable]) { + guard kind == .call, let offset = dictionary.offset else { + return nil + } + self.file = file + self.dictionary = dictionary + self.offset = offset + } + + var hasTrailingClosure: Bool { + guard let length = dictionary.length, + let text = file.contents.bridge().substringWithByteRange(start: offset, length: length) + else { + return false + } + + return !text.hasSuffix(")") + } + + var closureArguments: [[String: SourceKitRepresentable]] { + return dictionary.enclosedArguments.filter { argument in + guard let offset = argument.bodyOffset, + let length = argument.bodyLength, + let range = file.contents.bridge().byteRangeToNSRange(start: offset, length: length), + let match = regex("\\s*\\{").firstMatch(in: file.contents, options: [], range: range)?.range, + match.location == range.location else { + return false + } + + return true + } + } +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index f15ed5a4fd..9516f274b7 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -114,6 +114,7 @@ B3935A32BE03C4D11B4364D6 /* CannedCSVReporterOutput.csv in Resources */ = {isa = PBXBuildFile; fileRef = B3935939C8366514D2694722 /* CannedCSVReporterOutput.csv */; }; B3935EE74B1E8E14FBD65E7F /* String+XML.swift in Sources */ = {isa = PBXBuildFile; fileRef = B39353F28BCCA39247B316BD /* String+XML.swift */; }; B58AEED61C492C7B00E901FD /* ForceUnwrappingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = B58AEED51C492C7B00E901FD /* ForceUnwrappingRule.swift */; }; + BB00B4E91F5216090079869F /* MultipleClosuresWithTrailingClosureRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB00B4E71F5216070079869F /* MultipleClosuresWithTrailingClosureRule.swift */; }; BFF028AE1CBCF8A500B38A9D /* TrailingWhitespaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */; }; C328A2F71E6759AE00A9E4D7 /* ExplicitTypeInterfaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = C328A2F51E67595500A9E4D7 /* ExplicitTypeInterfaceRule.swift */; }; C3DE5DAC1E7DF9CA00761483 /* FatalErrorMessageRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = C3DE5DAA1E7DF99B00761483 /* FatalErrorMessageRule.swift */; }; @@ -427,6 +428,7 @@ B3935939C8366514D2694722 /* CannedCSVReporterOutput.csv */ = {isa = PBXFileReference; lastKnownFileType = file.csv; path = CannedCSVReporterOutput.csv; sourceTree = ""; }; B39359A325FE84B7EDD1C455 /* CannedJunitReporterOutput.xml */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xml; path = CannedJunitReporterOutput.xml; sourceTree = ""; }; B58AEED51C492C7B00E901FD /* ForceUnwrappingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ForceUnwrappingRule.swift; sourceTree = ""; }; + BB00B4E71F5216070079869F /* MultipleClosuresWithTrailingClosureRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultipleClosuresWithTrailingClosureRule.swift; sourceTree = ""; }; BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingWhitespaceConfiguration.swift; sourceTree = ""; }; C328A2F51E67595500A9E4D7 /* ExplicitTypeInterfaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExplicitTypeInterfaceRule.swift; sourceTree = ""; }; C3DE5DAA1E7DF99B00761483 /* FatalErrorMessageRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FatalErrorMessageRule.swift; sourceTree = ""; }; @@ -998,6 +1000,7 @@ 856651A61D6B395F005E6B29 /* MarkRule.swift */, 6238AE411ED4D734006C3601 /* MultilineParametersRule.swift */, 621061BE1ED57E640082D51E /* MultilineParametersRuleExamples.swift */, + BB00B4E71F5216070079869F /* MultipleClosuresWithTrailingClosureRule.swift */, E88DEA951B099CF200A66CB0 /* NestingRule.swift */, D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */, 1E18574A1EADBA51004F89F7 /* NoExtensionAccessModifierRule.swift */, @@ -1405,6 +1408,7 @@ D47079A91DFDBED000027086 /* ClosureParameterPositionRule.swift in Sources */, E8B67C3E1C095E6300FDED8E /* Correction.swift in Sources */, 623E36F21F3DB988002E5B71 /* QuickDiscouragedCallRuleExamples.swift in Sources */, + BB00B4E91F5216090079869F /* MultipleClosuresWithTrailingClosureRule.swift in Sources */, E88198531BEA944400333A11 /* LineLengthRule.swift in Sources */, D47F31151EC918B600E3E1CA /* ProtocolPropertyAccessorsOrderRule.swift in Sources */, 92CCB2D71E1EEFA300C8E5A3 /* UnusedOptionalBindingRule.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 191154920e..8e128ef52a 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -385,6 +385,7 @@ extension RulesTests { ("testLetVarWhitespace", testLetVarWhitespace), ("testMark", testMark), ("testMultilineParameters", testMultilineParameters), + ("testMultipleClosuresWithTrailingClosure", testMultipleClosuresWithTrailingClosure), ("testNesting", testNesting), ("testNoExtensionAccessModifierRule", testNoExtensionAccessModifierRule), ("testNoGroupingExtension", testNoGroupingExtension), diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift index dad188c080..4a037a85c2 100644 --- a/Tests/SwiftLintFrameworkTests/RulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift @@ -9,6 +9,7 @@ import SwiftLintFramework import XCTest +// swiftlint:disable file_length // swiftlint:disable type_body_length class RulesTests: XCTestCase { @@ -192,6 +193,10 @@ class RulesTests: XCTestCase { verifyRule(MultilineParametersRule.description) } + func testMultipleClosuresWithTrailingClosure() { + verifyRule(MultipleClosuresWithTrailingClosureRule.description) + } + func testNesting() { verifyRule(NestingRule.description) }