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

[berenm]

Summary

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

[djasper]

Looks good, thank you.

unittests/Format/FormatTest.cpp

8657

So, it is kind of interesting that we (would) align with a lambda on the next line here, but not with the lambda declaration a few lines above.

I guess this makes sense, but I would also vager that somebody is going to file a bug report about it ;-).

[djasper]

Submitted as r249552.

[berenm]

unittests/Format/FormatTest.cpp

8657

Yes, this would require to handle nested levels of alignment sequences and push the current one when a new scope is detected. It's just not implemented yet :)

lib/Format/WhitespaceManager.cpp

void WhitespaceManager::calculateLineBreakInformation() {
   }
 }
 
 // Walk through all of the changes and find sequences of "=" to align.  To do
 // so, keep track of the lines and whether or not an "=" was found on align. If
 // 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;
 
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;
   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.
   //
   // Sequences start from the first assignment token to align, and end at the
   // first token of the first line that doesn't need to be aligned.
   //
   // We need to adjust the StartOfTokenColumn of each Change that is on a line
   // containing any assignment to be aligned and located after such assignment
   auto AlignSequence = [&] {
     if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
       alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
     MinColumn = 0;
     MaxColumn = UINT_MAX;
     StartOfSequence = 0;
     EndOfSequence = 0;
   };
 
   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
         AlignSequence();
       }
 
       FoundAssignmentOnLine = false;
+      FoundLeftBraceOnLine = false;
       FoundLeftParenOnLine = false;
     }
 
     // If there is more than one "=" per line, or if the "=" appears first on
     // the line of if it appears last, end the sequence
     if (Changes[i].Kind == tok::equal &&
         (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
          Changes[i + 1].NewlinesBefore > 0)) {
       AlignSequence();
-    } else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
+    } 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;
 
       unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
       int LineLengthAfter = -Changes[i].Spaces;
       for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j)
         LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;

 }
 
 // Walk through all of the changes and find sequences of declaration names to
 // align.  To do so, keep track of the lines and whether or not a name was found
 // on align. If a name 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-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;
 
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;
   unsigned StartOfSequence = 0;
   unsigned EndOfSequence = 0;

unittests/Format/FormatTest.cpp

verifyFormat("int i = 1;\n"
                "int j = 2;",
                Alignment);
   Alignment.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   verifyFormat("int i = 1;\n"
                "LooooooooooongType loooooooooooooooooooooongVariable =\n"
                "    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"
       "SomeType a = SomeFunction(looooooooooooooooooooooongParameterA,\n"
       "                          loooooooooooooooooooooongParameterB);\n"
       "int j = 2;",
       Alignment);
 }
verifyFormat("float              i = 1;\n"
                "int j = 2;",
                Alignment);
   Alignment.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   verifyFormat("int                i = 1;\n"
                "LooooooooooongType loooooooooooooooooooooongVariable =\n"
                "    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"
       "SomeType a = SomeFunction(looooooooooooooooooooooongParameterA,\n"
       "                          loooooooooooooooooooooongParameterB);\n"
       "int j = 2;",
       Alignment);