OLD DRAFT [clang-format] Handle trailing comment sections in import statement lines

[krasimir]

Summary

This patch updates the handling of multiline trailing comment sections in
import statement lines to make it more consistent with the case in general.
This includes updating the parsing logic to collect the trailing comment
sections and the formatting logic to rely on the comment section breakable
tokens for formatting.

Specifically, before this patch this code:

#include <a> // line 1
             // line 2

will be turned into two unwrapped lines, whereas this code:

int i; // line 1
       // line 2

is turned into a single unwrapped line, enabling reflowing across comments.

An example where the old behaviour is bad is when partially formatting the lines
3 to 4 of this code:

#include <a> // line 1
             // line 2

int i;

which gets turned into:

#include <a> // line 1
             // line 2

             int i;

because the two comment lines were independent and the indent was copied.

[krasimir]

@djasper: ping

lib/Format/BreakableToken.h

 
 class BreakableComment : public BreakableToken {
 protected:
   /// \brief Creates a breakable token for a comment.
   ///
   /// \p StartColumn specifies the column in which the comment will start after
   /// formatting.
   BreakableComment(const FormatToken &Token, unsigned StartColumn,
-                   bool InPPDirective, encoding::Encoding Encoding,
-                   const FormatStyle &Style);
+                   bool InPPDirective, LineType ThisLineType,
+                   encoding::Encoding Encoding, const FormatStyle &Style);
 
 public:
   unsigned getLineCount() const override;
   Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
                  llvm::Regex &CommentPragmasRegex) const override;
   void compressWhitespace(unsigned LineIndex, unsigned TailOffset, Split Split,
                           WhitespaceManager &Whitespaces) override;
 
protected:
   // Returns the token containing the line at LineIndex.
   const FormatToken &tokenAt(unsigned LineIndex) const;
 
   // Checks if the content of line LineIndex may be reflown with the previous
   // line.
   virtual bool mayReflow(unsigned LineIndex,
                          llvm::Regex &CommentPragmasRegex) const = 0;
 
+  LineType ThisLineType;
+
   // Contains the original text of the lines of the block comment.
   //
   // In case of a block comments, excludes the leading /* in the first line and
   // trailing */ in the last line. In case of line comments, excludes the
   // leading // and spaces.
   SmallVector<StringRef, 16> Lines;
 
   // Contains the text of the lines excluding all leading and trailing
protected:
   // // comment 1comment 2
   StringRef ReflowPrefix = " ";
 };
 
 class BreakableBlockComment : public BreakableComment {
 public:
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
                         unsigned OriginalStartColumn, bool FirstInLine,
-                        bool InPPDirective, encoding::Encoding Encoding,
-                        const FormatStyle &Style);
+                        bool InPPDirective, LineType ThisLineType,
+                        encoding::Encoding Encoding, const FormatStyle &Style);
 
   unsigned getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset,
                                    StringRef::size_type Length) const override;
   void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
                    WhitespaceManager &Whitespaces) override;
   Split getSplitBefore(unsigned LineIndex, unsigned PreviousEndColumn,
                        unsigned ColumnLimit,
                        llvm::Regex &CommentPragmasRegex) const override;
