Merge pull request #31941 from nathawes/pattern-binding-decl-indentation

[SourceKit/CodeFormat] Don't column-align PatternBindingDecl entries in certain cases.

Author: Nathan Hawes

lib/IDE/Formatting.cpp

@@ -993,9 +993,14 @@ class ListAligner {
   SourceLoc AlignLoc;
   SourceLoc LastEndLoc;
   bool HasOutdent = false;
+  bool BreakAlignment = false;
 
 public:
 
+  /// Don't column-align if any element starts on the same line as IntroducerLoc
+  /// but ends on a later line.
+  bool BreakAlignmentIfSpanning = false;
+
   /// Constructs a new \c ListAligner for a list bounded by separate opening and
   /// closing tokens, e.g. tuples, array literals, parameter lists, etc.
   ///
@@ -1068,8 +1073,11 @@ class ListAligner {
     assert(Range.isValid());
     LastEndLoc = Range.End;
 
-    HasOutdent |= isOnSameLine(SM, IntroducerLoc, Range.Start) &&
-      OutdentChecker::hasOutdent(SM, Range, WalkableParent);
+    if (isOnSameLine(SM, IntroducerLoc, Range.Start)) {
+      HasOutdent |= OutdentChecker::hasOutdent(SM, Range, WalkableParent);
+      if (BreakAlignmentIfSpanning)
+        BreakAlignment |= !isOnSameLine(SM, IntroducerLoc, Range.End);
+    }
 
     if (HasOutdent || !SM.isBeforeInBuffer(Range.Start, TargetLoc))
       return;
@@ -1128,7 +1136,7 @@ class ListAligner {
     }
 
     bool ShouldIndent = shouldIndent(HasTrailingComma, TargetIsTrailing);
-    if (ShouldIndent && AlignLoc.isValid()) {
+    if (ShouldIndent && !BreakAlignment && AlignLoc.isValid()) {
       setAlignmentIfNeeded(Override);
       return IndentContext {AlignLoc, false, IndentContext::Exact};
     }
@@ -1140,7 +1148,7 @@ class ListAligner {
   /// This should be called before returning an \c IndentContext for a subrange
   /// of the list.
   void setAlignmentIfNeeded(ContextOverride &Override) {
-    if (HasOutdent || AlignLoc.isInvalid())
+    if (HasOutdent || BreakAlignment || AlignLoc.isInvalid())
       return;
     Override.setExact(SM, AlignLoc);
   }
@@ -1676,6 +1684,36 @@ class FormatWalker : public ASTWalker {
       SourceLoc ContextLoc = PBD->getStartLoc(), IntroducerLoc = PBD->getLoc();
 
       ListAligner Aligner(SM, TargetLocation, ContextLoc, IntroducerLoc);
+
+      // Don't column align PBD entries if any entry spans from the same line as
+      // the IntroducerLoc (var/let) to another line. E.g.
+      //
+      // let foo = someItem
+      //       .getValue(), // Column-alignment looks ok here, but...
+      //     bar = otherItem
+      //       .getValue()
+      //
+      // getAThing()
+      //   .andDoStuffWithIt()
+      // let foo = someItem
+      //       .getValue() // looks over-indented here, which is more common...
+      // getOtherThing()
+      //   .andDoStuffWithIt()
+      //
+      // getAThing()
+      //   .andDoStuffWithIt()
+      // let foo = someItem
+      //   .getValue() // so break column alignment in this case...
+      // doOtherThing()
+      //
+      // let foo = someItem.getValue(),
+      //     bar = otherItem.getValue() // but not in this case.
+      //
+      // Using this rule, rather than handling single and multi-entry PBDs
+      // differently, ensures that the as-typed-out indentation matches the
+      // re-indented indentation for multi-entry PBDs.
+      Aligner.BreakAlignmentIfSpanning = true;
+
       for (auto I: range(PBD->getNumPatternEntries())) {
         SourceRange EntryRange = PBD->getEqualLoc(I);
         VarDecl *SingleVar = nullptr;

test/SourceKit/CodeFormat/indent-multiline-string.swift

@@ -17,4 +17,4 @@ this is line1,
 // CHECK: key.sourcetext: "this is line1,"
 // CHECK: key.sourcetext: "     this is line2,"
 // CHECK: key.sourcetext: "\"\"\""
-// CHECK: key.sourcetext: "        \"content\""
+// CHECK: key.sourcetext: "    \"content\""

test/SourceKit/CodeFormat/rdar_32789463.swift

@@ -12,5 +12,5 @@ $
 
 // CHECK: key.sourcetext: "struct $ {"
 // CHECK: key.sourcetext: "    let $: <#Type#>"
-// CHECK: key.sourcetext: "        = foo(\"foo \\($) bar\") {"
+// CHECK: key.sourcetext: "    = foo(\"foo \\($) bar\") {"
 // CHECK: key.sourcetext: "    $"

test/swift-indent/basic.swift

@@ -68,9 +68,9 @@ test(arg1: 1,
 }
 
 let x = [1, 2, 3]
-        .filter {$0 < $1}
-        .filter {$0 < $1}
-        .filter {$0 < $1}
+    .filter {$0 < $1}
+    .filter {$0 < $1}
+    .filter {$0 < $1}
 
 bax(34949494949)
     .foo(a: Int,
@@ -141,11 +141,11 @@ if #available(
 // do the same.
 //
 let _ = []
-        .map {
-            f {
-                print()
-            } ?? 0
-        }
+    .map {
+        f {
+            print()
+        } ?? 0
+    }
 
 basename
     .foo(a: Int,
@@ -234,10 +234,10 @@ let arrayC = [2]
 let arrayD = [3]
 
 let array1 =
-        arrayA +
-        arrayB +
-        arrayC +
-        arrayD
+    arrayA +
+    arrayB +
+    arrayC +
+    arrayD
 
 array1 =
     arrayA +
@@ -254,9 +254,9 @@ arrayC +
 arrayD
 
 let array2 = arrayA +
-        arrayB +
-        arrayC +
-        arrayD
+    arrayB +
+    arrayC +
+    arrayD
 
 
 // Comments should not break exact alignment, and leading comments should be aligned, rather than the label.
@@ -867,7 +867,7 @@ func foo(
 ) {}
 
 var (d, e):
-        (Int, Int) = (1, 3),
+    (Int, Int) = (1, 3),
     (f, g): (
         Int,
         Int
@@ -1015,3 +1015,24 @@ catch MyErr.a(let code, let message),
 {
     print("ahhh!")
 }
+
+// Pattern binding decls should only column-align if no element spans from the first line to beyond it.
+
+public let x = 10,
+           y = 20
+
+private var firstThing = 20,
+            secondThing = item
+                .filter {},
+            thirdThing = 42
+
+public let myVar = itemWithALongName
+    .filter { $0 >= $1 && $0 - $1 < 50}
+
+public let first = 45, second = itemWithALongName
+    .filter { $0 >= $1 && $0 - $1 < 50}
+
+private var secondThing = item
+    .filter {},
+    firstThing = 20,
+    thirdThing = 56