Fix issue where for loop would break conditional assignment rule

nicklockwood · Nov 18, 2023 · 3bf9d4b · 3bf9d4b
1 parent 36344de
commit 3bf9d4b
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 7 deletions.
diff --git a/Sources/FormattingHelpers.swift b/Sources/FormattingHelpers.swift
@@ -1267,11 +1267,9 @@ extension Formatter {
     func startOfBody(atStartOfScope startOfScopeIndex: Int) -> Int {
         // If this is a closure that has an `in` clause, the body scope starts after that
         if isStartOfClosure(at: startOfScopeIndex),
-           let endOfScopeIndex = endOfScope(at: startOfScopeIndex),
-           let inToken = index(of: .keyword("in"), in: (startOfScopeIndex + 1) ..< endOfScopeIndex),
-           !indexIsWithinNestedClosure(inToken, startOfScopeIndex: startOfScopeIndex)
+           let inKeywordIndex = parseClosureArgumentList(at: startOfScopeIndex)?.inKeywordIndex
         {
-            return inToken
+            return inKeywordIndex
         } else {
             return startOfScopeIndex
         }

diff --git a/Sources/ParsingHelpers.swift b/Sources/ParsingHelpers.swift
@@ -1589,14 +1589,18 @@ extension Formatter {
                 tokens[indexAfterOpenBrace] == .startOfScope("("),
                 let endOfArgumentsScopeIndex = endOfScope(at: indexAfterOpenBrace),
                 let firstTokenInArgumentsList = index(of: .nonSpaceOrCommentOrLinebreak, after: indexAfterOpenBrace),
-                let lastTokenInArgumentsList = index(of: .nonSpaceOrCommentOrLinebreak, before: endOfArgumentsScopeIndex),
                 let indexAfterArguments = index(of: .nonSpaceOrCommentOrLinebreak, after: endOfArgumentsScopeIndex),
                 tokens[indexAfterArguments] == .keyword("in")
         {
             inKeywordIndex = indexAfterArguments
 
-            let argumentTokens = tokens[firstTokenInArgumentsList ... lastTokenInArgumentsList].split(separator: .delimiter(","))
-            argumentNames = argumentTokens.map { $0.first(where: \.isIdentifier)?.string ?? $0[0].string }
+            // This can be a completely empty argument list, like `{ () in ... }`.
+            if firstTokenInArgumentsList == endOfArgumentsScopeIndex {
+                return (argumentNames: [], inKeywordIndex: inKeywordIndex)
+            }
+
+            let argumentTokens = tokens[firstTokenInArgumentsList ... endOfArgumentsScopeIndex].split(separator: .delimiter(","))
+            argumentNames = argumentTokens.compactMap { $0.first(where: \.isIdentifier)?.string ?? $0[0].string }
         }
 
         // Otherwise this is an anonymous closure

diff --git a/Tests/RulesTests+Syntax.swift b/Tests/RulesTests+Syntax.swift
@@ -3884,6 +3884,22 @@ class SyntaxTests: RulesTests {
         testFormatting(for: input, output, rule: FormatRules.conditionalAssignment, options: options)
     }
 
+    func testDoesntConvertIfStatementWithForLoopInBranch() {
+        let input = """
+        var foo: Foo?
+        if condition {
+            foo = Foo("foo")
+            for foo in foos {
+                print(foo)
+            }
+        } else {
+            foo = Foo("bar")
+        }
+        """
+        let options = FormatOptions(swiftVersion: "5.9")
+        testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options)
+    }
+
     // MARK: - forLoop
 
     func testConvertSimpleForEachToForLoop() {