diff --git a/CHANGELOG.md b/CHANGELOG.md index 1829f8e31..74de39f66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,11 @@ [JP Simard](https://github.com/jpsim) [#1002](https://github.com/realm/SwiftLint/issues/1002) +* Add `multiline_arguments` opt-in rule that warns to either keep + all the arguments of a function call on the same line, + or one per line. + [Marcel Jackwerth](https://github.com/sirlantis) + ##### Bug Fixes * Improve how `opening_brace` rule reports violations locations. diff --git a/Rules.md b/Rules.md index 3f718feed..32a3bbcba 100644 --- a/Rules.md +++ b/Rules.md @@ -56,6 +56,7 @@ * [Variable Declaration Whitespace](#variable-declaration-whitespace) * [Line Length](#line-length) * [Mark](#mark) +* [Multiline Arguments](#multiline-arguments) * [Multiline Parameters](#multiline-parameters) * [Multiple Closures with Trailing Closure](#multiple-closures-with-trailing-closure) * [Nesting](#nesting) @@ -6529,6 +6530,131 @@ extension MarkTest {} +## Multiline Arguments + +Identifier | Enabled by default | Supports autocorrection | Kind +--- | --- | --- | --- +`multiline_arguments` | Disabled | No | style + +Arguments should be either on the same line, or one per line. + +### Examples + +
+Non Triggering Examples + +```swift +foo() +``` + +```swift +foo( + +) +``` + +```swift +foo { } +``` + +```swift +foo { + +} +``` + +```swift +foo(0) +``` + +```swift +foo(0, 1) +``` + +```swift +foo(0, 1) { } +``` + +```swift +foo(0, param1: 1) +``` + +```swift +foo(0, param1: 1) { } +``` + +```swift +foo(param1: 1) +``` + +```swift +foo(param1: 1) { } +``` + +```swift +foo(param1: 1, param2: true) { } +``` + +```swift +foo(param1: 1, param2: true, param3: [3]) { } +``` + +```swift +foo(param1: 1, param2: true, param3: [3]) { + bar() +} +``` + +```swift +foo(param1: 1, + param2: true, + param3: [3]) +``` + +```swift +foo( + param1: 1, param2: true, param3: [3] +) +``` + +```swift +foo( + param1: 1, + param2: true, + param3: [3] +) +``` + +
+
+Triggering Examples + +```swift +foo(0, + param1: 1, ↓param2: true, ↓param3: [3]) +``` + +```swift +foo(0, ↓param1: 1, + param2: true, ↓param3: [3]) +``` + +```swift +foo(0, ↓param1: 1, ↓param2: true, + param3: [3]) +``` + +```swift +foo( + 0, ↓param1: 1, + param2: true, ↓param3: [3] +) +``` + +
+ + + ## Multiline Parameters Identifier | Enabled by default | Supports autocorrection | Kind diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index c0a582229..0c855ece0 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -64,6 +64,7 @@ public let masterRuleList = RuleList(rules: [ LetVarWhitespaceRule.self, LineLengthRule.self, MarkRule.self, + MultilineArgumentsRule.self, MultilineParametersRule.self, MultipleClosuresWithTrailingClosureRule.self, NestingRule.self, diff --git a/Source/SwiftLintFramework/Rules/MultilineArgumentsConfiguration.swift b/Source/SwiftLintFramework/Rules/MultilineArgumentsConfiguration.swift new file mode 100644 index 000000000..1740ccf07 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/MultilineArgumentsConfiguration.swift @@ -0,0 +1,59 @@ +// +// MultilineArgumentsConfiguration.swift +// SwiftLint +// +// Created by Marcel Jackwerth on 9/29/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import Foundation + +private enum ConfigurationKey: String { + case firstArgumentLocation = "first_argument_location" +} + +public struct MultilineArgumentsConfiguration: RuleConfiguration, Equatable { + public enum FirstArgumentLocation: String { + case anyLine = "any_line" + case sameLine = "same_line" + case nextLine = "next_line" + + init(value: Any) throws { + guard + let string = (value as? String)?.lowercased(), + let value = FirstArgumentLocation(rawValue: string) else { + throw ConfigurationError.unknownConfiguration + } + + self = value + } + } + + private(set) var severityConfiguration = SeverityConfiguration(.warning) + private(set) var firstArgumentLocation = FirstArgumentLocation.anyLine + + public var consoleDescription: String { + return severityConfiguration.consoleDescription + + ", \(ConfigurationKey.firstArgumentLocation.rawValue): \(firstArgumentLocation.rawValue)" + } + + public mutating func apply(configuration: Any) throws { + guard let configuration = configuration as? [String: Any] else { + throw ConfigurationError.unknownConfiguration + } + + if let modeString = configuration[ConfigurationKey.firstArgumentLocation.rawValue] { + try firstArgumentLocation = FirstArgumentLocation(value: modeString) + } + + if let severityString = configuration["severity"] as? String { + try severityConfiguration.apply(configuration: severityString) + } + } + + public static func == (lhs: MultilineArgumentsConfiguration, + rhs: MultilineArgumentsConfiguration) -> Bool { + return lhs.severityConfiguration == rhs.severityConfiguration && + lhs.firstArgumentLocation == rhs.firstArgumentLocation + } +} diff --git a/Source/SwiftLintFramework/Rules/MultilineArgumentsRule.swift b/Source/SwiftLintFramework/Rules/MultilineArgumentsRule.swift new file mode 100644 index 000000000..44c3b2ea2 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/MultilineArgumentsRule.swift @@ -0,0 +1,88 @@ +// +// MultilineArgumentsRule.swift +// SwiftLint +// +// Created by Marcel Jackwerth on 09/29/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public struct MultilineArgumentsRule: ASTRule, OptInRule, ConfigurationProviderRule { + public var configuration = MultilineArgumentsConfiguration() + + public init() {} + + public static let description = RuleDescription( + identifier: "multiline_arguments", + name: "Multiline Arguments", + description: "Arguments should be either on the same line, or one per line.", + kind: .style, + nonTriggeringExamples: MultilineArgumentsRuleExamples.nonTriggeringExamples, + triggeringExamples: MultilineArgumentsRuleExamples.triggeringExamples + ) + + public func validate(file: File, + kind: SwiftExpressionKind, + dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { + guard + kind == .call, + case let arguments = dictionary.enclosedArguments, + arguments.count > 1, + case let contents = file.contents.bridge(), + let nameOffset = dictionary.nameOffset, + let (nameLine, _) = contents.lineAndCharacter(forByteOffset: nameOffset) else { + return [] + } + + var visitedLines = Set() + + if configuration.firstArgumentLocation == .sameLine { + visitedLines.insert(nameLine) + } + + let lastIndex = arguments.count - 1 + let violatingOffsets: [Int] = arguments.enumerated().flatMap { idx, argument in + guard + let offset = argument.offset, + let (line, _) = contents.lineAndCharacter(forByteOffset: offset) else { + return nil + } + + let (firstVisit, _) = visitedLines.insert(line) + + if idx == lastIndex && isTrailingClosure(dictionary: dictionary, file: file) { + return nil + } else if idx == 0 { + switch configuration.firstArgumentLocation { + case .anyLine: return nil + case .nextLine: return line > nameLine ? nil : offset + case .sameLine: return line > nameLine ? offset : nil + } + } else { + return firstVisit ? nil : offset + } + } + + // only report violations if multiline + guard visitedLines.count > 1 else { return [] } + + return violatingOffsets.map { + StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severityConfiguration.severity, + location: Location(file: file, byteOffset: $0)) + } + } + + private func isTrailingClosure(dictionary: [String: SourceKitRepresentable], file: File) -> Bool { + guard let offset = dictionary.offset, + let length = dictionary.length, + case let start = min(offset, offset + length - 1), + let text = file.contents.bridge().substringWithByteRange(start: start, length: length) else { + return false + } + + return !text.hasSuffix(")") + } +} diff --git a/Source/SwiftLintFramework/Rules/MultilineArgumentsRuleExamples.swift b/Source/SwiftLintFramework/Rules/MultilineArgumentsRuleExamples.swift new file mode 100644 index 000000000..194343a46 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/MultilineArgumentsRuleExamples.swift @@ -0,0 +1,58 @@ +// +// MultilineArgumentsRuleExamples.swift +// SwiftLint +// +// Created by Marcel Jackwerth on 09/29/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import Foundation + +internal struct MultilineArgumentsRuleExamples { + static let nonTriggeringExamples = [ + "foo()", + "foo(\n" + + " \n" + + ")", + "foo { }", + "foo {\n" + + " \n" + + "}", + "foo(0)", + "foo(0, 1)", + "foo(0, 1) { }", + "foo(0, param1: 1)", + "foo(0, param1: 1) { }", + "foo(param1: 1)", + "foo(param1: 1) { }", + "foo(param1: 1, param2: true) { }", + "foo(param1: 1, param2: true, param3: [3]) { }", + "foo(param1: 1, param2: true, param3: [3]) {\n" + + " bar()\n" + + "}", + "foo(param1: 1,\n" + + " param2: true,\n" + + " param3: [3])", + "foo(\n" + + " param1: 1, param2: true, param3: [3]\n" + + ")", + "foo(\n" + + " param1: 1,\n" + + " param2: true,\n" + + " param3: [3]\n" + + ")" + ] + + static let triggeringExamples = [ + "foo(0,\n" + + " param1: 1, ↓param2: true, ↓param3: [3])", + "foo(0, ↓param1: 1,\n" + + " param2: true, ↓param3: [3])", + "foo(0, ↓param1: 1, ↓param2: true,\n" + + " param3: [3])", + "foo(\n" + + " 0, ↓param1: 1,\n" + + " param2: true, ↓param3: [3]\n" + + ")" + ] +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index b36fe4029..eb82b4308 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -105,6 +105,10 @@ 92CCB2D71E1EEFA300C8E5A3 /* UnusedOptionalBindingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 92CCB2D61E1EEFA300C8E5A3 /* UnusedOptionalBindingRule.swift */; }; 93E0C3CE1D67BD7F007FA25D /* ConditionalReturnsOnNewlineRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 93E0C3CD1D67BD7F007FA25D /* ConditionalReturnsOnNewlineRule.swift */; }; A1A6F3F21EE319ED00A9F9E2 /* ObjectLiteralConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = A1A6F3F11EE319ED00A9F9E2 /* ObjectLiteralConfiguration.swift */; }; + B25DCD0B1F7E9F9E0028A199 /* MultilineArgumentsRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = B25DCD091F7E9BB50028A199 /* MultilineArgumentsRuleExamples.swift */; }; + B25DCD0C1F7E9FA20028A199 /* MultilineArgumentsRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = B25DCD071F7E9B5F0028A199 /* MultilineArgumentsRule.swift */; }; + B25DCD0E1F7EF2280028A199 /* MultilineArgumentsConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = B25DCD0D1F7EF2280028A199 /* MultilineArgumentsConfiguration.swift */; }; + B25DCD101F7EF6DC0028A199 /* MultilineArgumentsRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B25DCD0F1F7EF6DC0028A199 /* MultilineArgumentsRuleTests.swift */; }; B2902A0C1D66815600BFCCF7 /* PrivateUnitTestRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */; }; B2902A0E1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */; }; B3935371E92E0CF3F7668303 /* CannedJunitReporterOutput.xml in Resources */ = {isa = PBXBuildFile; fileRef = B39359A325FE84B7EDD1C455 /* CannedJunitReporterOutput.xml */; }; @@ -428,6 +432,10 @@ 92CCB2D61E1EEFA300C8E5A3 /* UnusedOptionalBindingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedOptionalBindingRule.swift; sourceTree = ""; }; 93E0C3CD1D67BD7F007FA25D /* ConditionalReturnsOnNewlineRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ConditionalReturnsOnNewlineRule.swift; sourceTree = ""; }; A1A6F3F11EE319ED00A9F9E2 /* ObjectLiteralConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ObjectLiteralConfiguration.swift; sourceTree = ""; }; + B25DCD071F7E9B5F0028A199 /* MultilineArgumentsRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultilineArgumentsRule.swift; sourceTree = ""; }; + B25DCD091F7E9BB50028A199 /* MultilineArgumentsRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultilineArgumentsRuleExamples.swift; sourceTree = ""; }; + B25DCD0D1F7EF2280028A199 /* MultilineArgumentsConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultilineArgumentsConfiguration.swift; sourceTree = ""; }; + B25DCD0F1F7EF6DC0028A199 /* MultilineArgumentsRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultilineArgumentsRuleTests.swift; sourceTree = ""; }; B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateUnitTestRule.swift; sourceTree = ""; }; B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateUnitTestConfiguration.swift; sourceTree = ""; }; B3935001033261E5A70CE101 /* CannedEmojiReporterOutput.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = CannedEmojiReporterOutput.txt; sourceTree = ""; }; @@ -889,6 +897,7 @@ 3B63D46E1E1F09DF0057BE35 /* LineLengthRuleTests.swift */, D4C27BFF1E12DFF500DF713E /* LinterCacheTests.swift */, D4CA758E1E2DEEA500A40E8A /* NumberSeparatorRuleTests.swift */, + B25DCD0F1F7EF6DC0028A199 /* MultilineArgumentsRuleTests.swift */, 825F19D01EEFF19700969EF1 /* ObjectLiteralRuleTests.swift */, D4246D6E1F30DB260097E658 /* PrivateOverFilePrivateRuleTests.swift */, E81ADD711ED5ED9D000CD451 /* RegionTests.swift */, @@ -1023,6 +1032,9 @@ C946FEC91EAE5E20007DD778 /* LetVarWhitespaceRule.swift */, E88DEA7B1B098D7D00A66CB0 /* LineLengthRule.swift */, 856651A61D6B395F005E6B29 /* MarkRule.swift */, + B25DCD0D1F7EF2280028A199 /* MultilineArgumentsConfiguration.swift */, + B25DCD071F7E9B5F0028A199 /* MultilineArgumentsRule.swift */, + B25DCD091F7E9BB50028A199 /* MultilineArgumentsRuleExamples.swift */, 6238AE411ED4D734006C3601 /* MultilineParametersRule.swift */, 621061BE1ED57E640082D51E /* MultilineParametersRuleExamples.swift */, BB00B4E71F5216070079869F /* MultipleClosuresWithTrailingClosureRule.swift */, @@ -1403,6 +1415,7 @@ 623E36F01F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift in Sources */, BFF028AE1CBCF8A500B38A9D /* TrailingWhitespaceConfiguration.swift in Sources */, 3B034B6E1E0BE549005D49A9 /* LineLengthConfiguration.swift in Sources */, + B25DCD0C1F7E9FA20028A199 /* MultilineArgumentsRule.swift in Sources */, D4C4A34C1DEA4FF000E0E04C /* AttributesConfiguration.swift in Sources */, 83D71E281B131ECE000395DE /* RuleDescription.swift in Sources */, D4470D571EB69225008A1B2E /* ImplicitReturnRule.swift in Sources */, @@ -1515,12 +1528,14 @@ E88DEA771B098D0C00A66CB0 /* Rule.swift in Sources */, D47079AB1DFDCF7A00027086 /* SwiftExpressionKind.swift in Sources */, 00B8D9791E2D1223004E0EEC /* LegacyConstantRuleExamples.swift in Sources */, + B25DCD0E1F7EF2280028A199 /* MultilineArgumentsConfiguration.swift in Sources */, 24B4DF0D1D6DFDE90097803B /* RedundantNilCoalescingRule.swift in Sources */, D4130D971E16183F00242361 /* IdentifierNameRuleExamples.swift in Sources */, 7250948A1D0859260039B353 /* StatementPositionConfiguration.swift in Sources */, E81619531BFC162C00946723 /* QueuedPrint.swift in Sources */, E87E4A051BFB927C00FCFE46 /* TrailingSemicolonRule.swift in Sources */, D4B472411F66486300BD6EF1 /* FallthroughRule.swift in Sources */, + B25DCD0B1F7E9F9E0028A199 /* MultilineArgumentsRuleExamples.swift in Sources */, E88198421BEA929F00333A11 /* NestingRule.swift in Sources */, D46A317F1F1CEDCD00AF914A /* UnneededParenthesesInClosureArgumentRule.swift in Sources */, D4470D591EB6B4D1008A1B2E /* EmptyEnumArgumentsRule.swift in Sources */, @@ -1604,6 +1619,7 @@ 3B63D46D1E1F05160057BE35 /* LineLengthConfigurationTests.swift in Sources */, 6C7045441C6ADA450003F15A /* SourceKitCrashTests.swift in Sources */, D4246D6F1F30DB260097E658 /* PrivateOverFilePrivateRuleTests.swift in Sources */, + B25DCD101F7EF6DC0028A199 /* MultilineArgumentsRuleTests.swift in Sources */, 3BB47D871C51DE6E00AE6A10 /* CustomRulesTests.swift in Sources */, E812249A1B04F85B001783D2 /* TestHelpers.swift in Sources */, 3B20CD0C1EB699C20069EF2E /* TypeNameRuleTests.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 58f38afd7..9bd3e9be9 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -253,6 +253,14 @@ extension LinterCacheTests { ] } +extension MultilineArgumentsRuleTests { + static var allTests: [(String, (MultilineArgumentsRuleTests) -> () throws -> Void)] = [ + ("testMultilineArgumentsWithDefaultConfiguration", testMultilineArgumentsWithDefaultConfiguration), + ("testMultilineArgumentsWithWithNextLine", testMultilineArgumentsWithWithNextLine), + ("testMultilineArgumentsWithWithSameLine", testMultilineArgumentsWithWithSameLine) + ] +} + extension NumberSeparatorRuleTests { static var allTests: [(String, (NumberSeparatorRuleTests) -> () throws -> Void)] = [ ("testNumberSeparatorWithDefaultConfiguration", testNumberSeparatorWithDefaultConfiguration), @@ -533,6 +541,7 @@ XCTMain([ testCase(LineLengthConfigurationTests.allTests), testCase(LineLengthRuleTests.allTests), testCase(LinterCacheTests.allTests), + testCase(MultilineArgumentsRuleTests.allTests), testCase(NumberSeparatorRuleTests.allTests), testCase(ObjectLiteralRuleTests.allTests), testCase(PrivateOverFilePrivateRuleTests.allTests), diff --git a/Tests/SwiftLintFrameworkTests/MultilineArgumentsRuleTests.swift b/Tests/SwiftLintFrameworkTests/MultilineArgumentsRuleTests.swift new file mode 100644 index 000000000..2a0802ea9 --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/MultilineArgumentsRuleTests.swift @@ -0,0 +1,69 @@ +// +// MultilineArgumentsRuleTests.swift +// SwiftLint +// +// Created by Marcel Jackwerth on 09/29/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import SwiftLintFramework +import XCTest + +class MultilineArgumentsRuleTests: XCTestCase { + + func testMultilineArgumentsWithDefaultConfiguration() { + verifyRule(MultilineArgumentsRule.description) + } + + func testMultilineArgumentsWithWithNextLine() { + let nonTriggeringExamples = [ + "foo()", + "foo(0)", + "foo(1, bar: baz) { }", + "foo(2, bar: baz) {\n}", + "foo(\n" + + " 3,\n" + + " bar: baz) { }", + "foo(\n" + + " 4, bar: baz) { }" + ] + + let triggeringExamples = [ + "foo(↓1,\n" + + " bar: baz) { }" + ] + + let description = MultilineArgumentsRule.description + .with(triggeringExamples: triggeringExamples) + .with(nonTriggeringExamples: nonTriggeringExamples) + + verifyRule(description, ruleConfiguration: ["first_argument_location": "next_line"]) + } + + func testMultilineArgumentsWithWithSameLine() { + let nonTriggeringExamples = [ + "foo()", + "foo(0)", + "foo(1, bar: 1) { }", + "foo(2, bar: 2) {\n" + + " bar()\n" + + "}", + "foo(3,\n" + + " bar: 3) { }" + ] + + let triggeringExamples = [ + "foo(\n" + + " ↓1, ↓bar: baz) { }", + "foo(\n" + + " ↓2,\n" + + " bar: baz) { }" + ] + + let description = MultilineArgumentsRule.description + .with(triggeringExamples: triggeringExamples) + .with(nonTriggeringExamples: nonTriggeringExamples) + + verifyRule(description, ruleConfiguration: ["first_argument_location": "same_line"]) + } +}