[clang-format] Support <>-style proto message fields

[krasimir]

Summary

This patch adds support for <>-style proto message fields inside proto options.
Previously these were wrongly treated as binary operators and as such were
working only by chance for a limited number of cases.

[krasimir]

• Fixed newlines

[krasimir]

• Rebase with master
• Updated implementation with the new tests

[krasimir]

• Remove todo test case

[djasper]

Looks good.

lib/Format/ContinuationIndenter.cpp

 
 // Returns \c true if \c Current starts a new parameter.
 static bool startsNextParameter(const FormatToken &Current,
                                 const FormatStyle &Style) {
   const FormatToken &Previous = *Current.Previous;
   if (Current.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma)
     return true;
+  if (Style.Language == FormatStyle::LK_Proto && Current.is(TT_SelectorName))
+    return true;
   return Previous.is(tok::comma) && !Current.isTrailingComment() &&
          ((Previous.isNot(TT_CtorInitializerComma) ||
            Style.BreakConstructorInitializers !=
                FormatStyle::BCIS_BeforeComma) &&
           (Previous.isNot(TT_InheritanceComma) ||
            !Style.BreakBeforeInheritanceComma));
 }
 
if (HasTrailingCall && State.Stack.size() > 1 &&
       State.Stack.back().LastSpace = State.Column;
   }
 }
 
 unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
                                                  bool DryRun) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
-
   // Extra penalty that needs to be added because of the way certain line
   // breaks are chosen.
   unsigned Penalty = 0;
 
   const FormatToken *PreviousNonComment = Current.getPreviousNonComment();
   const FormatToken *NextNonComment = Previous.getNextNonComment();
   if (!NextNonComment)
     NextNonComment = &Current;
if (PreviousNonComment &&
           TT_LeadingJavaAnnotation) &&
       Current.isNot(TT_BinaryOperator) && !PreviousNonComment->opensScope())
     State.Stack.back().BreakBeforeParameter = true;
 
   // If we break after { or the [ of an array initializer, we should also break
   // before the corresponding } or ].
   if (PreviousNonComment &&
       (PreviousNonComment->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
+      (PreviousNonComment->is(tok::less) && Style.Language == FormatStyle::LK_Proto) ||
        (PreviousNonComment->is(TT_TemplateString) &&
         PreviousNonComment->opensScope())))
     State.Stack.back().BreakBeforeClosingBrace = true;
 
   if (State.Stack.back().AvoidBinPacking) {
     // If we are breaking after '(', '{', '<', this is not bin packing
     // unless AllowAllParametersOfDeclarationOnNextLine is false or this is a
     // dict/object literal.
unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   if (Style.Language == FormatStyle::LK_Java &&
       Current.isOneOf(Keywords.kw_implements, Keywords.kw_extends))
     return std::max(State.Stack.back().LastSpace,
                     State.Stack.back().Indent + Style.ContinuationIndentWidth);
 
   if (NextNonComment->is(tok::l_brace) && NextNonComment->BlockKind == BK_Block)
     return Current.NestingLevel == 0 ? State.FirstIndent
                                      : State.Stack.back().Indent;
-  if (Current.isOneOf(tok::r_brace, tok::r_square) && State.Stack.size() > 1) {
+  if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
+       (Current.is(tok::greater) && Style.Language == FormatStyle::LK_Proto)) &&
+      State.Stack.size() > 1) {
     if (Current.closesBlockOrBlockTypeList(Style))
       return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
     if (Current.MatchingParen &&
         Current.MatchingParen->BlockKind == BK_BracedInit)
       return State.Stack[State.Stack.size() - 2].LastSpace;
     return State.FirstIndent;
   }
   // Indent a closing parenthesis at the previous level if followed by a semi or
void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
   }
 
   unsigned NewIndent;
   unsigned LastSpace = State.Stack.back().LastSpace;
   bool AvoidBinPacking;
   bool BreakBeforeParameter = false;
   unsigned NestedBlockIndent = std::max(State.Stack.back().StartOfFunctionCall,
                                         State.Stack.back().NestedBlockIndent);
