Merge pull request swiftlang#103 from dylansturg/stop_breaking_conditional_compilation

Disable breaks inside of #if conditionals.

Author: allevato

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),
     ]
 }