[clang-format] Support python-style comments in text protos

[krasimir]

Summary

This patch adds support for python-style comments in text protos.

[krasimir]

• Add more tests

[bkramer]

lib/Format/BreakableToken.cpp

48

These should stay arrays. You can use ArrayRef to get a convenient reference to them.

[djasper]

lib/Format/FormatTokenLexer.cpp

344

There should be a linebreak here.

lib/Format/UnwrappedLineParser.cpp

59

Maybe it's now better to do:

return FormatTok.is(tok::comment) && !FormatTok.TokenText.startsWith("/*");

?

[krasimir]

• [clang-format] Address review comments

lib/Format/BreakableToken.cpp

static bool IsBlank(char C) {
   case '\f':
   case '\r':
     return true;
   default:
     return false;
   }
 }
 
-static StringRef getLineCommentIndentPrefix(StringRef Comment) {
-  static const char *const KnownPrefixes[] = {"///<", "//!<", "///", "//",
-                                              "//!"};
+static StringRef getLineCommentIndentPrefix(StringRef Comment,
+                                            const FormatStyle &Style) {
+  static const SmallVector<StringRef, 8> KnownCStylePrefixes{
+      "///<", "//!<", "///", "//", "//!"};
+  static const SmallVector<StringRef, 8> KnownTextProtoPrefixes{"//", "#"};
+  const SmallVectorImpl<StringRef> &KnownPrefixes =
+      (Style.Language == FormatStyle::LK_TextProto) ? KnownTextProtoPrefixes
+                                                    : KnownCStylePrefixes;
+
   StringRef LongestPrefix;
   for (StringRef KnownPrefix : KnownPrefixes) {
     if (Comment.startswith(KnownPrefix)) {
       size_t PrefixLength = KnownPrefix.size();
       while (PrefixLength < Comment.size() && Comment[PrefixLength] == ' ')
         ++PrefixLength;
       if (PrefixLength > LongestPrefix.size())
         LongestPrefix = Comment.substr(0, PrefixLength);
BreakableLineCommentSection::BreakableLineCommentSection(
   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);
-    assert(TokenText.startswith("//"));
+    assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
+           "unsupported line comment prefix, '//' and '#' are supported");
     size_t FirstLineIndex = Lines.size();
     TokenText.split(Lines, "\n");
     Content.resize(Lines.size());
     ContentColumn.resize(Lines.size());
     OriginalContentColumn.resize(Lines.size());
     Tokens.resize(Lines.size());
     Prefix.resize(Lines.size());
     OriginalPrefix.resize(Lines.size());
     for (size_t i = FirstLineIndex, e = Lines.size(); i < e; ++i) {
       // We need to trim the blanks in case this is not the first line in a
       // multiline comment. Then the indent is included in Lines[i].
       StringRef IndentPrefix =
-          getLineCommentIndentPrefix(Lines[i].ltrim(Blanks));
-      assert(IndentPrefix.startswith("//"));
+          getLineCommentIndentPrefix(Lines[i].ltrim(Blanks), Style);
+      assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
+             "unsupported line comment prefix, '//' and '#' are supported");
       OriginalPrefix[i] = Prefix[i] = IndentPrefix;
       if (Lines[i].size() > Prefix[i].size() &&
           isAlphanumeric(Lines[i][Prefix[i].size()])) {
         if (Prefix[i] == "//")
           Prefix[i] = "// ";
         else if (Prefix[i] == "///")
           Prefix[i] = "/// ";
         else if (Prefix[i] == "//!")
           Prefix[i] = "//! ";
         else if (Prefix[i] == "///<")
           Prefix[i] = "///< ";
         else if (Prefix[i] == "//!<")
           Prefix[i] = "//!< ";
+        else if (Prefix[i] == "#" &&
+                 Style.Language == FormatStyle::LK_TextProto)
+          Prefix[i] = "# ";
       }
 
       Tokens[i] = LineTok;
       Content[i] = Lines[i].substr(IndentPrefix.size());
       OriginalContentColumn[i] =
           StartColumn + encoding::columnWidthWithTabs(OriginalPrefix[i],
                                                       StartColumn,
                                                       Style.TabWidth, Encoding);

lib/Format/FormatTokenLexer.h

private:
   // embedding expressions nested in ${expr-here}. Template strings can be
   // nested recursively, i.e. expressions can contain template strings in turn.
   //
   // The code below parses starting from a backtick, up to a closing backtick or
   // an opening ${. It also maintains a stack of lexing contexts to handle
   // nested template parts by balancing curly braces.
   void handleTemplateStrings();
 
+  void tryParsePythonComment();
+
   bool tryMerge_TMacro();
 
   bool tryMergeConflictMarkers();
 
   FormatToken *getStashedToken();
 
   FormatToken *getNextToken();
 

lib/Format/FormatTokenLexer.cpp

ArrayRef<FormatToken *> FormatTokenLexer::lex() {
   assert(Tokens.empty());
   assert(FirstInLineIndex == 0);
   do {
     Tokens.push_back(getNextToken());
     if (Style.Language == FormatStyle::LK_JavaScript) {
       tryParseJSRegexLiteral();
       handleTemplateStrings();
     }
+    if (Style.Language == FormatStyle::LK_TextProto)
+      tryParsePythonComment();
     tryMergePreviousTokens();
     if (Tokens.back()->NewlinesBefore > 0 || Tokens.back()->IsMultiline)
       FirstInLineIndex = Tokens.size() - 1;
   } while (Tokens.back()->Tok.isNot(tok::eof));
   return Tokens;
 }
 
 void FormatTokenLexer::tryMergePreviousTokens() {
void FormatTokenLexer::handleTemplateStrings() {
   }
 
   SourceLocation loc = Offset < Lex->getBuffer().end()
                            ? Lex->getSourceLocation(Offset + 1)
                            : SourceMgr.getLocForEndOfFile(ID);
   resetLexer(SourceMgr.getFileOffset(loc));
 }
 
+void FormatTokenLexer::tryParsePythonComment() {
+  FormatToken *HashToken = Tokens.back();
+  if (HashToken->isNot(tok::hash))
+    return;
+  // Turn the remainder of this line into a comment.
+  const char *CommentBegin =
+      Lex->getBufferLocation() - HashToken->TokenText.size(); // at "#"
+  size_t From = CommentBegin - Lex->getBuffer().begin();
+  size_t To = Lex->getBuffer().find_first_of('\n', From);
+  if (To == StringRef::npos) To = Lex->getBuffer().size();
+  size_t Len = To - From;
+  HashToken->Type = TT_LineComment;
+  HashToken->Tok.setKind(tok::comment);
+  HashToken->TokenText = Lex->getBuffer().substr(From, Len);
+  SourceLocation Loc = To < Lex->getBuffer().size()
+                           ? Lex->getSourceLocation(CommentBegin + Len)
+                           : SourceMgr.getLocForEndOfFile(ID);
+  resetLexer(SourceMgr.getFileOffset(Loc));
+}
+
 bool FormatTokenLexer::tryMerge_TMacro() {
   if (Tokens.size() < 4)
     return false;
   FormatToken *Last = Tokens.back();
   if (!Last->is(tok::r_paren))
     return false;
 
   FormatToken *String = Tokens[Tokens.size() - 2];

lib/Format/UnwrappedLineParser.cpp

public:
   }
 
 private:
   UnwrappedLine &Line;
   std::vector<bool> &Stack;
 };
 
 static bool isLineComment(const FormatToken &FormatTok) {
-  return FormatTok.is(tok::comment) && FormatTok.TokenText.startswith("//");
+  return FormatTok.is(tok::comment) && (FormatTok.TokenText.startswith("//") ||
+                                        FormatTok.TokenText.startswith("#"));
 }
 
 // Checks if \p FormatTok is a line comment that continues the line comment
 // \p Previous. The original column of \p MinColumnToken is used to determine
 // whether \p FormatTok is indented enough to the right to continue \p Previous.
 static bool continuesLineComment(const FormatToken &FormatTok,
                                  const FormatToken *Previous,
                                  const FormatToken *MinColumnToken) {

unittests/Format/FormatTestComments.cpp

protected:
   }
 
   FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
     FormatStyle Style = getLLVMStyle();
     Style.ColumnLimit = ColumnLimit;
     return Style;
   }
 
+  FormatStyle getTextProtoStyleWithColumns(unsigned ColumnLimit) {
+    FormatStyle Style = getGoogleStyle(FormatStyle::FormatStyle::LK_TextProto);
+    Style.ColumnLimit = ColumnLimit;
+    return Style;
+  }
+
   void verifyFormat(llvm::StringRef Code,
                     const FormatStyle &Style = getLLVMStyle()) {
     EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 
   void verifyGoogleFormat(llvm::StringRef Code) {
     verifyFormat(Code, getGoogleStyle());
   }
TEST_F(FormatTestComments, NonTrailingBlockComments) {
 
   EXPECT_EQ("const /** comment comment comment\n"
             "         comment */\n"
             "    A = B;",
             format("const /** comment comment comment comment */\n"
                    "    A = B;",
                    getLLVMStyleWithColumns(40)));
 }
+
+TEST_F(FormatTestComments, PythonStyleComments) {
+  // Keeps a space after '#'.
+  EXPECT_EQ("# comment\n"
+            "key: value",
+            format("#comment\n"
+                   "key:value",
+                   getTextProtoStyleWithColumns(20)));
+  EXPECT_EQ("# comment\n"
+            "key: value",
+            format("# comment\n"
+                   "key:value",
+                   getTextProtoStyleWithColumns(20)));
+  // Breaks long comment.
+  EXPECT_EQ("# comment comment\n"
+            "# comment\n"
+            "key: value",
+            format("# comment comment comment\n"
+                   "key:value",
+                   getTextProtoStyleWithColumns(20)));
+  // Indents comments.
+  EXPECT_EQ("data {\n"
+            "  # comment comment\n"
+            "  # comment\n"
+            "  key: value\n"
+            "}",
+            format("data {\n"
+                   "# comment comment comment\n"
+                   "key: value}",
+                   getTextProtoStyleWithColumns(20)));
+  EXPECT_EQ("data {\n"
+            "  # comment comment\n"
+            "  # comment\n"
+            "  key: value\n"
+            "}",
+            format("data {# comment comment comment\n"
+                   "key: value}",
+                   getTextProtoStyleWithColumns(20)));
+  // Reflows long comments.
+  EXPECT_EQ("# comment comment\n"
+            "# comment comment\n"
+            "key: value",
+            format("# comment comment comment\n"
+                   "# comment\n"
+                   "key:value",
+                   getTextProtoStyleWithColumns(20)));
+  // Breaks trailing comments.
+  EXPECT_EQ("k: val  # comment\n"
+            "        # comment\n"
+            "a: 1",
+            format("k:val#comment comment\n"
+                   "a:1",
+                   getTextProtoStyleWithColumns(20)));
+  EXPECT_EQ("id {\n"
+            "  k: val  # comment\n"
+            "          # comment\n"
+            "  # line line\n"
+            "  a: 1\n"
+            "}",
+            format("id {k:val#comment comment\n"
+                   "# line line\n"
+                   "a:1}",
+                   getTextProtoStyleWithColumns(20)));
+  // Aligns trailing comments.
+  EXPECT_EQ("k: val  # commen1\n"
+            "        # commen2\n"
+            "        # commen3\n"
+            "# commen4\n"
+            "a: 1  # commen5\n"
+            "      # commen6\n"
+            "      # commen7",
+            format("k:val#commen1 commen2\n"
+                   " # commen3\n"
+                   "# commen4\n"
+                   "a:1#commen5 commen6\n"
+                   " #commen7",
+                   getTextProtoStyleWithColumns(20)));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang