Merge newline token into break token.

The handling of separate newlines and breaks in the PrettyPrinter resulted in a fairly complex implementation. In particular, it required storing the most recent break, and mutating the indentation stack to retroactively "fire" a break. This has been a source of bugs.

This updates the TokenStreamCreator and PrettyPrinter to a model without an explicit newline token. Instead, newlines are encapsulated by breaks. The same types of newlines exist ("flexible", "discretionary", and "mandatory" newlines have equivalent counterparts), but they exist as associated data with a break. This means newlines always exist where breaks are by definition.

This refactor "broke" some cases where comments were working with the right indentation "by accident". In cases where a comment existed outside of any scoping tokens (i.e. open/close breaks, continue breaks, etc.), the PrettyPrinter carried over indentation of the last break even if that break wasn't really relevant. To fix this, I added `splitScopingBeforeTokens(of:)` which splits the before-tokens so that those that start a scope can be placed before comments when visiting a token. This fixed several existing comment indentation issues, and fixed comments whose indentation was only "accidentally" working thanks to the PrettyPrinter's break carry-over behavior.

Author: dylansturg

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -91,11 +91,6 @@ public class PrettyPrinter {
   /// Indicates whether or not the printer is currently at the beginning of a line.
   private var isAtStartOfLine = true
 
-  /// Indicates whether the kind of the last break was one that triggers a continuation line (i.e.,
-  /// a `.continue`, an `.open(.continuation)`, or a `.close` break that causes
-  /// `currentLineIsContinuation` to become true).
-  private var wasLastBreakKindContinue = false
-
   /// Tracks how many printer control tokens to suppress firing breaks are active.
   private var activeBreakSuppressionCount = 0
 
@@ -159,8 +154,8 @@ public class PrettyPrinter {
     outputBuffer.append(String(str))
   }
 
-  /// Ensures that the given number of newlines to the output stream (taking into account any
-  /// pre-existing consecutive newlines).
+  /// Writes newlines into the output stream, taking into account any pre-existing consecutive
+  /// newlines and the maximum allowed number of blank lines.
   ///
   /// This function does some implicit collapsing of consecutive newlines to ensure that the
   /// results are consistent when breaks and explicit newlines coincide. For example, imagine a
@@ -171,28 +166,20 @@ public class PrettyPrinter {
   /// subtract the previously written newlines during the second call so that we end up with the
   /// correct number overall.
   ///
-  /// - Parameters:
-  ///   - count: The number of newlines to write.
-  ///   - kind: Indicates whether the newlines are flexible, discretionary, or mandatory newlines.
-  ///     Refer to the documentation of `NewlineKind` for details on how each of these are printed.
-  private func writeNewlines(_ count: Int, kind: NewlineKind) {
-    // We add 1 because it takes 2 newlines to create a blank line.
+  /// - Parameter newlines: The number and type of newlines to write.
+  private func writeNewlines(_ newlines: NewlineBehavior) {
     let numberToPrint: Int
-    if kind == .mandatory {
+    switch newlines {
+    case .elective:
+      numberToPrint = consecutiveNewlineCount == 0 ? 1 : 0
+    case .soft(let count, _):
+      // We add 1 to the max blank lines because it takes 2 newlines to create the first blank line.
+      numberToPrint = min(count - consecutiveNewlineCount, configuration.maximumBlankLines + 1)
+    case .hard(let count):
       numberToPrint = count
-    } else {
-      let maximumNewlines = configuration.maximumBlankLines + 1
-      if count <= maximumNewlines {
-        numberToPrint = count - consecutiveNewlineCount
-      } else {
-        numberToPrint = maximumNewlines - consecutiveNewlineCount
-      }
-
-      guard (kind == .discretionary && numberToPrint > 0) || consecutiveNewlineCount == 0 else {
-        return
-      }
     }
 
+    guard numberToPrint > 0 else { return }
     writeRaw(String(repeating: "\n", count: numberToPrint))
     lineNumber += numberToPrint
     isAtStartOfLine = true
@@ -259,8 +246,7 @@ public class PrettyPrinter {
 
     // Create a line break if needed. Calculate the indentation required and adjust spaceRemaining
     // accordingly.
-    case .break(let kind, let size, _):
-      wasLastBreakKindContinue = false
+    case .break(let kind, let size, let newline):
       var mustBreak = forceBreakStack.last ?? false
 
       // Tracks whether the current line should be considered a continuation line, *if and only if
@@ -304,12 +290,6 @@ public class PrettyPrinter {
 
         continuationStack.append(currentLineIsContinuation)
 
-        // If the open break kind is a continuation and it fired, then we don't want to set this
-        // flag because the active open break will provide the continuation indentation for the
-        // remaining lines. If the break doesn't fire, we need to set it so that the remaining lines
-        // get the appropriate indentation.
-        wasLastBreakKindContinue = openKind == .continuation && !continuationBreakWillFire
-
         // Once we've reached an open break and preserved the continuation state, the "scope" we now
         // enter is *not* a continuation scope. If it was one, we'll re-enter it when we reach the
         // corresponding close.
@@ -382,11 +362,9 @@ public class PrettyPrinter {
 
         // Restore the continuation state of the scope we were in before the open break occurred.
         currentLineIsContinuation = currentLineIsContinuation || wasContinuationWhenOpened
-        wasLastBreakKindContinue = wasContinuationWhenOpened
         isContinuationIfBreakFires = wasContinuationWhenOpened
 
       case .continue:
-        wasLastBreakKindContinue = true
         isContinuationIfBreakFires = true
 
       case .same:
@@ -396,9 +374,24 @@ public class PrettyPrinter {
         mustBreak = currentLineIsContinuation
       }
 
-      if !isBreakingSupressed && ((!isAtStartOfLine && length > spaceRemaining) || mustBreak) {
+      var overrideBreakingSuppressed = false
+      switch newline {
+      case .elective: break
+      case .soft(_, let discretionary):
+        // A discretionary newline (i.e. from the source) should create a line break even if the
+        // rules for breaking are disabled.
+        overrideBreakingSuppressed = discretionary
+        mustBreak = true
+      case .hard:
+        // A hard newline must always create a line break, regardless of the context.
+        overrideBreakingSuppressed = true
+        mustBreak = true
+      }
+
+      let suppressBreaking = isBreakingSupressed && !overrideBreakingSuppressed
+      if !suppressBreaking && ((!isAtStartOfLine && length > spaceRemaining) || mustBreak) {
         currentLineIsContinuation = isContinuationIfBreakFires
-        writeNewlines(1, kind: .flexible)
+        writeNewlines(newline)
         lastBreak = true
       } else {
         if isAtStartOfLine {
@@ -417,24 +410,6 @@ public class PrettyPrinter {
     case .space(let size, _):
       enqueueSpaces(size)
 
-    // Apply `count` line breaks, calculate the indentation required, and adjust spaceRemaining.
-    case .newlines(let count, let kind):
-      // If a newline immediately followed an open-continue break, then this is effectively the
-      // same as if it had fired. Activate it, and reset the last-break-kind flag so that the
-      // indentation of subsequent lines is contributed by that break and not by inherited
-      // continuation state.
-      if let lastActiveOpenBreak = activeOpenBreaks.last,
-        lastActiveOpenBreak.index == idx - 1,
-        lastActiveOpenBreak.kind == .continuation
-      {
-        activeOpenBreaks[activeOpenBreaks.count - 1].contributesContinuationIndent = true
-        wasLastBreakKindContinue = false
-      }
-
-      currentLineIsContinuation = wasLastBreakKindContinue
-      writeNewlines(count, kind: kind)
-      lastBreak = true
-
     // Print any indentation required, followed by the text content of the syntax token.
     case .syntax(let text):
       guard !text.isEmpty else { break }
@@ -515,42 +490,27 @@ public class PrettyPrinter {
 
       // Break lengths are equal to its size plus the token or group following it. Calculate the
       // length of any prior break tokens.
-      case .break(_, let size, _):
+      case .break(_, let size, let newline):
         if let index = delimIndexStack.last, case .break = tokens[index] {
           lengths[index] += total
           delimIndexStack.removeLast()
         }
-
         lengths.append(-total)
         delimIndexStack.append(i)
-        total += size
+
+        if case .elective = newline {
+          total += size
+        } else {
+          // `size` is never used in this case, because the break always fires. Use `maxLineLength`
+          // to ensure enclosing groups are large enough to force preceding breaks to fire.
+          total += maxLineLength
+        }
 
       // Space tokens have a length equal to its size.
       case .space(let size, _):
         lengths.append(size)
         total += size
 
-      // The length of newlines are equal to the maximum allowed line length. Calculate the length
-      // of any prior break tokens.
-      case .newlines:
-        if let index = delimIndexStack.last, case .break = tokens[index] {
-          if index == i - 1 {
-            // A break immediately preceding a newline should have a length of zero, so that it
-            // doesn't fire.
-            lengths[index] = 0
-          } else {
-            lengths[index] += total
-          }
-          delimIndexStack.removeLast()
-        }
-
-        // Since newlines must always cause a line-break, we set their length as the full allowed
-        // width of the line. This causes any enclosing groups to have a length exceeding the line
-        // limit, and so the group must break and indent. e.g. single-line versus multi-line
-        // function bodies.
-        lengths.append(maxLineLength)
-        total += maxLineLength
-
       // Syntax tokens have a length equal to the number of columns needed to print its contents.
       case .syntax(let text):
         lengths.append(text.count)
@@ -615,11 +575,9 @@ public class PrettyPrinter {
       printDebugIndent()
       print("[SYNTAX \"\(syntax)\" Length: \(length) Idx: \(idx)]")
 
-    case .break(let kind, let size, let ignoresDiscretionary):
+    case .break(let kind, let size, let newline):
       printDebugIndent()
-      print(
-        "[BREAK Kind: \(kind) Size: \(size) Length: \(length) "
-          + "Ignores Discretionary NL: \(ignoresDiscretionary) Idx: \(idx)]")
+      print("[BREAK Kind: \(kind) Size: \(size) Length: \(length) NL: \(newline) Idx: \(idx)]")
 
     case .open(let breakstyle):
       printDebugIndent()
@@ -636,10 +594,6 @@ public class PrettyPrinter {
       printDebugIndent()
       print("[CLOSE Idx: \(idx)]")
 
-    case .newlines(let N, let required):
-      printDebugIndent()
-      print("[NEWLINES N: \(N) Required: \(required) Length: \(length) Idx: \(idx)]")
-
     case .space(let size, let flexible):
       printDebugIndent()
       print("[SPACE Size: \(size) Flexible: \(flexible) Length: \(length) Idx: \(idx)]")

Sources/SwiftFormatPrettyPrint/Token.swift

@@ -118,27 +118,32 @@ enum BreakKind: Equatable {
   static let open = BreakKind.open(kind: .block)
 }
 
-enum NewlineKind {
-  /// A newline that has been inserted by the formatter independent of the source code given by the
-  /// user (for example, between the getter and setter blocks of a computed property).
-  ///
-  /// Flexible newlines are only printed if a discretionary or mandatory newline has not yet been
-  /// printed at the same location, and only up to the maximum allowed by the formatter
-  /// configuration.
-  case flexible
-
-  /// A newline that was present in the source code given by the user (that is, at the user's
-  /// discretion).
-  ///
-  /// Discretionary newlines are printed after excluding any other consecutive newlines printed thus
-  /// far at the same location, and only up to the maximum allowed by the formatter configuration.
-  case discretionary
-
-  /// A mandatory newline that must always be printed (for example, in a multiline string literal).
-  ///
-  /// Mandatory newlines are never omitted by the pretty printer, even if it would result in a
-  /// number of consecutive newlines that exceeds that allowed by the formatter configuration.
-  case mandatory
+/// Behaviors for creating newlines as part of a break, i.e. where breaking onto a newline is
+/// allowed.
+enum NewlineBehavior {
+  /// Breaking onto a newline is allowed if necessary, but is not required. `ignoresDiscretionary`
+  /// specifies whether a user-entered discretionary newline should be respected.
+  case elective(ignoresDiscretionary: Bool)
+
+  /// Breaking onto a newline `count` times is required, unless it would create more blank lines
+  /// than are allowed by the current configuration. Any blank lines over the configured limit are
+  /// discarded. `discretionary` tracks whether these newlines were created based on user-entered
+  /// discretionary newlines, from the source, or were inserted by the formatter.
+  case soft(count: Int, discretionary: Bool)
+
+  /// Breaking onto a newline `count` times is required and any limits on blank lines are
+  /// **ignored**. Exactly `count` newlines are always printed, regardless of existing consecutive
+  /// newlines and the configured maximum number of blank lines.
+  case hard(count: Int)
+
+  /// An elective newline that respects discretionary newlines from the user-entered text.
+  static let elective = NewlineBehavior.elective(ignoresDiscretionary: false)
+
+  /// A single soft newline that is created by the formatter, i.e. *not* discretionary.
+  static let soft = NewlineBehavior.soft(count: 1, discretionary: false)
+
+  /// A single hard newline.
+  static let hard = NewlineBehavior.hard(count: 1)
 }
 
 /// Kinds of printer control tokens that can be used to customize the pretty printer's behavior in
@@ -160,9 +165,8 @@ enum Token {
   case syntax(String)
   case open(GroupBreakStyle)
   case close
-  case `break`(BreakKind, size: Int, ignoresDiscretionary: Bool)
+  case `break`(BreakKind, size: Int, newlines: NewlineBehavior)
   case space(size: Int, flexible: Bool)
-  case newlines(Int, kind: NewlineKind)
   case comment(Comment, wasEndOfLine: Bool)
   case verbatim(Verbatim)
   case printerControl(kind: PrinterControlKind)
@@ -174,24 +178,20 @@ enum Token {
     return Token.open(breakStyle)
   }
 
-  /// A single, flexible newline.
-  static let newline = Token.newlines(1, kind: .flexible)
-
-  /// Returns a single newline with the given kind.
-  static func newline(kind: NewlineKind) -> Token {
-    return Token.newlines(1, kind: kind)
-  }
-
   static let space = Token.space(size: 1, flexible: false)
 
   static func space(size: Int) -> Token {
     return .space(size: size, flexible: false)
   }
 
-  static let `break` = Token.break(.continue, size: 1, ignoresDiscretionary: false)
+  static let `break` = Token.break(.continue, size: 1, newlines: .elective)
 
   static func `break`(_ kind: BreakKind, size: Int = 1) -> Token {
-    return .break(kind, size: size, ignoresDiscretionary: false)
+    return .break(kind, size: size, newlines: .elective)
+  }
+
+  static func `break`(_ kind: BreakKind, newlines: NewlineBehavior) -> Token {
+    return .break(kind, size: 1, newlines: newlines)
   }
 
   static func verbatim(text: String) -> Token {