Rewrite MultilineArgumentsRule using SwiftSyntax (#4750)

* Rewrite MultilineArgumentsRule using SwiftSyntax

* Add examples and changelog

Fixes #3399, #3605

CHANGELOG.md

@@ -55,6 +55,12 @@
   [SimplyDanny](https://github.com/SimplyDanny)
   [#4637](https://github.com/realm/SwiftLint/issues/4637)
 
+* Rewrite `multiline_arguments` rule using SwiftSyntax, ignoring trailing
+  closures.  
+  [Marcelo Fabri](https://github.com/marcelofabri)
+  [#3399](https://github.com/realm/SwiftLint/issues/3399)
+  [#3605](https://github.com/realm/SwiftLint/issues/3605)
+
 #### Bug Fixes
 
 * Report violations in all `<scope>_length` rules when the error threshold is

Source/SwiftLintFramework/Rules/RuleConfigurations/MultilineArgumentsConfiguration.swift

@@ -4,7 +4,7 @@ private enum ConfigurationKey: String {
     case onlyEnforceAfterFirstClosureOnFirstLine = "only_enforce_after_first_closure_on_first_line"
 }
 
-struct MultilineArgumentsConfiguration: RuleConfiguration, Equatable {
+struct MultilineArgumentsConfiguration: RuleConfiguration, SeverityBasedRuleConfiguration, Equatable {
     enum FirstArgumentLocation: String {
         case anyLine = "any_line"
         case sameLine = "same_line"

Source/SwiftLintFramework/Rules/Style/MultilineArgumentsRule.swift

@@ -1,7 +1,7 @@
 import Foundation
-import SourceKittenFramework
+import SwiftSyntax
 
-struct MultilineArgumentsRule: ASTRule, OptInRule, ConfigurationProviderRule {
+struct MultilineArgumentsRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule {
     var configuration = MultilineArgumentsConfiguration()
 
     init() {}
@@ -15,147 +15,122 @@ struct MultilineArgumentsRule: ASTRule, OptInRule, ConfigurationProviderRule {
         triggeringExamples: MultilineArgumentsRuleExamples.triggeringExamples
     )
 
-    func validate(file: SwiftLintFile,
+    func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
-                  kind: SwiftExpressionKind,
+        Visitor(
-                  dictionary: SourceKittenDictionary) -> [StyleViolation] {
+            onlyEnforceAfterFirstClosureOnFirstLine: configuration.onlyEnforceAfterFirstClosureOnFirstLine,
-        guard
+            firstArgumentLocation: configuration.firstArgumentLocation,
-            kind == .call,
+            locationConverter: file.locationConverter
-            case let arguments = dictionary.enclosedArguments,
+        )
-            arguments.count > 1
+    }
-        else {
+}
-            return []
+
+private extension MultilineArgumentsRule {
+    final class Visitor: ViolationsSyntaxVisitor {
+        let onlyEnforceAfterFirstClosureOnFirstLine: Bool
+        let firstArgumentLocation: MultilineArgumentsConfiguration.FirstArgumentLocation
+        let locationConverter: SourceLocationConverter
+
+        init(onlyEnforceAfterFirstClosureOnFirstLine: Bool,
+             firstArgumentLocation: MultilineArgumentsConfiguration.FirstArgumentLocation,
+             locationConverter: SourceLocationConverter) {
+            self.onlyEnforceAfterFirstClosureOnFirstLine = onlyEnforceAfterFirstClosureOnFirstLine
+            self.firstArgumentLocation = firstArgumentLocation
+            self.locationConverter = locationConverter
+            super.init(viewMode: .sourceAccurate)
         }
 
-        let wrappedArguments: [Argument] = arguments
+        override func visitPost(_ node: FunctionCallExprSyntax) {
-            .enumerated()
+            guard node.argumentList.count > 1,
-            .compactMap { idx, argument in
+                  case let functionCallPosition = node.calledExpression.positionAfterSkippingLeadingTrivia,
-                Argument(dictionary: argument, file: file, index: idx)
+                  let functionCallLine = locationConverter.location(for: functionCallPosition).line else {
+                return
             }
 
-        var violatingArguments = findViolations(in: wrappedArguments,
+            let wrappedArguments: [Argument] = node.argumentList
-                                                dictionary: dictionary,
+                .enumerated()
-                                                file: file)
+                .compactMap { idx, argument in
-
+                    Argument(element: argument, locationConverter: locationConverter, index: idx)
-        if configuration.onlyEnforceAfterFirstClosureOnFirstLine {
-            violatingArguments = removeViolationsBeforeFirstClosure(arguments: wrappedArguments,
-                                                                    violations: violatingArguments,
-                                                                    file: file)
-        }
-
-        return violatingArguments.map {
-            return StyleViolation(ruleDescription: Self.description,
-                                  severity: self.configuration.severityConfiguration.severity,
-                                  location: Location(file: file, byteOffset: $0.offset))
-        }
-    }
-
-    // MARK: - Violation Logic
-
-    private func findViolations(in arguments: [Argument],
-                                dictionary: SourceKittenDictionary,
-                                file: SwiftLintFile) -> [Argument] {
-        guard case let contents = file.stringView,
-            let nameOffset = dictionary.nameOffset,
-            let (nameLine, _) = contents.lineAndCharacter(forByteOffset: nameOffset)
-        else {
-            return []
-        }
-
-        var visitedLines = Set<Int>()
-
-        if configuration.firstArgumentLocation == .sameLine {
-            visitedLines.insert(nameLine)
-        }
-
-        let lastIndex = arguments.count - 1
-
-        let violations = arguments.compactMap { argument -> Argument? in
-            let (line, idx) = (argument.line, argument.index)
-            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 : argument
-                case .sameLine: return line > nameLine ? argument : nil
                 }
-            } else {
+
-                return firstVisit ? nil : argument
+            var violatingArguments = findViolations(in: wrappedArguments, functionCallLine: functionCallLine)
+
+            if onlyEnforceAfterFirstClosureOnFirstLine {
+                violatingArguments = removeViolationsBeforeFirstClosure(arguments: wrappedArguments,
+                                                                        violations: violatingArguments)
             }
+
+            violations.append(contentsOf: violatingArguments.map(\.offset))
         }
 
-        // only report violations if multiline
+        // MARK: - Violation Logic
-        return visitedLines.count > 1 ? violations : []
-    }
 
-    private func removeViolationsBeforeFirstClosure(arguments: [Argument],
+        private func findViolations(in arguments: [Argument],
-                                                    violations: [Argument],
+                                    functionCallLine: Int) -> [Argument] {
-                                                    file: SwiftLintFile) -> [Argument] {
+            var visitedLines = Set<Int>()
-        guard let firstClosure = arguments.first(where: isClosure(in: file)),
+
-            let firstArgument = arguments.first else {
+            if firstArgumentLocation == .sameLine {
-            return violations
+                visitedLines.insert(functionCallLine)
+            }
+
+            let violations = arguments.compactMap { argument -> Argument? in
+                let (line, idx) = (argument.line, argument.index)
+                let (firstVisit, _) = visitedLines.insert(line)
+
+               if idx == 0 {
+                    switch firstArgumentLocation {
+                    case .anyLine: return nil
+                    case .nextLine: return line > functionCallLine ? nil : argument
+                    case .sameLine: return line > functionCallLine ? argument : nil
+                    }
+                } else {
+                    return firstVisit ? nil : argument
+                }
+            }
+
+            // only report violations if multiline
+            return visitedLines.count > 1 ? violations : []
         }
 
-        let violationSlice: ArraySlice<Argument> = violations
+        private func removeViolationsBeforeFirstClosure(arguments: [Argument],
-            .drop { argument in
+                                                        violations: [Argument]) -> [Argument] {
-                // drop violations if they precede the first closure,
+            guard let firstClosure = arguments.first(where: { $0.isClosure }),
-                // if that closure is in the first line
+                  let firstArgument = arguments.first else {
-                firstArgument.line == firstClosure.line &&
+                return violations
+            }
+
+            let violationSlice: ArraySlice<Argument> = violations
+                .drop { argument in
+                    // drop violations if they precede the first closure,
+                    // if that closure is in the first line
+                    firstArgument.line == firstClosure.line &&
                     argument.line == firstClosure.line &&
                     argument.index <= firstClosure.index
-            }
+                }
 
-        return Array(violationSlice)
+            return Array(violationSlice)
-    }
-
-    // MARK: - Syntax Helpers
-
-    private func isTrailingClosure(dictionary: SourceKittenDictionary, file: SwiftLintFile) -> Bool {
-        guard let offset = dictionary.offset,
-            let length = dictionary.length,
-            case let start = min(offset, offset + length - 1),
-            case let byteRange = ByteRange(location: start, length: length),
-            let text = file.stringView.substringWithByteRange(byteRange)
-        else {
-            return false
-        }
-
-        return !text.hasSuffix(")")
-    }
-
-    private func isClosure(in file: SwiftLintFile) -> (Argument) -> Bool {
-        return { argument in
-            let contents = file.stringView
-            let closureMatcher = regex("^\\s*\\{")
-            guard let range = contents.byteRangeToNSRange(argument.bodyRange) else {
-                return false
-            }
-
-            let matches = closureMatcher.matches(in: file.contents, options: [], range: range)
-            return matches.count == 1
         }
     }
 }
 
 private struct Argument {
-    let offset: ByteCount
+    let offset: AbsolutePosition
     let line: Int
     let index: Int
-    let bodyRange: ByteRange
+    let expression: ExprSyntax
 
-    init?(dictionary: SourceKittenDictionary, file: SwiftLintFile, index: Int) {
+    init?(element: TupleExprElementSyntax, locationConverter: SourceLocationConverter, index: Int) {
-        guard let offset = dictionary.offset,
+        let offset = element.positionAfterSkippingLeadingTrivia
-            let (line, _) = file.stringView.lineAndCharacter(forByteOffset: offset),
+        guard let line = locationConverter.location(for: offset).line else {
-            let bodyRange = dictionary.bodyByteRange
-        else {
             return nil
         }
 
         self.offset = offset
         self.line = line
         self.index = index
-        self.bodyRange = bodyRange
+        self.expression = element.expression
+    }
+
+    var isClosure: Bool {
+        expression.is(ClosureExprSyntax.self)
     }
 }

Source/SwiftLintFramework/Rules/Style/MultilineArgumentsRuleExamples.swift

@@ -37,6 +37,33 @@ internal struct MultilineArgumentsRuleExamples {
             param2: true,
             param3: [3]
         )
+        """),
+        Example(#"""
+        Picker(selection: viewStore.binding(\.$someProperty)) {
+           ForEach(SomeEnum.allCases, id: \.rawValue) { someCase in
+              Text(someCase.rawValue)
+                 .tag(someCase)
+           }
+        } label: {
+           EmptyView()
+        }
+        """#),
+        Example("""
+        UIView.animate(withDuration: 1,
+                       delay: 0) {
+            // sample
+            print("a")
+        } completion: { _ in
+            // sample
+            print("b")
+        }
+        """),
+        Example("""
+        UIView.animate(withDuration: 1, delay: 0) {
+            print("a")
+        } completion: { _ in
+            print("b")
+        }
         """)
     ]