[clang-format] Stop alignment sequences on open braces and parens when

aligning assignments.

This was done correctly when aligning the declarations, but not when
aligning assignments.

FIXME: The code between assignments and declarations alignment is
roughly duplicated and
would benefit from factorization.

Bug 25090: https://llvm.org/bugs/show_bug.cgi?id=25090

Patch by Beren Minor. Thank you.

llvm-svn: 249552

2 files changed, 58 insertions, 8 deletions

diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 1b11e73..3536132 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -153,6 +153,9 @@ void WhitespaceManager::calculateLineBreakInformation() {
 // a "=" is found on a line, extend the current sequence. If the current line
 // cannot be part of a sequence, e.g. because there is an empty line before it
 // or it contains non-assignments, finalize the previous sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveAssignments() {
   if (!Style.AlignConsecutiveAssignments)
     return;
@@ -162,6 +165,7 @@ void WhitespaceManager::alignConsecutiveAssignments() {
   unsigned StartOfSequence = 0;
   unsigned EndOfSequence = 0;
   bool FoundAssignmentOnLine = false;
+  bool FoundLeftBraceOnLine = false;
   bool FoundLeftParenOnLine = false;
 
   // Aligns a sequence of assignment tokens, on the MinColumn column.
@@ -181,11 +185,13 @@ void WhitespaceManager::alignConsecutiveAssignments() {
   };
 
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-    if (Changes[i].NewlinesBefore > 0) {
+    if (Changes[i].NewlinesBefore != 0) {
       EndOfSequence = i;
-      // If there is a blank line or if the last line didn't contain any
-      // assignment, the sequence ends here.
-      if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+      // If there is a blank line, if the last line didn't contain any
+      // assignment, or if we found an open brace or paren, the sequence ends
+      // here.
+      if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
+          FoundLeftBraceOnLine || FoundLeftParenOnLine) {
         // NB: In the latter case, the sequence should end at the beggining of
         // the previous line, but it doesn't really matter as there is no
         // assignment on it
@@ -193,6 +199,7 @@ void WhitespaceManager::alignConsecutiveAssignments() {
       }
 
       FoundAssignmentOnLine = false;
+      FoundLeftBraceOnLine = false;
       FoundLeftParenOnLine = false;
     }
 
@@ -202,14 +209,24 @@ void WhitespaceManager::alignConsecutiveAssignments() {
         (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
          Changes[i + 1].NewlinesBefore > 0)) {
       AlignSequence();
-    } else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
-      AlignSequence();
+    } else if (Changes[i].Kind == tok::r_brace) {
+      if (!FoundLeftBraceOnLine)
+        AlignSequence();
+      FoundLeftBraceOnLine = false;
+    } else if (Changes[i].Kind == tok::l_brace) {
+      FoundLeftBraceOnLine = true;
+      if (!FoundAssignmentOnLine)
+        AlignSequence();
+    } else if (Changes[i].Kind == tok::r_paren) {
+      if (!FoundLeftParenOnLine)
+        AlignSequence();
+      FoundLeftParenOnLine = false;
     } else if (Changes[i].Kind == tok::l_paren) {
       FoundLeftParenOnLine = true;
       if (!FoundAssignmentOnLine)
         AlignSequence();
-    } else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
-               Changes[i].Kind == tok::equal) {
+    } else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
+               !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
       FoundAssignmentOnLine = true;
       if (StartOfSequence == 0)
         StartOfSequence = i;
@@ -267,6 +284,9 @@ void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
 // current line cannot be part of a sequence, e.g. because there is an empty
 // line before it or it contains non-declarations, finalize the previous
 // sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveDeclarations() {
   if (!Style.AlignConsecutiveDeclarations)
     return;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 00f87ba..5f94c21 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8649,6 +8649,19 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) {
                "    someLooooooooooooooooongFunction();\n"
                "int j = 2;",
                Alignment);
+
+  verifyFormat("auto lambda = []() {\n"
+               "  auto i = 0;\n"
+               "  return 0;\n"
+               "};\n"
+               "int i  = 0;\n"
+               "auto v = type{\n"
+               "    i = 1,   //\n"
+               "    (i = 2), //\n"
+               "    i = 3    //\n"
+               "};",
+               Alignment);
+
   // FIXME: Should align all three assignments
   verifyFormat(
       "int i      = 1;\n"
@@ -8817,6 +8830,23 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
                "    someLooooooooooooooooongFunction();\n"
                "int j = 2;",
                Alignment);
+
+  Alignment.AlignConsecutiveAssignments = true;
+  verifyFormat("auto lambda = []() {\n"
+               "  auto  ii = 0;\n"
+               "  float j  = 0;\n"
+               "  return 0;\n"
+               "};\n"
+               "int   i  = 0;\n"
+               "float i2 = 0;\n"
+               "auto  v  = type{\n"
+               "    i = 1,   //\n"
+               "    (i = 2), //\n"
+               "    i = 3    //\n"
+               "};",
+               Alignment);
+  Alignment.AlignConsecutiveAssignments = false;
+
   // FIXME: Should align all three declarations
   verifyFormat(
       "int      i = 1;\n"