-  if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare)) {
+  if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
+      Style.Language == FormatStyle::LK_Proto) {
     if (Current.opensBlockOrBlockTypeList(Style)) {
       NewIndent = Style.IndentWidth +
                   std::min(State.Column, State.Stack.back().NestedBlockIndent);
     } else {
       NewIndent = State.Stack.back().LastSpace + Style.ContinuationIndentWidth;
     }
     const FormatToken *NextNoComment = Current.getNextNonComment();
     bool EndsInComma = Current.MatchingParen &&

lib/Format/FormatToken.h

struct FormatToken {
   /// \brief Returns \c true if this tokens starts a block-type list, i.e. a
   /// list that should be indented with a block indent.
   bool opensBlockOrBlockTypeList(const FormatStyle &Style) const {
     if (is(TT_TemplateString) && opensScope())
       return true;
     return is(TT_ArrayInitializerLSquare) ||
            (is(tok::l_brace) &&
             (BlockKind == BK_Block || is(TT_DictLiteral) ||
-             (!Style.Cpp11BracedListStyle && NestingLevel == 0)));
+             (!Style.Cpp11BracedListStyle && NestingLevel == 0))) ||
+           (is(tok::less) && Style.Language == FormatStyle::LK_Proto);
   }
 
   /// \brief Same as opensBlockOrBlockTypeList, but for the closing token.
   bool closesBlockOrBlockTypeList(const FormatStyle &Style) const {
     if (is(TT_TemplateString) && closesScope())
       return true;
     return MatchingParen && MatchingParen->opensBlockOrBlockTypeList(Style);
   }

lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
         return true;
       }
       if (CurrentToken->is(tok::question) &&
           Style.Language == FormatStyle::LK_Java) {
         next();
         continue;
       }
       if (CurrentToken->isOneOf(tok::r_paren, tok::r_square, tok::r_brace) ||
-          (CurrentToken->isOneOf(tok::colon, tok::question) && InExprContext))
+          (CurrentToken->isOneOf(tok::colon, tok::question) && InExprContext &&
+           Style.Language != FormatStyle::LK_Proto))
         return false;
       // If a && or || is found and interpreted as a binary operator, this set
       // of angles is likely part of something like "a < b && c > d". If the
       // angles are inside an expression, the ||/&& might also be a binary
       // operator that was misinterpreted because we are parsing template
       // parameters.
       // FIXME: This is getting out of hand, write a decent parser.
       if (CurrentToken->Previous->isOneOf(tok::pipepipe, tok::ampamp) &&
           CurrentToken->Previous->is(TT_BinaryOperator) &&
           Contexts[Contexts.size() - 2].IsExpression &&
           !Line.startsWith(tok::kw_template))
         return false;
       updateParameterCount(Left, CurrentToken);
