Merge pull request #402 from DavidBrunow/fixMorePostfixPoundIfs

Fix more postfix pound if scenarios

Author: allevato

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -1396,11 +1396,24 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     }
 
     if isNestedInPostfixIfConfig(node: Syntax(node)) {
+      let breakToken: Token
+      let currentIfConfigDecl = node.parent?.parent?.as(IfConfigDeclSyntax.self)
+
+      if let currentIfConfigDecl = currentIfConfigDecl,
+         let tokenBeforeCurrentIfConfigDecl = currentIfConfigDecl.previousToken,
+         isNestedInIfConfig(node: Syntax(tokenBeforeCurrentIfConfigDecl)) ||
+          tokenBeforeCurrentIfConfigDecl.text == "}" {
+        breakToken = .break(.reset)
+      } else {
+        breakToken = .break
+        before(currentIfConfigDecl?.poundEndif, tokens: [.break])
+      }
+
       before(
         node.firstToken,
         tokens: [
           .printerControl(kind: .enableBreaking),
-          .break(.reset),
+          breakToken,
         ]
       )
     } else if let condition = node.condition {
@@ -3567,17 +3580,39 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
 }
 
 private func isNestedInPostfixIfConfig(node: Syntax) -> Bool {
-    var this: Syntax? = node
+  var this: Syntax? = node
 
-    while this?.parent != nil {
-      if this?.parent?.is(PostfixIfConfigExprSyntax.self) == true {
-        return true
-      }
+  while this?.parent != nil {
+    // This guard handles the situation where a type with its own modifiers
+    // is nested inside of an if config. That type should not count as being
+    // in a postfix if config because its entire body is inside the if config.
+    if this?.is(TupleExprElementSyntax.self) == true {
+      return false
+    }
 
-      this = this?.parent
+    if this?.is(IfConfigDeclSyntax.self) == true &&
+        this?.parent?.is(PostfixIfConfigExprSyntax.self) == true {
+      return true
     }
 
-    return false
+    this = this?.parent
+  }
+
+  return false
+}
+
+private func isNestedInIfConfig(node: Syntax) -> Bool {
+  var this: Syntax? = node
+
+  while this?.parent != nil {
+    if this?.is(IfConfigClauseSyntax.self) == true {
+      return true
+    }
+
+    this = this?.parent
+  }
+
+  return false
 }
 
 extension Syntax {

Tests/SwiftFormatPrettyPrintTests/IfConfigTests.swift

@@ -247,10 +247,10 @@ final class IfConfigTests: PrettyPrintTestCase {
       """
       VStack {
         Text("something")
-        #if os(iOS)
-          .iOSSpecificModifier()
-        #endif
-        .commonModifier()
+          #if os(iOS)
+            .iOSSpecificModifier()
+          #endif
+          .commonModifier()
       }
 
       """
@@ -277,13 +277,13 @@ final class IfConfigTests: PrettyPrintTestCase {
       """
       VStack {
         Text("something")
-        #if os(iOS)
-          .iOSSpecificModifier()
-          .anotherModifier()
-          .anotherAnotherModifier()
-        #endif
-        .commonModifier()
-        .anotherCommonModifier()
+          #if os(iOS)
+            .iOSSpecificModifier()
+            .anotherModifier()
+            .anotherAnotherModifier()
+          #endif
+          .commonModifier()
+          .anotherCommonModifier()
       }
 
       """
@@ -311,14 +311,14 @@ final class IfConfigTests: PrettyPrintTestCase {
       """
       VStack {
         Text("something")
-        #if os(iOS) || os(watchOS)
-          #if os(iOS)
-            .iOSModifier()
-          #else
-            .watchOSModifier()
+          #if os(iOS) || os(watchOS)
+            #if os(iOS)
+              .iOSModifier()
+            #else
+              .watchOSModifier()
+            #endif
+            .iOSAndWatchOSModifier()
           #endif
-          .iOSAndWatchOSModifier()
-        #endif
       }
 
       """
@@ -343,10 +343,10 @@ final class IfConfigTests: PrettyPrintTestCase {
       """
       VStack {
         textView
-        #if os(iOS)
-          .iOSSpecificModifier()
-        #endif
-        .commonModifier()
+          #if os(iOS)
+            .iOSSpecificModifier()
+          #endif
+          .commonModifier()
       }
 
       """
@@ -390,4 +390,68 @@ final class IfConfigTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
   }
+
+  func testPostfixPoundIfBetweenOtherModifiers() {
+    let input =
+      """
+      EmptyView()
+        .padding([.vertical])
+      #if os(iOS)
+        .iOSSpecificModifier()
+      #endif
+        .commonModifier()
+      """
+
+    let expected =
+      """
+      EmptyView()
+        .padding([.vertical])
+        #if os(iOS)
+          .iOSSpecificModifier()
+        #endif
+        .commonModifier()
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
+  }
+
+  func testPostfixPoundIfWithTypeInModifier() {
+    let input =
+      """
+      EmptyView()
+        .padding([.vertical])
+      #if os(iOS)
+        .iOSSpecificModifier(
+          SpecificType()
+            .onChanged { _ in
+              // do things
+            }
+            .onEnded { _ in
+              // do things
+            }
+        )
+      #endif
+      """
+
+    let expected =
+      """
+      EmptyView()
+        .padding([.vertical])
+        #if os(iOS)
+          .iOSSpecificModifier(
+            SpecificType()
+              .onChanged { _ in
+                // do things
+              }
+              .onEnded { _ in
+                // do things
+              }
+          )
+        #endif
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
+  }
 }