[clang-format] Fix breaking of comment sections in unwrapped lines containing newlines.

[krasimir]

Summary

The breaking of line comment sections was misaligning the case where the first comment line is on an unwrapped line containing newlines. In this case, the breaking column must be based on the source column of the last token that is preceded by a newline, not on the first token of the unwrapped line.

source:

enum A {
  a, // line 1
  // line 2
};

format before:

enum A {
  a, // line 1
     // line 2
};

format after:

enum A {
  a, // line 1
  // line 2
};

[krasimir]

• Add tests for comments in the end of enums.

[djasper]

Nice :)

lib/Format/BreakableToken.cpp

for (size_t i = FirstLineIndex, e = Lines.size(); i < e; ++i) {
       size_t EndOfLine = Content[i].find_last_not_of(Blanks);
       if (EndOfLine == StringRef::npos)
         EndOfLine = Content[i].size();
       else
         ++EndOfLine;
       Content[i] = Content[i].substr(0, EndOfLine);
     }
     LineTok = CurrentTok->Next;
-    if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+    if (CurrentTok->Next && !CurrentTok->Next->ContinuesLineCommentSection) {
       // A line comment section needs to broken by a line comment that is
       // preceded by at least two newlines. Note that we put this break here
       // instead of breaking at a previous stage during parsing, since that
       // would split the contents of the enum into two unwrapped lines in this
       // example, which is undesirable:
       // enum A {
       //   a, // comment about a
       //

lib/Format/FormatToken.h

struct FormatToken {
   /// with the same precedence, points to the next operator.
   FormatToken *NextOperator = nullptr;
 
   /// \brief Is this token part of a \c DeclStmt defining multiple variables?
   ///
   /// Only set if \c Type == \c TT_StartOfName.
   bool PartOfMultiVariableDeclStmt = false;
 
+  /// \brief Does this line comment continue a line comment section?
+  ///
+  /// Only set to true if \c Type == \c TT_LineComment.
+  bool ContinuesLineCommentSection = false;
+
   /// \brief If this is a bracket, this points to the matching one.
   FormatToken *MatchingParen = nullptr;
 
   /// \brief The previous token in the unwrapped line.
   FormatToken *Previous = nullptr;
 
   /// \brief The next token in the unwrapped line.
   FormatToken *Next = nullptr;

lib/Format/UnwrappedLineParser.cpp

static bool continuesLineComment(const FormatToken &FormatTok,
   // that '{'. Otherwise, the min column token of the line is the first token of
   // the line.
   //
   // If Line starts with a token other than a line comment, then FormatTok
   // continues the comment section if its original column is greater than the
   // original start column of the min column token of the line.
   //
   // For example, the second line comment continues the first in these cases:
+  //
   // // first line
   // // second line
+  //
   // and:
+  //
   // // first line
   //  // second line
+  //
   // and:
+  //
   // int i; // first line
   //  // second line
+  //
   // and:
+  //
   // do { // first line
   //      // second line
   //   int i;
   // } while (true);
   //
+  // and:
+  //
+  // enum {
+  //   a, // first line
+  //    // second line
+  //   b
+  // };
+  //
   // The second line comment doesn't continue the first in these cases:
+  //
   //   // first line
   //  // second line
+  //
   // and:
+  //
   // int i; // first line
   // // second line
+  //
   // and:
+  //
   // do { // first line
   //   // second line
   //   int i;
   // } while (true);
+  //
+  // and:
+  //
+  // enum {
+  //   a, // first line
+  //   // second line
+  // };
   const FormatToken *MinColumnToken = Line.Tokens.front().Tok;
 
   // Scan for '{//'. If found, use the column of '{' as a min column for line
   // comment section continuation.
   const FormatToken *PreviousToken = nullptr;
   for (const UnwrappedLineNode Node : Line.Tokens) {
     if (PreviousToken && PreviousToken->is(tok::l_brace) &&
         isLineComment(*Node.Tok)) {
       MinColumnToken = PreviousToken;
       break;
     }
     PreviousToken = Node.Tok;
+
+    // Grab the last newline preceding a token in this unwrapped line.
+    if (Node.Tok->NewlinesBefore > 0) {
+      MinColumnToken = Node.Tok;
+    }
   }
   if (PreviousToken && PreviousToken->is(tok::l_brace)) {
     MinColumnToken = PreviousToken;
   }
 
   unsigned MinContinueColumn =
       MinColumnToken->OriginalColumn +
       (isLineComment(*MinColumnToken) ? 0 : 1);
for (SmallVectorImpl<FormatToken *>::const_iterator
     // Line comments that belong to the same line comment section are put on the
     // same line since later we might want to reflow content between them.
     // Additional fine-grained breaking of line comment sections is controlled
     // by the class BreakableLineCommentSection in case it is desirable to keep
     // several line comment sections in the same unwrapped line.
     //
     // FIXME: Consider putting separate line comment sections as children to the
     // unwrapped line instead.
-    if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
+    (*I)->ContinuesLineCommentSection = continuesLineComment(**I, *Line);
+    if (isOnNewLine(**I) && JustComments && !(*I)->ContinuesLineCommentSection)
       addUnwrappedLine();
     pushToken(*I);
   }
   if (NewlineBeforeNext && JustComments)
     addUnwrappedLine();
   CommentsBeforeNextToken.clear();
 }
 
do {
 
     if (!PPStack.empty() && (PPStack.back() == PP_Unreachable) &&
         !Line->InPPDirective) {
       continue;
     }
 
     if (!FormatTok->Tok.is(tok::comment))
       return;
-    if (!continuesLineComment(*FormatTok, *Line) &&
+    FormatTok->ContinuesLineCommentSection =
+        continuesLineComment(*FormatTok, *Line);
+    if (!FormatTok->ContinuesLineCommentSection &&
         (isOnNewLine(*FormatTok) || FormatTok->IsFirst)) {
       CommentsInCurrentLine = false;
     }
     if (CommentsInCurrentLine) {
       pushToken(FormatTok);
     } else {
       CommentsBeforeNextToken.push_back(FormatTok);
     }

unittests/Format/FormatTest.cpp

EXPECT_EQ("enum A {\n"
                    "  // line a\n"
                    "  a,\n"
                    "  b, // line b\n"
                    "\n"
                    "  // line c\n"
                    "  c\n"
                    "};",
                    getLLVMStyleWithColumns(20)));
-
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "  // line 2\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "  // line 2\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "     // line 2\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "   // line 2\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "  // line 2\n"
+            "  b\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "  // line 2\n"
+                   "  b\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "     // line 2\n"
+            "  b\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "   // line 2\n"
+                   "  b\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
   verifyFormat(
       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
       "    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb; // Trailing comment");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
                "    // Comment inside a statement.\n"
                "    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;");
   verifyFormat("SomeFunction(a,\n"
                "             // comment\n"