Rewrite multiline_literal_brackets using SwiftSyntax (realm#5333)

mildm8nnered · Nov 19, 2023 · ff1e966 · ff1e966
1 parent 3511512
commit ff1e966
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,10 @@
   [SimplyDanny](https://github.com/SimplyDanny)
   [#1762](https://github.com/realm/SwiftLint/pull/1762)
 
+* Rewrite `multiline_literal_brackets` rule using SwiftSyntax, fixing some
+  false positives that would happen when comments are present.  
+  [Marcelo Fabri](https://github.com/marcelofabri)
+
 #### Bug Fixes
 
 * Ignore overridden functions with default parameters in the `unneeded_override`

diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/MultilineLiteralBracketsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/MultilineLiteralBracketsRule.swift
@@ -1,7 +1,8 @@
 import Foundation
-import SourceKittenFramework
+import SwiftSyntax
 
-struct MultilineLiteralBracketsRule: ASTRule, OptInRule {
+@SwiftSyntaxRule
+struct MultilineLiteralBracketsRule: OptInRule {
     var configuration = SeverityConfiguration<Self>(.warning)
 
     static let description = RuleDescription(
@@ -97,43 +98,79 @@ struct MultilineLiteralBracketsRule: ASTRule, OptInRule {
                      4, 5, 6,
                      7, 8, 9
                 ]
+            """),
+            Example("""
+            class Hogwarts {
+                let houseCup = [
+                    "gryffindor": 460, "hufflepuff": 370,
+                    "ravenclaw": 410, "slytherin": slytherinPoints.filter {
+                        $0.isValid
+                    }.sum()↓]
+            }
             """)
         ]
     )
+}
 
-    func validate(file: SwiftLintFile,
-                  kind: SwiftExpressionKind,
-                  dictionary: SourceKittenDictionary) -> [StyleViolation] {
-        guard
-            [.array, .dictionary].contains(kind),
-            let bodyByteRange = dictionary.bodyByteRange,
-            let body = file.stringView.substringWithByteRange(bodyByteRange)
-        else {
-            return []
+private extension MultilineLiteralBracketsRule {
+    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
+        override func visitPost(_ node: ArrayExprSyntax) {
+            validate(
+                node,
+                openingToken: node.leftSquare,
+                closingToken: node.rightSquare,
+                firstElement: node.elements.first?.expression,
+                lastElement: node.elements.last?.expression
+            )
         }
 
-        let isMultiline = body.contains("\n")
-        guard isMultiline else {
-            return []
+        override func visitPost(_ node: DictionaryExprSyntax) {
+            switch node.content {
+            case .colon:
+                break
+            case .elements(let elements):
+                validate(
+                    node,
+                    openingToken: node.leftSquare,
+                    closingToken: node.rightSquare,
+                    firstElement: elements.first?.key,
+                    lastElement: elements.last?.value
+                )
+            }
         }
 
-        let expectedBodyBeginRegex = regex("\\A[ \\t]*\\n")
-        let expectedBodyEndRegex = regex("\\n[ \\t]*\\z")
+        private func validate(_ node: some ExprSyntaxProtocol,
+                              openingToken: TokenSyntax,
+                              closingToken: TokenSyntax,
+                              firstElement: (some ExprSyntaxProtocol)?,
+                              lastElement: (some ExprSyntaxProtocol)?) {
+            guard let firstElement, let lastElement,
+                    isMultiline(node) else {
+                return
+            }
+
+            if areOnTheSameLine(openingToken, firstElement) {
+                // don't skip trivia to keep violations in the same position as the legacy implementation
+                violations.append(firstElement.position)
+            }
 
-        var violatingByteOffsets = [ByteCount]()
-        if expectedBodyBeginRegex.firstMatch(in: body, options: [], range: body.fullNSRange) == nil {
-            violatingByteOffsets.append(bodyByteRange.location)
+            if areOnTheSameLine(lastElement, closingToken) {
+                violations.append(closingToken.positionAfterSkippingLeadingTrivia)
+            }
         }
 
-        if expectedBodyEndRegex.firstMatch(in: body, options: [], range: body.fullNSRange) == nil {
-            violatingByteOffsets.append(bodyByteRange.upperBound)
+        private func isMultiline(_ node: some ExprSyntaxProtocol) -> Bool {
+            let startLocation = locationConverter.location(for: node.positionAfterSkippingLeadingTrivia)
+            let endLocation = locationConverter.location(for: node.endPositionBeforeTrailingTrivia)
+
+            return endLocation.line > startLocation.line
         }
 
-        return violatingByteOffsets.map { byteOffset in
-            StyleViolation(
-                ruleDescription: Self.description, severity: configuration.severity,
-                location: Location(file: file, byteOffset: byteOffset)
-            )
+        private func areOnTheSameLine(_ first: some SyntaxProtocol, _ second: some SyntaxProtocol) -> Bool {
+            let firstLocation = locationConverter.location(for: first.endPositionBeforeTrailingTrivia)
+            let secondLocation = locationConverter.location(for: second.positionAfterSkippingLeadingTrivia)
+
+            return firstLocation.line == secondLocation.line
         }
     }
 }