+      if (Style.Language == FormatStyle::LK_Proto) {
+        if (FormatToken *Previous = CurrentToken->getPreviousNonComment()) {
+          if (CurrentToken->is(tok::colon) ||
+              (CurrentToken->isOneOf(tok::l_brace, tok::less) &&
+               Previous->isNot(tok::colon)))
+            Previous->Type = TT_SelectorName;
+        }
+      }
       if (!consumeToken())
         return false;
     }
     return false;
   }
 
   bool parseParens(bool LookForDecls = false) {
     if (!CurrentToken)
if (CurrentToken) {
           Left->MatchingParen = CurrentToken;
           CurrentToken->MatchingParen = Left;
           next();
           return true;
         }
         if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
           return false;
         updateParameterCount(Left, CurrentToken);
-        if (CurrentToken->isOneOf(tok::colon, tok::l_brace)) {
+        if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
           FormatToken *Previous = CurrentToken->getPreviousNonComment();
           if (((CurrentToken->is(tok::colon) &&
                 (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
                Style.Language == FormatStyle::LK_Proto) &&
               (Previous->Tok.getIdentifierInfo() ||
                Previous->is(tok::string_literal)))
             Previous->Type = TT_SelectorName;
           if (CurrentToken->is(tok::colon) ||
if (CurrentToken && CurrentToken->is(tok::less)) {
         CurrentToken->Previous->ClosesTemplateDeclaration = true;
       return true;
     }
     return false;
   }
 
   bool consumeToken() {
     FormatToken *Tok = CurrentToken;
+
     next();
     switch (Tok->Tok.getKind()) {
     case tok::plus:
     case tok::minus:
       if (!Tok->Previous && Line.MustBeDeclaration)
         Tok->Type = TT_ObjCMethodSpecifier;
       break;
     case tok::colon:
bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
   if (Style.BreakBeforeInheritanceComma &&
       Right.is(TT_InheritanceComma))
    return true;
   if (Right.is(tok::string_literal) && Right.TokenText.startswith("R\""))
     // Raw string literals are special wrt. line breaks. The author has made a
     // deliberate choice and might have aligned the contents of the string
     // literal accordingly. Thus, we try keep existing line breaks.
     return Right.NewlinesBefore > 0;
-  if (Right.Previous->is(tok::l_brace) && Right.NestingLevel == 1 &&
-      Style.Language == FormatStyle::LK_Proto)
-    // Don't put enums onto single lines in protocol buffers.
+  if (Right.Previous->isOneOf(tok::l_brace, tok::less) &&
+      Right.NestingLevel == 1 && Style.Language == FormatStyle::LK_Proto)
+    // Don't put enums or option definitions onto single lines in protocol
+    // buffers.
     return true;
   if (Right.is(TT_InlineASMBrace))
     return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
     return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
            (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
            (Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)

lib/Format/UnwrappedLineParser.h

private:
   void parsePPIf(bool IfDef);
   void parsePPElIf();
   void parsePPElse();
   void parsePPEndIf();
   void parsePPUnknown();
   void readTokenWithJavaScriptASI();
   void parseStructuralElement();
   bool tryToParseBracedList();
-  bool parseBracedList(bool ContinueOnSemicolons = false);
+  bool parseBracedList(bool ContinueOnSemicolons = false,
+                       tok::TokenKind ClosingBraceKind = tok::r_brace);
   void parseParens();
   void parseSquare();
   void parseIfThenElse();
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
   void parseLabel();
   void parseCaseLabel();

lib/Format/UnwrappedLineParser.cpp

case tok::equal:
       if (FormatTok->is(TT_JsFatArrow)) {
         nextToken();
         if (FormatTok->is(tok::l_brace))
           parseChildBlock();
         break;
       }
 
       nextToken();
-      if (FormatTok->Tok.is(tok::l_brace)) {
+      if (FormatTok->Tok.is(tok::l_brace))
         parseBracedList();
-      }
+      else if (Style.Language == FormatStyle::LK_Proto &&
+               FormatTok->Tok.is(tok::less))
+        parseBracedList(/*ClosingBraceKind=*/tok::greater);
       break;
     case tok::l_square:
       parseSquare();
       break;
     case tok::kw_new:
       parseNew();
       break;
     default:
if (FormatTok->BlockKind == BK_Unknown)
     calculateBraceTypes();
   assert(FormatTok->BlockKind != BK_Unknown);
   if (FormatTok->BlockKind == BK_Block)
     return false;
   parseBracedList();
   return true;
 }
 
-bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons) {
+bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
+                                          tok::TokenKind ClosingBraceKind) {
   bool HasError = false;
   nextToken();
 
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
   // replace this by using parseAssigmentExpression() inside.
   do {
     if (Style.Language == FormatStyle::LK_JavaScript) {
       if (FormatTok->is(Keywords.kw_function) ||
if (Style.Language == FormatStyle::LK_JavaScript) {
       }
       if (FormatTok->is(tok::l_brace)) {
         // Could be a method inside of a braced list `{a() { return 1; }}`.
         if (tryToParseBracedList())
           continue;
         parseChildBlock();
       }
     }
+    if (FormatTok->Tok.getKind() == ClosingBraceKind) {
+      nextToken();
+      return !HasError;
+    }
     switch (FormatTok->Tok.getKind()) {
     case tok::caret:
       nextToken();
       if (FormatTok->is(tok::l_brace)) {
         parseChildBlock();
       }
       break;
     case tok::l_square:
case tok::l_paren:
       }
       break;
     case tok::l_brace:
       // Assume there are no blocks inside a braced init list apart
       // from the ones we explicitly parse out (like lambdas).
       FormatTok->BlockKind = BK_BracedInit;
       parseBracedList();
       break;
-    case tok::r_brace:
-      nextToken();
-      return !HasError;
     case tok::semi:
       // JavaScript (or more precisely TypeScript) can have semicolons in braced
       // lists (in so-called TypeMemberLists). Thus, the semicolon cannot be
       // used for error recovery if we have otherwise determined that this is
       // a braced list.
       if (Style.Language == FormatStyle::LK_JavaScript) {
         nextToken();
         break;

unittests/Format/FormatTestProto.cpp

 //===- unittest/Format/FormatTestProto.cpp --------------------------------===//
 //
+//
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "FormatTestUtils.h"
verifyFormat("option (MyProto.options) = {\n"
                "  msg_field{field_d: 123}\n"
                "};");
 
   // Support syntax with <> instead of {}.
   verifyFormat("option (MyProto.options) = {\n"
                "  field_c: \"OK\",\n"
                "  msg_field: <field_d: 123>\n"
                "};");
+
+  verifyFormat("option (MyProto.options) = {\n"
+               "  msg_field: <>\n"
+               "  field_c: \"OK\",\n"
+               "  msg_field: <field_d: 123>\n"
+               "  field_e: OK\n"
+               "  msg_field: <field_d: 12>\n"
+               "};");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: OK\n"
+               "  field_b: \"OK\"\n"
+               "  field_c: 1\n"
+               "  field_d: 12.5\n"
+               "  field_e: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: OK,\n"
+               "  field_b: \"OK\",\n"
+               "  field_c: 1,\n"
+               "  field_d: 12.5,\n"
+               "  field_e: OK,\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field: {field_b: OK}\n"
+               "  field_g: OK\n"
+               "  field_g: OK\n"
+               "  field_g: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field<\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  >\n"
+               "  field_g: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field<\n"
+               "    field_b: OK,\n"
+               "    field_c: OK,\n"
+               "    field_d: OK,\n"
+               "    field_e: OK,\n"
+               "    field_f: OK\n"
+               "  >\n"
+               "  field_g: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field: <\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  >\n"
+               "  field_g: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field: {\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  }\n"
+               "  field_g: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field{\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  }\n"
+               "  field_g: OK\n"
+               ">;");
+
+  verifyFormat("option (MyProto.options) = {\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field<\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  >\n"
+               "  field_g: OK\n"
+               "};");
+
+  verifyFormat("option (MyProto.options) = {\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field: <\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  >\n"
+               "  field_g: OK\n"
+               "};");
+
+  verifyFormat("option (MyProto.options) = <\n"
+               "  field_a: \"OK\"\n"
+               "  msg_field{\n"
+               "    field_b: OK\n"
+               "    field_c: OK\n"
+               "    field_d: OK\n"
+               "    msg_field<\n"
+               "      field_A: 1\n"
+               "      field_B: 2\n"
+               "      field_C: 3\n"
+               "      field_D: 4\n"
+               "      field_E: 5\n"
+               "    >\n"
+               "    msg_field<field_A: 1 field_B: 2 field_C: 3 field_D: 4>\n"
+               "    field_e: OK\n"
+               "    field_f: OK\n"
+               "  }\n"
+               "  field_g: OK\n"
+               ">;");
 }
 
 TEST_F(FormatTestProto, FormatsService) {
   verifyFormat("service SearchService {\n"
                "  rpc Search(SearchRequest) returns (SearchResponse) {\n"
                "    option foo = true;\n"
                "  }\n"
                "};");