Merge pull request #554 from allevato/poundif-after-paren

Fix postfix-`#if` formatting when they come after a closing parenthesis.

Author: allevato

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -988,14 +988,22 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
 
   override func visit(_ node: MemberAccessExprSyntax) -> SyntaxVisitorContinueKind {
     preVisitInsertingContextualBreaks(node)
-
     return .visitChildren
   }
 
   override func visitPost(_ node: MemberAccessExprSyntax) {
     clearContextualBreakState(node)
   }
 
+  override func visit(_ node: PostfixIfConfigExprSyntax) -> SyntaxVisitorContinueKind {
+    preVisitInsertingContextualBreaks(node)
+    return .visitChildren
+  }
+
+  override func visitPost(_ node: PostfixIfConfigExprSyntax) {
+    clearContextualBreakState(node)
+  }
+
   override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
     preVisitInsertingContextualBreaks(node)
 
@@ -1456,29 +1464,10 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
       before(tokenToOpenWith.nextToken(viewMode: .all), tokens: .break(breakKindClose, newlines: .soft), .close)
     }
 
-    if isNestedInPostfixIfConfig(node: Syntax(node)) {
-      let breakToken: Token
-      let currentIfConfigDecl = node.parent?.parent?.as(IfConfigDeclSyntax.self)
-
-      if let currentIfConfigDecl = currentIfConfigDecl,
-         let tokenBeforeCurrentIfConfigDecl = currentIfConfigDecl.previousToken(viewMode: .sourceAccurate),
-         isNestedInIfConfig(node: Syntax(tokenBeforeCurrentIfConfigDecl)) ||
-          tokenBeforeCurrentIfConfigDecl.text == "}" {
-        breakToken = .break(.reset)
-      } else {
-        breakToken = .break
-        before(currentIfConfigDecl?.poundEndif, tokens: [.break])
-      }
-
+    if !isNestedInPostfixIfConfig(node: Syntax(node)), let condition = node.condition {
       before(
-        node.firstToken(viewMode: .sourceAccurate),
-        tokens: [
-          .printerControl(kind: .enableBreaking),
-          breakToken,
-        ]
-      )
-    } else if let condition = node.condition {
-      before(condition.firstToken(viewMode: .sourceAccurate), tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: true)))
+        condition.firstToken(viewMode: .sourceAccurate),
+        tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: true)))
       after(
         condition.lastToken(viewMode: .sourceAccurate),
         tokens: .printerControl(kind: .enableBreaking), .break(.reset, size: 0))
@@ -3304,7 +3293,11 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
   private func mustBreakBeforeClosingDelimiter<T: ExprSyntaxProtocol>(
     of expr: T, argumentListPath: KeyPath<T, TupleExprElementListSyntax>
   ) -> Bool {
-    guard let parent = expr.parent, parent.is(MemberAccessExprSyntax.self) else { return false }
+    guard
+      let parent = expr.parent,
+      parent.is(MemberAccessExprSyntax.self) || parent.is(PostfixIfConfigExprSyntax.self)
+    else { return false }
+
     let argumentList = expr[keyPath: argumentListPath]
 
     // When there's a single compact argument, there is no extra indentation for the argument and
@@ -3739,6 +3732,23 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
         after(expr.lastToken(viewMode: .sourceAccurate), tokens: .contextualBreakingEnd)
       }
       return (hasCompoundExpression, true)
+    } else if let postfixIfExpr = expr.as(PostfixIfConfigExprSyntax.self),
+      let base = postfixIfExpr.base
+    {
+      // For postfix-if expressions with bases (i.e., they aren't the first `#if` nested inside
+      // another `#if`), add contextual breaks before the top-level clauses (and the terminating
+      // `#endif`) so that they nest or line-up properly based on the preceding node. We don't do
+      // this for initial nested `#if`s because they will already get open/close breaks to control
+      // their indentation from their parent clause.
+      before(postfixIfExpr.firstToken(viewMode: .sourceAccurate), tokens: .contextualBreakingStart)
+      after(postfixIfExpr.lastToken(viewMode: .sourceAccurate), tokens: .contextualBreakingEnd)
+
+      for clause in postfixIfExpr.config.clauses {
+        before(clause.poundKeyword, tokens: .break(.contextual, size: 0))
+      }
+      before(postfixIfExpr.config.poundEndif, tokens: .break(.contextual, size: 0))
+
+      return insertContextualBreaks(base, isTopLevel: false)
     } else if let callingExpr = expr.asProtocol(CallingExprSyntaxProtocol.self) {
       let calledExpression = callingExpr.calledExpression
       let (hasCompoundExpression, hasMemberAccess) =
@@ -3805,20 +3815,6 @@ private func isNestedInPostfixIfConfig(node: Syntax) -> Bool {
   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 {
   /// Creates a pretty-printable token stream for the provided Syntax node.
   func makeTokenStream(configuration: Configuration, operatorTable: OperatorTable) -> [Token] {

Tests/SwiftFormatPrettyPrintTests/IfConfigTests.swift

@@ -163,25 +163,25 @@ final class IfConfigTests: PrettyPrintTestCase {
 
   func testInvalidDiscretionaryLineBreaksRemoved() {
     let input =
-         """
-         #if (canImport(SwiftUI) &&
-         !(os(iOS) &&
-          arch(arm)) &&
-            ((canImport(AppKit) ||
-         canImport(UIKit)) && !os(watchOS)))
-         conditionalFunc(foo, bar, baz)
-           #endif
-         """
-
-       let expected =
-         """
-         #if (canImport(SwiftUI) && !(os(iOS) && arch(arm)) && ((canImport(AppKit) || canImport(UIKit)) && !os(watchOS)))
-           conditionalFunc(foo, bar, baz)
-         #endif
-
-         """
-
-       assertPrettyPrintEqual(input: input, expected: expected, linelength: 40)
+      """
+      #if (canImport(SwiftUI) &&
+      !(os(iOS) &&
+       arch(arm)) &&
+         ((canImport(AppKit) ||
+      canImport(UIKit)) && !os(watchOS)))
+      conditionalFunc(foo, bar, baz)
+        #endif
+      """
+
+    let expected =
+      """
+      #if (canImport(SwiftUI) && !(os(iOS) && arch(arm)) && ((canImport(AppKit) || canImport(UIKit)) && !os(watchOS)))
+        conditionalFunc(foo, bar, baz)
+      #endif
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 40)
   }
 
   func testValidDiscretionaryLineBreaksRetained() {
@@ -299,6 +299,8 @@ final class IfConfigTests: PrettyPrintTestCase {
         #if os(iOS) || os(watchOS)
           #if os(iOS)
           .iOSModifier()
+          #elseif os(tvOS)
+          .tvOSModifier()
           #else
           .watchOSModifier()
           #endif
@@ -314,6 +316,8 @@ final class IfConfigTests: PrettyPrintTestCase {
           #if os(iOS) || os(watchOS)
             #if os(iOS)
               .iOSModifier()
+            #elseif os(tvOS)
+              .tvOSModifier()
             #else
               .watchOSModifier()
             #endif
@@ -326,7 +330,6 @@ final class IfConfigTests: PrettyPrintTestCase {
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
   }
 
-
   func testPostfixPoundIfAfterVariables() {
     let input =
       """
@@ -398,6 +401,7 @@ final class IfConfigTests: PrettyPrintTestCase {
         .padding([.vertical])
       #if os(iOS)
         .iOSSpecificModifier()
+        .anotherIOSSpecificModifier()
       #endif
         .commonModifier()
       """
@@ -408,6 +412,7 @@ final class IfConfigTests: PrettyPrintTestCase {
         .padding([.vertical])
         #if os(iOS)
           .iOSSpecificModifier()
+          .anotherIOSSpecificModifier()
         #endif
         .commonModifier()
 
@@ -454,4 +459,61 @@ final class IfConfigTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
   }
+
+  func testPostfixPoundIfNotIndentedIfClosingParenOnOwnLine() {
+    let input =
+      """
+      SomeFunction(
+        foo,
+        bar
+      )
+      #if os(iOS)
+      .iOSSpecificModifier()
+      #endif
+      .commonModifier()
+      """
+
+    let expected =
+      """
+      SomeFunction(
+        foo,
+        bar
+      )
+      #if os(iOS)
+        .iOSSpecificModifier()
+      #endif
+      .commonModifier()
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
+  }
+
+  func testPostfixPoundIfForcesPrecedingClosingParenOntoNewLine() {
+    let input =
+      """
+      SomeFunction(
+        foo,
+        bar)
+        #if os(iOS)
+        .iOSSpecificModifier()
+        #endif
+        .commonModifier()
+      """
+
+    let expected =
+      """
+      SomeFunction(
+        foo,
+        bar
+      )
+      #if os(iOS)
+        .iOSSpecificModifier()
+      #endif
+      .commonModifier()
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
+  }
 }