private:
   // the decorations.
   unsigned DecorationColumn;
 };
 
 class BreakableLineCommentSection : public BreakableComment {
 public:
   BreakableLineCommentSection(const FormatToken &Token, unsigned StartColumn,
                               unsigned OriginalStartColumn, bool FirstInLine,
-                              bool InPPDirective, encoding::Encoding Encoding,
+                              bool InPPDirective, LineType ThisLineType,
+                              encoding::Encoding Encoding,
                               const FormatStyle &Style);
 
   unsigned getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset,
                                    StringRef::size_type Length) const override;
   void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
                    WhitespaceManager &Whitespaces) override;
   Split getSplitBefore(unsigned LineIndex, unsigned PreviousEndColumn,
                        unsigned ColumnLimit,

lib/Format/BreakableToken.cpp

void BreakableStringLiteral::insertBreak(unsigned LineIndex,
   Whitespaces.replaceWhitespaceInToken(
       Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
       Prefix, InPPDirective, 1, StartColumn);
 }
 
 BreakableComment::BreakableComment(const FormatToken &Token,
                                    unsigned StartColumn,
                                    bool InPPDirective,
+                                   LineType ThisLineType,
                                    encoding::Encoding Encoding,
                                    const FormatStyle &Style)
     : BreakableToken(Token, InPPDirective, Encoding, Style),
-      StartColumn(StartColumn) {}
+      ThisLineType(ThisLineType), StartColumn(StartColumn) {}
 
 unsigned BreakableComment::getLineCount() const { return Lines.size(); }
 
 BreakableToken::Split
 BreakableComment::getSplit(unsigned LineIndex, unsigned TailOffset,
                            unsigned ColumnLimit,
                            llvm::Regex &CommentPragmasRegex) const {
-  // Don't break lines matching the comment pragmas regex.
-  if (CommentPragmasRegex.match(Content[LineIndex]))
+  // Don't break lines matching the comment pragmas regex and trailing comments
+  // in import statements, as they are usually too long.
+  if (CommentPragmasRegex.match(Content[LineIndex]) ||
+      ThisLineType == LT_ImportStatement)
     return Split(StringRef::npos, 0);
   return getCommentSplit(Content[LineIndex].substr(TailOffset),
                          getContentStartColumn(LineIndex, TailOffset),
                          ColumnLimit, Style.TabWidth, Encoding);
 }
 
 void BreakableComment::compressWhitespace(unsigned LineIndex,
                                           unsigned TailOffset, Split Split,
return Content.size() >= 2 && !hasSpecialMeaningPrefix &&
          // Note that this is UTF-8 safe, since if isPunctuation(Content[0]) is
          // true, then the first code point must be 1 byte long.
          (!isPunctuation(Content[0]) || !isPunctuation(Content[1]));
 }
 
 BreakableBlockComment::BreakableBlockComment(
     const FormatToken &Token, unsigned StartColumn,
     unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-    encoding::Encoding Encoding, const FormatStyle &Style)
-    : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style) {
+    LineType ThisLineType, encoding::Encoding Encoding,
+    const FormatStyle &Style)
+    : BreakableComment(Token, StartColumn, InPPDirective, ThisLineType,
+                       Encoding, Style) {
   assert(Tok.is(TT_BlockComment) &&
          "block comment section must start with a block comment");
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
   TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
unsigned WhitespaceLength = Content[LineIndex].data() -
                               WhitespaceOffsetInToken;
   Whitespaces.replaceWhitespaceInToken(
       tokenAt(LineIndex), WhitespaceOffsetInToken, WhitespaceLength, "", Prefix,
       InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
 }
 
 bool BreakableBlockComment::mayReflow(unsigned LineIndex,
                                       llvm::Regex &CommentPragmasRegex) const {
+  if (ThisLineType == LT_ImportStatement) return false;
   // Content[LineIndex] may exclude the indent after the '*' decoration. In that
   // case, we compute the start of the comment pragma manually.
   StringRef IndentContent = Content[LineIndex];
   if (Lines[LineIndex].ltrim(Blanks).startswith("*")) {
     IndentContent = Lines[LineIndex].ltrim(Blanks).substr(1);
   }
   return LineIndex > 0 && !CommentPragmasRegex.match(IndentContent) &&
          mayReflowContent(Content[LineIndex]) && !Tok.Finalized &&
          !switchesFormatting(tokenAt(LineIndex));
 }
 
 unsigned
 BreakableBlockComment::getContentStartColumn(unsigned LineIndex,
                                              unsigned TailOffset) const {
   // If we break, we always break at the predefined indent.
   if (TailOffset != 0)
     return IndentAtLineBreak;
   return std::max(0, ContentColumn[LineIndex]);
 }
 
 BreakableLineCommentSection::BreakableLineCommentSection(
     const FormatToken &Token, unsigned StartColumn,
     unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-    encoding::Encoding Encoding, const FormatStyle &Style)
-    : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style) {
+    LineType ThisLineType, encoding::Encoding Encoding,
+    const FormatStyle &Style)
+    : BreakableComment(Token, StartColumn, InPPDirective, ThisLineType,
+                       Encoding, Style) {
   assert(Tok.is(TT_LineComment) &&
          "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
   for (const FormatToken *CurrentTok = &Tok;
        CurrentTok && CurrentTok->is(TT_LineComment);
        CurrentTok = CurrentTok->Next) {
     LastLineTok = LineTok;
     StringRef TokenText(CurrentTok->TokenText);

lib/Format/ContinuationIndenter.cpp

 unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
                                                     LineState &State,
                                                     bool DryRun) {
   // Don't break multi-line tokens other than block comments. Instead, just
   // update the state.
   if (Current.isNot(TT_BlockComment) && Current.IsMultiline)
     return addMultilineToken(Current, State);
 
-  // Don't break implicit string literals or import statements.
+  // Don't break implicit string literals or import statements, however let
+  // BreakableComment manage trailing line comment sections in import statement
+  // lines.
   if (Current.is(TT_ImplicitStringLiteral) ||
-      State.Line->Type == LT_ImportStatement)
+      (Current.isNot(TT_LineComment) && State.Line->Type == LT_ImportStatement))
     return 0;
 
   if (!Current.isStringLiteral() && !Current.is(tok::comment))
     return 0;
 
   std::unique_ptr<BreakableToken> Token;
   unsigned StartColumn = State.Column - Current.ColumnWidth;
   unsigned ColumnLimit = getColumnLimit(State);
if (Current.isStringLiteral()) {
     if (!Current.isTrailingComment() || !Style.ReflowComments ||
         // If a comment token switches formatting, like
         // /* clang-format on */, we don't want to break it further,
         // but we may still want to adjust its indentation.
         switchesFormatting(Current))
       return addMultilineToken(Current, State);
     Token.reset(new BreakableBlockComment(
         Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-        State.Line->InPPDirective, Encoding, Style));
-  } else if (Current.is(TT_LineComment) &&
-             (Current.Previous == nullptr ||
-              Current.Previous->isNot(TT_ImplicitStringLiteral))) {
+        State.Line->InPPDirective, State.Line->Type, Encoding, Style));
+  } else if (Current.is(TT_LineComment)) {
     if (!Style.ReflowComments ||
         CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
         switchesFormatting(Current))
       return 0;
     Token.reset(new BreakableLineCommentSection(
         Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-        /*InPPDirective=*/false, Encoding, Style));
+        /*InPPDirective=*/false, State.Line->Type, Encoding, Style));
     // We don't insert backslashes when breaking line comments.
     ColumnLimit = Style.ColumnLimit;
   } else {
     return 0;
   }
   if (Current.UnbreakableTailLength >= ColumnLimit)
     return 0;
 

