Merge pull request swiftlang#77 from allevato/fix-props-with-defaults-and-accessors

Fix formatting for vars with default values and accessors.

Author: allevato

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -1333,21 +1333,37 @@ private final class TokenStreamCreator: SyntaxVisitor {
   }
 
   func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
+    // If the type annotation and/or the initializer clause need to wrap, we want those
+    // continuations to stack to improve readability. So, we need to keep track of how many open
+    // breaks we create (so we can close them at the end of the binding) and also keep track of the
+    // right-most token that will anchor the close breaks.
+    var closesNeeded: Int = 0
+    var closeAfterToken: TokenSyntax? = nil
+
+    if let typeAnnotation = node.typeAnnotation {
+      after(typeAnnotation.colon, tokens: .break(.open(kind: .continuation)))
+      closesNeeded += 1
+      closeAfterToken = typeAnnotation.lastToken
+    }
+    if let initializer = node.initializer {
+      after(initializer.equal, tokens: .break(.open(kind: .continuation)))
+      closesNeeded += 1
+      closeAfterToken = initializer.lastToken
+    }
+
     if let accessorOrCodeBlock = node.accessor {
-      if let typeAnnotation = node.typeAnnotation {
-        after(typeAnnotation.colon, tokens: .break)
-      }
       arrangeAccessorOrCodeBlock(accessorOrCodeBlock)
-    } else {
-      if let typeAnnotation = node.typeAnnotation {
-        after(typeAnnotation.colon, tokens: .break(.open(kind: .continuation)))
-        after(node.lastToken, tokens: .break(.close, size: 0))
-      }
-      if let initializer = node.initializer {
-        after(initializer.equal, tokens: .break(.open(kind: .continuation)))
-        after(node.lastToken, tokens: .break(.close, size: 0))
-      }
+    } else if let trailingComma = node.trailingComma {
+      // If this is one of multiple comma-delimited bindings, move any pending close breaks to
+      // follow the comma so that it doesn't get separated from the tokens before it.
+      closeAfterToken = trailingComma
     }
+
+    if closeAfterToken != nil && closesNeeded > 0 {
+      let closeTokens = [Token](repeatElement(.break(.close, size: 0), count: closesNeeded))
+      after(closeAfterToken, tokens: closeTokens)
+    }
+
     return .visitChildren
   }
 

Tests/SwiftFormatPrettyPrintTests/AccessorTests.swift

@@ -151,4 +151,148 @@ public class AccessorTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: input + "\n", linelength: 50)
   }
+
+  public func testDefaultValueAndAccessor() {
+    let input =
+      """
+      var property = defaultValue {
+        didSet {
+          foo()
+          bar()
+        }
+      }
+      """
+
+    assertPrettyPrintEqual(input: input, expected: input + "\n", linelength: 80)
+
+    let expected20 =
+      """
+      var property =
+        defaultValue
+      {
+        didSet {
+          foo()
+          bar()
+        }
+      }
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected20, linelength: 20)
+  }
+
+  public func testTypeDefaultValueAndAccessor() {
+    let input =
+      """
+      var property: SomeType = defaultValue {
+        didSet {
+          foo()
+          bar()
+        }
+      }
+      """
+
+    assertPrettyPrintEqual(input: input, expected: input + "\n", linelength: 80)
+
+    let expected25 =
+      """
+      var property: SomeType =
+        defaultValue
+      {
+        didSet {
+          foo()
+          bar()
+        }
+      }
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected25, linelength: 25)
+
+    let expected20 =
+      """
+      var property:
+        SomeType =
+          defaultValue
+      {
+        didSet {
+          foo()
+          bar()
+        }
+      }
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected20, linelength: 20)
+  }
+
+  public func testMultipleBindingsWithAccessors() {
+    // NOTE: These examples are not actually valid Swift! The syntax parser will allow a variable
+    // declaration that has multiple comma-separated bindings that have accessors, but the compiler
+    // rejects these at a later stage ("error: 'var' declarations with multiple variables cannot
+    // have explicit getters/setters"). But since the parser allows it, we make an attempt to format
+    // them correctly, rather than bail out and potentially leave the source code in a worse state
+    // than the original.
+
+    let input =
+      """
+      var property1: SomeType = defaultValue {
+        didSet {
+          foo()
+          bar()
+        }
+      }, property2: SomeType = defaultValue {
+        didSet {
+          foo()
+          bar()
+        }
+      }
+      """
+
+    let expected =
+      """
+      var
+        property1: SomeType = defaultValue {
+          didSet {
+            foo()
+            bar()
+          }
+        },
+        property2: SomeType = defaultValue {
+          didSet {
+            foo()
+            bar()
+          }
+        }
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 80)
+
+    let expected20 =
+      """
+      var
+        property1:
+          SomeType =
+            defaultValue
+        {
+          didSet {
+            foo()
+            bar()
+          }
+        },
+        property2:
+          SomeType =
+            defaultValue
+        {
+          didSet {
+            foo()
+            bar()
+          }
+        }
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected20, linelength: 20)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift

@@ -7,10 +7,13 @@ extension AccessorTests {
     // to regenerate.
     static let __allTests__AccessorTests = [
         ("testBasicAccessors", testBasicAccessors),
+        ("testDefaultValueAndAccessor", testDefaultValueAndAccessor),
         ("testEmptyAccessorBody", testEmptyAccessorBody),
         ("testEmptyAccessorBodyWithComment", testEmptyAccessorBodyWithComment),
         ("testEmptyAccessorList", testEmptyAccessorList),
+        ("testMultipleBindingsWithAccessors", testMultipleBindingsWithAccessors),
         ("testSetModifier", testSetModifier),
+        ("testTypeDefaultValueAndAccessor", testTypeDefaultValueAndAccessor),
     ]
 }