Disable breaks inside of #if conditionals.

Adding line breaks inside of #if conditionals is dangerous because line breaks are only valid if the entire condition is wrapped in parens. It's difficult to conditionally add parens only if a break is necessary because it implies backtracking into the printer's output buffer to insert the opening paren.

Our options for fixing this problem were:
1. Implement paren wrapping the #if condition if a break fires (very complex).
2. Always paren wrap the #if condition (looks bad in the most common case where there's no breaking).
3. Disable breaking in the pretty print but allow users to explicitly break in the condition using discretionary newlines.

I selected option swiftlang#3 and implemented by adding new kind of token known as "printer control" to disable/enable suppressing of break tokens. These printer control tokens can be used in other scenarios, such as suppressing breaks in string interpolation expressions.

Fixes SR-11815

Author: dylansturg

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -96,6 +96,15 @@ public class PrettyPrinter {
   /// `currentLineIsContinuation` to become true).
   private var wasLastBreakKindContinue = false
 
+  /// Tracks how many printer control tokens to suppress firing breaks are active.
+  private var activeBreakSuppressionCount = 0
+
+  /// Whether breaks are supressed from firing. When true, no breaks should fire and the only way to
+  /// move to a new line is an explicit new line token.
+  private var isBreakingSupressed: Bool {
+    return activeBreakSuppressionCount > 0
+  }
+
   /// The computed indentation level, as a number of spaces, based on the state of any unclosed
   /// delimiters and whether or not the current line is a continuation line.
   private var currentIndentation: [Indent] {
@@ -283,6 +292,7 @@ public class PrettyPrinter {
         // scope. Additionally, continuation open breaks must indent when the break fires.
         let continuationBreakWillFire = openKind == .continuation
           && (isAtStartOfLine || length > spaceRemaining || mustBreak)
+          && !isBreakingSupressed
         let contributesContinuationIndent = currentLineIsContinuation || continuationBreakWillFire
 
         activeOpenBreaks.append(
@@ -387,7 +397,7 @@ public class PrettyPrinter {
         mustBreak = currentLineIsContinuation
       }
 
-      if (!isAtStartOfLine && length > spaceRemaining) || mustBreak {
+      if !isBreakingSupressed && ((!isAtStartOfLine && length > spaceRemaining) || mustBreak) {
         currentLineIsContinuation = isContinuationIfBreakFires
         writeNewlines(1, kind: .flexible)
         lastBreak = true
@@ -451,6 +461,14 @@ public class PrettyPrinter {
       pendingSpaces = 0
       lastBreak = false
       spaceRemaining -= length
+
+    case .printerControl(let kind):
+      switch kind {
+      case .disableBreaking:
+        activeBreakSuppressionCount += 1
+      case .enableBreaking:
+        activeBreakSuppressionCount -= 1
+      }
     }
   }
 
@@ -554,6 +572,10 @@ public class PrettyPrinter {
         }
         lengths.append(length)
         total += length
+
+      case .printerControl:
+        // Control tokens have no length. They aren't printed.
+        lengths.append(0)
       }
     }
 
@@ -642,6 +664,9 @@ public class PrettyPrinter {
       printDebugIndent()
       print("[VERBATIM Length: \(length) Idx: \(idx)]")
       print(verbatim.print(indent: debugIndent))
+
+    case .printerControl(let kind):
+      print("[PRINTER CONTROL Kind: \(kind)]")
     }
   }
 

Sources/SwiftFormatPrettyPrint/Token.swift

@@ -141,6 +141,21 @@ enum NewlineKind {
   case mandatory
 }
 
+/// Kinds of printer control tokens that can be used to customize the pretty printer's behavior in
+/// a subsection of a token stream.
+enum PrinterControlKind {
+  /// A signal that break tokens shouldn't be allowed to fire until a corresponding enable breaking
+  /// control token is encountered.
+  ///
+  /// It's valid to nest `disableBreaking` and `enableBreaking` tokens. Breaks will be suppressed
+  /// long as there is at least 1 unmatched disable token.
+  case disableBreaking
+
+  /// A signal that break tokens should be allowed to fire following this token, as long as there
+  /// are no other unmatched disable tokens.
+  case enableBreaking
+}
+
 enum Token {
   case syntax(String)
   case open(GroupBreakStyle)
@@ -150,6 +165,7 @@ enum Token {
   case newlines(Int, kind: NewlineKind)
   case comment(Comment, wasEndOfLine: Bool)
   case verbatim(Verbatim)
+  case printerControl(kind: PrinterControlKind)
 
   // Convenience overloads for the enum types
   static let open = Token.open(.inconsistent, 0)

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -956,6 +956,12 @@ private final class TokenStreamCreator: SyntaxVisitor {
     } else {
       before(tokenToOpenWith.nextToken, tokens: .break(breakKindClose), .newline, .close)
     }
+
+    if let condition = node.condition {
+      before(condition.firstToken, tokens: .printerControl(kind: .disableBreaking))
+      after(condition.lastToken, tokens: .printerControl(kind: .enableBreaking), .break(.reset, size: 0))
+    }
+
     return .visitChildren
   }
 

Tests/SwiftFormatPrettyPrintTests/IfConfigTests.swift

@@ -141,4 +141,93 @@ public class IfConfigTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
   }
+
+  public func testPrettyPrintLineBreaksDisabled() {
+    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)
+  }
+
+  public 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)
+  }
+
+  public func testValidDiscretionaryLineBreaksRetained() {
+    let input =
+      """
+      #if (canImport(SwiftUI)
+      && !(os(iOS)
+      && arch(arm))
+      && ((canImport(AppKit)
+      || canImport(UIKit)) && !os(watchOS))
+      && canImport(Foundation))
+        conditionalFunc(foo, bar, baz)
+      #endif
+
+      #if (canImport(SwiftUI)
+        && os(iOS)
+        && arch(arm)
+        && canImport(AppKit)
+        || canImport(UIKit) && !os(watchOS)
+        && canImport(Foundation))
+        conditionalFunc(foo, bar, baz)
+      #endif
+      """
+
+    let expected =
+      """
+      #if (canImport(SwiftUI)
+        && !(os(iOS)
+          && arch(arm))
+          && ((canImport(AppKit)
+            || canImport(UIKit)) && !os(watchOS))
+          && canImport(Foundation))
+        conditionalFunc(foo, bar, baz)
+      #endif
+
+      #if (canImport(SwiftUI)
+        && os(iOS)
+          && arch(arm)
+          && canImport(AppKit)
+          || canImport(UIKit) && !os(watchOS)
+          && canImport(Foundation))
+        conditionalFunc(foo, bar, baz)
+      #endif
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 40)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift

@@ -313,7 +313,10 @@ extension IfConfigTests {
     static let __allTests__IfConfigTests = [
         ("testBasicIfConfig", testBasicIfConfig),
         ("testIfConfigNoIndentation", testIfConfigNoIndentation),
+        ("testInvalidDiscretionaryLineBreaksRemoved", testInvalidDiscretionaryLineBreaksRemoved),
         ("testPoundIfAroundMembers", testPoundIfAroundMembers),
+        ("testPrettyPrintLineBreaksDisabled", testPrettyPrintLineBreaksDisabled),
+        ("testValidDiscretionaryLineBreaksRetained", testValidDiscretionaryLineBreaksRetained),
     ]
 }