lib/Format/TokenAnnotator.cpp

bool consumeToken() {
     }
     return true;
   }
 
   void parseIncludeDirective() {
     if (CurrentToken && CurrentToken->is(tok::less)) {
       next();
       while (CurrentToken) {
-        if (CurrentToken->isNot(tok::comment) || CurrentToken->Next)
+        // Mark tokens up to the trailing line comments as implicit string
+        // literals.
+        if (CurrentToken->isNot(tok::comment) &&
+            !CurrentToken->TokenText.startswith("//"))
           CurrentToken->Type = TT_ImplicitStringLiteral;
         next();
       }
     }
   }
 
   void parseWarningOrError() {
     next();

lib/Format/UnwrappedLineParser.cpp

else
       Line.MustBeDeclaration = true;
   }
 
 private:
   UnwrappedLine &Line;
   std::vector<bool> &Stack;
 };
 
+static bool isLineComment(const FormatToken &FormatTok) {
+  return FormatTok.is(tok::comment) &&
+         FormatTok.TokenText.startswith("//");
+}
+
 class ScopedMacroState : public FormatTokenSource {
 public:
   ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
                    FormatToken *&ResetToken)
       : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
         PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
-        Token(nullptr) {
+        Token(nullptr), PreviousToken(nullptr) {
     TokenSource = this;
     Line.Level = 0;
     Line.InPPDirective = true;
   }
 
   ~ScopedMacroState() override {
     TokenSource = PreviousTokenSource;
     ResetToken = Token;
     Line.InPPDirective = false;
     Line.Level = PreviousLineLevel;
   }
 
   FormatToken *getNextToken() override {
     // The \c UnwrappedLineParser guards against this by never calling
     // \c getNextToken() after it has encountered the first eof token.
     assert(!eof());
+    PreviousToken = Token;
     Token = PreviousTokenSource->getNextToken();
     if (eof())
       return getFakeEOF();
     return Token;
   }
 
   unsigned getPosition() override { return PreviousTokenSource->getPosition(); }
 
   FormatToken *setPosition(unsigned Position) override {
+    PreviousToken = nullptr;
     Token = PreviousTokenSource->setPosition(Position);
     return Token;
   }
 
 private:
-  bool eof() { return Token && Token->HasUnescapedNewline; }
+  bool eof() {
+    return Token && Token->HasUnescapedNewline &&
+           !(PreviousToken && isLineComment(*PreviousToken) &&
+             isLineComment(*Token) && Token->NewlinesBefore == 1);
+  }
 
   FormatToken *getFakeEOF() {
     static bool EOFInitialized = false;
     static FormatToken FormatTok;
     if (!EOFInitialized) {
       FormatTok.Tok.startToken();
       FormatTok.Tok.setKind(tok::eof);
       EOFInitialized = true;
     }
     return &FormatTok;
   }
 
   UnwrappedLine &Line;
   FormatTokenSource *&TokenSource;
   FormatToken *&ResetToken;
   unsigned PreviousLineLevel;
   FormatTokenSource *PreviousTokenSource;
 
   FormatToken *Token;
+  FormatToken *PreviousToken = nullptr;
 };
 
 } // end anonymous namespace
 
 class ScopedLineState {
 public:
   ScopedLineState(UnwrappedLineParser &Parser,
                   bool SwitchToPreprocessorLines = false)

 
 bool UnwrappedLineParser::eof() const { return FormatTok->Tok.is(tok::eof); }
 
 bool UnwrappedLineParser::isOnNewLine(const FormatToken &FormatTok) {
   return (Line->InPPDirective || FormatTok.HasUnescapedNewline) &&
          FormatTok.NewlinesBefore > 0;
 }
 
-static bool isLineComment(const FormatToken &FormatTok) {
-  return FormatTok.is(tok::comment) &&
-         FormatTok.TokenText.startswith("//");
-}
-
 // Checks if \p FormatTok is a line comment that continues the line comment
 // section on \p Line.
 static bool continuesLineComment(const FormatToken &FormatTok,
                                  const UnwrappedLine &Line,
                                  llvm::Regex &CommentPragmasRegex) {
   if (Line.Tokens.empty())
     return false;
 

unittests/Format/FormatTestSelective.cpp

EXPECT_EQ(
       "var x = 'a';\n"
       "var x = \"a\";",
       format("var x = \"a\";\n"
              "var x = \"a\";\n"
              "var x = \"a\";",
              20, 0));
 }
 
+TEST_F(FormatTestSelective, KeepsIndentAfterCommentSectionImport) {
+  std::string Code = "#include <a> // line 1\n" // 23 chars long
+                     "             // line 2\n" // 23 chars long
+                     "\n"                       // this newline is char 47
+                     "int i;";                  // this line is not indented
+  EXPECT_EQ(Code, format(Code, 47, 1));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang