Merge pull request swiftlang#224 from google/fix-brace

Allow breaks to ignore discretionary newlines.

Author: dabelknap

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -193,7 +193,7 @@ public class PrettyPrinter {
 
     // Create a line break if needed. Calculate the indentation required and adjust spaceRemaining
     // accordingly.
-    case .break(let kind, let size):
+    case .break(let kind, let size, _):
       lastBreakKind = kind
       var mustBreak = forceBreakStack.last ?? false
       var isContinuation = false
@@ -346,7 +346,7 @@ 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, _):
         if let index = delimIndexStack.last, case .break = tokens[index] {
           lengths[index] += total
           delimIndexStack.removeLast()
@@ -436,9 +436,11 @@ public class PrettyPrinter {
       printDebugIndent()
       print("[SYNTAX \"\(syntax)\" Length: \(length)]")
 
-    case .break(let kind, let size):
+    case .break(let kind, let size, let ignoresDiscretionary):
       printDebugIndent()
-      print("[BREAK Kind: \(kind) Size: \(size) Length: \(length)]")
+      print(
+        "[BREAK Kind: \(kind) Size: \(size) Length: \(length) "
+          + "Ignores Discretionary NL: \(ignoresDiscretionary)]")
 
     case .open(let breakstyle):
       printDebugIndent()

Sources/SwiftFormatPrettyPrint/Token.swift

@@ -108,7 +108,7 @@ enum Token {
   case syntax(String)
   case open(GroupBreakStyle)
   case close
-  case `break`(BreakKind, size: Int)
+  case `break`(BreakKind, size: Int, ignoresDiscretionary: Bool)
   case space(size: Int)
   case newlines(Int, discretionary: Bool)
   case comment(Comment, wasEndOfLine: Bool)
@@ -126,9 +126,9 @@ enum Token {
 
   static let space = Token.space(size: 1)
 
-  static let `break` = Token.break(.continue, size: 1)
-  static func `break`(_ kind: BreakKind) -> Token {
-    return .break(kind, size: 1)
+  static let `break` = Token.break(.continue, size: 1, ignoresDiscretionary: false)
+  static func `break`(_ kind: BreakKind, size: Int = 1) -> Token {
+    return .break(kind, size: size, ignoresDiscretionary: false)
   }
 
   static func verbatim(text: String) -> Token {

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -1460,7 +1460,7 @@ private final class TokenStreamCreator: SyntaxVisitor {
     guard let node = node, let contentsKeyPath = contentsKeyPath else { return }
 
     if shouldResetBeforeLeftBrace {
-      before(node.leftBrace, tokens: .break(.reset, size: 1))
+      before(node.leftBrace, tokens: .break(.reset, size: 1, ignoresDiscretionary: true))
     }
     if !areBracesCompletelyEmpty(node, contentsKeyPath: contentsKeyPath) {
       after(node.leftBrace, tokens: .break(.open, size: 1), .open)
@@ -1626,17 +1626,25 @@ private final class TokenStreamCreator: SyntaxVisitor {
   /// Returns a value indicating whether or not discretionary newlines are permitted before the
   /// given syntax token.
   ///
-  /// Discretionary newlines are allowed before any token that is preceded by a break or an existing
-  /// newline (ignoring open/close group tokens, which do not contribute to this). In other words,
-  /// this means that users may insert their own breaks in places where the pretty printer allows
-  /// them, even if those breaks wouldn't cause wrapping based on the column limit, but they may not
-  /// place them in places where the pretty printer would not break (for example, at a space token
-  /// that is intended to keep two tokens glued together).
+  /// Discretionary newlines are allowed before any token (ignoring open/close group tokens, which
+  /// do not contribute to this) that is preceded by an existing newline or that is preceded by a
+  /// break whose `ignoresDiscretionary` property is false. In other words, this means that users
+  /// may insert their own breaks in places where the pretty printer allows them, even if those
+  /// breaks wouldn't cause wrapping based on the column limit, but they may not place them in
+  /// places where the pretty printer would not break (for example, at a space token that is
+  /// intended to keep two tokens glued together).
+  ///
+  /// Furthermore, breaks with `ignoresDiscretionary` equal to `true` are in effect "last resort"
+  /// breaks; a user's newline will be discarded unless the algorithm *must* break there. For
+  /// example, an open curly brace on a non-continuation line should always be kept on the same line
+  /// as the tokens before it unless the tokens before it are exactly the length of the line and a
+  /// break must be inserted there to prevent the brace from going over the limit.
   private func isDiscretionaryNewlineAllowed(before token: TokenSyntax) -> Bool {
     func isBreakMoreRecentThanNonbreakingContent(_ tokens: [Token]) -> Bool? {
       for token in tokens.reversed() as ReversedCollection {
         switch token {
-        case .break, .newlines: return true
+        case .newlines: return true
+        case .break(_, _, let ignoresDiscretionary): return !ignoresDiscretionary
         case .comment, .space, .syntax, .verbatim: return false
         default: break
         }

Tests/SwiftFormatPrettyPrintTests/FunctionDeclTests.swift

@@ -552,4 +552,33 @@ public class FunctionDeclTests: PrettyPrintTestCase {
       """
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 35)
   }
+
+  public func testRemovesLineBreakBeforeOpenBraceUnlessAbsolutelyNecessary() {
+    let input =
+      """
+      func foo(bar: Int)
+      {
+        baz()
+      }
+
+      func foo(longer: Int)
+      {
+        baz()
+      }
+      """
+
+    let expected =
+      """
+      func foo(bar: Int) {
+        baz()
+      }
+
+      func foo(longer: Int)
+      {
+        baz()
+      }
+
+      """
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 21)
+  }
 }