[clang-format] Adds space around angle brackets in text protos

[krasimir]

Summary

This patch adds spaces around angle brackets in text proto Google style.
Previously these were detected as template openers and closers, which happened
to have the expected effect. Now we detect them as scope openers and closers
similarly to the way braces are handled in this context.

[krasimir]

• Remove braces around single statement

[djasper]

lib/Format/TokenAnnotator.cpp

82

Here and in many other places, you are only checking against LK_TextProto. However, in LK_Proto, we also have a lot of text-formatted protos in field options. So, most places, these two should result in the same behavior, I think.

2517

No braces.

[krasimir]

• Address review comments

[djasper]

Looks good.

lib/Format/ContinuationIndenter.cpp

if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
       (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
     return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
     return State.Stack[State.Stack.size() - 2].LastSpace;
   if (Current.is(tok::identifier) && Current.Next &&
       (Current.Next->is(TT_DictLiteral) ||
        ((Style.Language == FormatStyle::LK_Proto ||
          Style.Language == FormatStyle::LK_TextProto) &&
-        Current.Next->isOneOf(TT_TemplateOpener, tok::l_brace))))
+        Current.Next->isOneOf(tok::less, tok::l_brace))))
     return State.Stack.back().Indent;
   if (NextNonComment->is(TT_ObjCStringLiteral) &&
       State.StartOfStringLiteral != 0)
     return State.StartOfStringLiteral - 1;
   if (NextNonComment->isStringLiteral() && State.StartOfStringLiteral != 0)
     return State.StartOfStringLiteral;
   if (NextNonComment->is(tok::lessless) &&
       State.Stack.back().FirstLessLess != 0)
void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
   if (!Current.closesScope())
     return;
 
   // If we encounter a closing ), ], } or >, we can remove a level from our
   // stacks.
   if (State.Stack.size() > 1 &&
       (Current.isOneOf(tok::r_paren, tok::r_square, TT_TemplateString) ||
        (Current.is(tok::r_brace) && State.NextToken != State.Line->First) ||
-       State.NextToken->is(TT_TemplateCloser)))
+       State.NextToken->is(TT_TemplateCloser) ||
+       (Current.is(tok::greater) && Current.is(TT_DictLiteral))))
     State.Stack.pop_back();
 
   if (Current.is(tok::r_square)) {
     // If this ends the array subscript expr, reset the corresponding value.
     const FormatToken *NextNonComment = Current.getNextNonComment();
     if (NextNonComment && NextNonComment->isNot(tok::l_square))
       State.Stack.back().StartOfArraySubscripts = 0;
   }

lib/Format/FormatToken.h

struct FormatToken {
   bool isObjCAccessSpecifier() const {
     return is(tok::at) && Next &&
            (Next->isObjCAtKeyword(tok::objc_public) ||
             Next->isObjCAtKeyword(tok::objc_protected) ||
             Next->isObjCAtKeyword(tok::objc_package) ||
             Next->isObjCAtKeyword(tok::objc_private));
   }
 
-  /// \brief Returns whether \p Tok is ([{ or a template opening <.
+  /// \brief Returns whether \p Tok is ([{ or an opening < of a template or in
+  /// protos.
   bool opensScope() const {
     if (is(TT_TemplateString) && TokenText.endswith("${"))
       return true;
+    if (is(TT_DictLiteral) && is(tok::less))
+      return true;
     return isOneOf(tok::l_paren, tok::l_brace, tok::l_square,
                    TT_TemplateOpener);
   }
-  /// \brief Returns whether \p Tok is )]} or a template closing >.
+  /// \brief Returns whether \p Tok is )]} or a closing > of a template or in
+  /// protos.
   bool closesScope() const {
     if (is(TT_TemplateString) && TokenText.startswith("}"))
       return true;
+    if (is(TT_DictLiteral) && is(tok::greater))
+      return true;
     return isOneOf(tok::r_paren, tok::r_brace, tok::r_square,
                    TT_TemplateCloser);
   }
 
   /// \brief Returns \c true if this is a "." or "->" accessing a member.
   bool isMemberAccess() const {
     return isOneOf(tok::arrow, tok::period, tok::arrowstar) &&
            !isOneOf(TT_DesignatedInitializerPeriod, TT_TrailingReturnArrow,

lib/Format/TokenAnnotator.cpp

bool parseAngle() {
     if (Style.Language == FormatStyle::LK_Java &&
         CurrentToken->is(tok::question))
       next();
 
     while (CurrentToken) {
       if (CurrentToken->is(tok::greater)) {
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
+        if (Style.Language == FormatStyle::LK_TextProto)
+          CurrentToken->Type = TT_DictLiteral;
+        else
           CurrentToken->Type = TT_TemplateCloser;
         next();
         return true;
       }
       if (CurrentToken->is(tok::question) &&
           Style.Language == FormatStyle::LK_Java) {
         next();
         continue;
       }
case tok::l_brace:
       }
       if (!parseBrace())
         return false;
       break;
     case tok::less:
       if (parseAngle()) {
         Tok->Type = TT_TemplateOpener;
         if (Style.Language == FormatStyle::LK_TextProto) {
+          Tok->Type = TT_DictLiteral;
           FormatToken *Previous = Tok->getPreviousNonComment();
           if (Previous && Previous->Type != TT_DictLiteral)
             Previous->Type = TT_SelectorName;
         }
       } else {
         Tok->Type = TT_BinaryOperator;
         NonTemplateLess.insert(Tok);
         CurrentToken = Tok;
         next();
       }
       break;
     case tok::r_paren:
     case tok::r_square:
       return false;
     case tok::r_brace:
       // Lines can start with '}'.
       if (Tok->Previous)
         return false;
       break;
     case tok::greater:
+      if (Style.Language != FormatStyle::LK_TextProto)
         Tok->Type = TT_BinaryOperator;
       break;
     case tok::kw_operator:
       while (CurrentToken &&
              !CurrentToken->isOneOf(tok::l_paren, tok::semi, tok::r_paren)) {
         if (CurrentToken->isOneOf(tok::star, tok::amp))
           CurrentToken->Type = TT_PointerOrReference;
         consumeToken();
         if (CurrentToken &&
if (Current.is(Keywords.kw_instanceof)) {
           Line.MustBeDeclaration && !Contexts.back().IsExpression) {
         // In JavaScript, `interface X { foo?(): bar; }` is an optional method
         // on the interface, not a ternary expression.
         Current.Type = TT_JsTypeOptionalQuestion;
       } else {
         Current.Type = TT_ConditionalExpr;
       }
     } else if (Current.isBinaryOperator() &&
-               (!Current.Previous || Current.Previous->isNot(tok::l_square))) {
+               (!Current.Previous || Current.Previous->isNot(tok::l_square)) &&
+               (!Current.is(tok::greater) &&
+                Style.Language != FormatStyle::LK_TextProto)) {
       Current.Type = TT_BinaryOperator;
     } else if (Current.is(tok::comment)) {
       if (Current.TokenText.startswith("/*")) {
         if (Current.TokenText.endswith("*/"))
           Current.Type = TT_BlockComment;
         else
           // The lexer has for some reason determined a comment here. But we
           // cannot really handle it, if it isn't properly terminated.
if (Left.is(tok::at) &&
       Right.isOneOf(tok::identifier, tok::string_literal, tok::char_constant,
                     tok::numeric_constant, tok::l_paren, tok::l_brace,
                     tok::kw_true, tok::kw_false))
     return false;
   if (Left.is(tok::colon))
     return !Left.is(TT_ObjCMethodExpr);
   if (Left.is(tok::coloncolon))
     return false;
-  if (Left.is(tok::less) || Right.isOneOf(tok::greater, tok::less))
+  if (Left.is(tok::less) || Right.isOneOf(tok::greater, tok::less)) {
+    if (Style.Language == FormatStyle::LK_TextProto) {
+      // Format empty list as `<>`.
+      if (Left.is(tok::less) && Right.is(tok::greater))
+        return false;
+      return !Style.Cpp11BracedListStyle;
+    }
     return false;
+  }
   if (Right.is(tok::ellipsis))
     return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous &&
                                     Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
     return false;
   if (Right.is(TT_PointerOrReference)) {
     if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
       if (!Left.MatchingParen)
if (Left.is(TT_UnaryOperator))
     return Right.is(TT_BinaryOperator);
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
   if (Left.is(TT_CastRParen))
     return Style.SpaceAfterCStyleCast ||
            Right.isOneOf(TT_BinaryOperator, TT_SelectorName);
 
-  if (Left.is(tok::greater) && Right.is(tok::greater))
+  if (Left.is(tok::greater) && Right.is(tok::greater)) {
+    if (Style.Language == FormatStyle::LK_TextProto) {
+      return !Style.Cpp11BracedListStyle;
+    }
     return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
            (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
+  }
   if (Right.isOneOf(tok::arrow, tok::arrowstar, tok::periodstar) ||
       Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
       (Right.is(tok::period) && Right.isNot(TT_DesignatedInitializerPeriod)))
     return false;
   if (!Style.SpaceBeforeAssignmentOperators &&
       Right.getPrecedence() == prec::Assignment)
     return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))

unittests/Format/FormatTestRawStrings.cpp

expect_eq(
           R"test(P p = TP(R"pb(  item_1 :  1   )pb");)test",
           getRawStringPbStyleWithColumns(40)));
   expect_eq(
       R"test(P p = TP(R"pb(item_1: 1 item_2: 2)pb");)test",
       format(
           R"test(P p = TP(R"pb(item_1:1 item_2:2)pb");)test",
           getRawStringPbStyleWithColumns(40)));
   expect_eq(
-      R"test(P p = TP(R"pb(item_1 <1> item_2: { 2 })pb");)test",
+      R"test(P p = TP(R"pb(item_1 < 1 > item_2: { 2 })pb");)test",
       format(
           R"test(P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
           getRawStringPbStyleWithColumns(40)));
 
   // Merge two short lines into one.
   expect_eq(R"test(
 std::string s = R"pb(
   item_1: 1 item_2: 2
TPPPPPPPPPPPPPPP(
         R"pb(item_1: 1,
              item_2: 2,
              item_3: 3)pb");)test",
             format(R"test(
 P p = TPPPPPPPPPPPPPPP(R"pb(item_1: 1, item_2: 2, item_3: 3)pb");)test",
                    getRawStringPbStyleWithColumns(40)));
 
   expect_eq(R"test(
-P p = TP(R"pb(item_1 <1>
-              item_2: <2>
+P p = TP(R"pb(item_1 < 1 >
+              item_2: < 2 >
               item_3 {})pb");)test",
       format(R"test(
 P p = TP(R"pb(item_1<1> item_2:<2> item_3{ })pb");)test",
           getRawStringPbStyleWithColumns(40)));
 
   expect_eq(
       R"test(
 P p = TP(R"pb(item_1: 1,
               item_2: 2,
               item_3: 3,
               item_4: 4)pb");)test",
       format(
           R"test(
 P p = TP(R"pb(item_1: 1, item_2: 2, item_3: 3, item_4: 4)pb");)test",
           getRawStringPbStyleWithColumns(40)));
 
   expect_eq(R"test(
 P p = TPPPPPPPPPPPPPPP(
-    R"pb(item_1 <1>,
+    R"pb(item_1 < 1 >,
          item_2: { 2 },
-         item_3: <3>,
+         item_3: < 3 >,
          item_4: { 4 })pb");)test",
             format(R"test(
 P p = TPPPPPPPPPPPPPPP(R"pb(item_1<1>, item_2: {2}, item_3: <3>, item_4:{4})pb");)test",
                    getRawStringPbStyleWithColumns(40)));
 
   // Breaks before a short raw string exceeding the column limit.
   expect_eq(R"test(
 FFFFFFFFFFFFFFFFFFFFFFFFFFF(

unittests/Format/FormatTestTextProto.cpp

verifyFormat("field_a: OK\n"
                "  field_b: OK  // Comment\n"
                "}");
 }
 
 TEST_F(FormatTestTextProto, SupportsAngleBracketMessageFields) {
   // Single-line tests
   verifyFormat("msg_field <>");
   verifyFormat("msg_field: <>");
-  verifyFormat("msg_field <field_a: OK>");
-  verifyFormat("msg_field: <field_a: 123>");
-  verifyFormat("msg_field <field_a <>>");
-  verifyFormat("msg_field <field_a <field_b <>>>");
-  verifyFormat("msg_field: <field_a <field_b: <>>>");
-  verifyFormat("msg_field <field_a: OK, field_b: \"OK\">");
-  verifyFormat("msg_field <field_a: OK field_b: <>, field_c: OK>");
-  verifyFormat("msg_field <field_a { field_b: 1 }, field_c: <field_d: 2>>");
-  verifyFormat("msg_field: <field_a: OK, field_b: \"OK\">");
-  verifyFormat("msg_field: <field_a: OK field_b: <>, field_c: OK>");
-  verifyFormat("msg_field: <field_a { field_b: 1 }, field_c: <field_d: 2>>");
-  verifyFormat("field_a: \"OK\", msg_field: <field_b: 123>, field_c: {}");
-  verifyFormat("field_a <field_b: 1>, msg_field: <field_b: 123>, field_c <>");
-  verifyFormat("field_a <field_b: 1> msg_field: <field_b: 123> field_c <>");
-  verifyFormat("field <field <field: <>>, field <>> field: <field: 1>");
+  verifyFormat("msg_field < field_a: OK >");
+  verifyFormat("msg_field: < field_a: 123 >");
+  verifyFormat("msg_field < field_a <> >");
+  verifyFormat("msg_field < field_a < field_b <> > >");
+  verifyFormat("msg_field: < field_a < field_b: <> > >");
+  verifyFormat("msg_field < field_a: OK, field_b: \"OK\" >");
+  verifyFormat("msg_field < field_a: OK field_b: <>, field_c: OK >");
+  verifyFormat("msg_field < field_a { field_b: 1 }, field_c: < f_d: 2 > >");
+  verifyFormat("msg_field: < field_a: OK, field_b: \"OK\" >");
+  verifyFormat("msg_field: < field_a: OK field_b: <>, field_c: OK >");
+  verifyFormat("msg_field: < field_a { field_b: 1 }, field_c: < fd_d: 2 > >");
+  verifyFormat("field_a: \"OK\", msg_field: < field_b: 123 >, field_c: {}");
+  verifyFormat("field_a < field_b: 1 >, msg_fid: < fiel_b: 123 >, field_c <>");
+  verifyFormat("field_a < field_b: 1 > msg_fied: < field_b: 123 > field_c <>");
+  verifyFormat("field < field < field: <> >, field <> > field: < field: 1 >");
 
   // Multiple lines tests
   verifyFormat("msg_field <\n"
                "  field_a: OK\n"
                "  field_b: \"OK\"\n"
                "  field_c: 1\n"
                "  field_d: 12.5\n"
                "  field_e: OK\n"
                ">");
 
   verifyFormat("msg_field: <>\n"
                "field_c: \"OK\",\n"
-               "msg_field: <field_d: 123>\n"
+               "msg_field: < field_d: 123 >\n"
                "field_e: OK\n"
-               "msg_field: <field_d: 12>");
+               "msg_field: < field_d: 12 >");
 
   verifyFormat("field_a: OK,\n"
-               "field_b <field_c: OK>,\n"
-               "field_d: <12.5>,\n"
+               "field_b < field_c: OK >,\n"
+               "field_d: < 12.5 >,\n"
                "field_e: OK");
 
   verifyFormat("field_a: OK\n"
-               "field_b <field_c: OK>\n"
-               "field_d: <12.5>\n"
+               "field_b < field_c: OK >\n"
+               "field_d: < 12.5 >\n"
                "field_e: OKOKOK");
 
   verifyFormat("msg_field <\n"
                "  field_a: OK,\n"
-               "  field_b <field_c: OK>,\n"
-               "  field_d: <12.5>,\n"
+               "  field_b < field_c: OK >,\n"
+               "  field_d: < 12.5 >,\n"
                "  field_e: OK\n"
                ">");
 
   verifyFormat("msg_field <\n"
-               "  field_a: <field: OK>,\n"
-               "  field_b <field_c: OK>,\n"
-               "  field_d: <12.5>,\n"
+               "  field_a: < field: OK >,\n"
+               "  field_b < field_c: OK >,\n"
+               "  field_d: < 12.5 >,\n"
                "  field_e: OK,\n"
                ">");
 
   verifyFormat("msg_field: <\n"
                "  field_a: \"OK\"\n"
                "  msg_field: { field_b: OK }\n"
                "  field_g: OK\n"
                "  field_g: OK\n"
                "  field_g: OK\n"
                ">");
 
   verifyFormat("field_a {\n"
                "  field_d: ok\n"
-               "  field_b: <field_c: 1>\n"
+               "  field_b: < field_c: 1 >\n"
                "  field_d: ok\n"
                "  field_d: ok\n"
                "}");
 
   verifyFormat("field_a: {\n"
                "  field_d: ok\n"
-               "  field_b: <field_c: 1>\n"
+               "  field_b: < field_c: 1 >\n"
                "  field_d: ok\n"
                "  field_d: ok\n"
                "}");
 
-  verifyFormat("field_a: <f1: 1, f2: <>>\n"
+  verifyFormat("field_a: < f1: 1, f2: <> >\n"
                "field_b <\n"
                "  field_b1: <>\n"
                "  field_b2: ok,\n"
                "  field_b3: <\n"
                "    field_x {}  // Comment\n"
                "    field_y: { field_z: 1 }\n"
                "    field_w: ok\n"
                "  >\n"
                "  field {\n"
                "    field_x <>  // Comment\n"
-               "    field_y: <field_z: 1>\n"
+               "    field_y: < field_z: 1 >\n"
                "    field_w: ok\n"
                "    msg_field: <\n"
                "      field: <>\n"
-               "      field: <field: 1>\n"
-               "      field: <field: 2>\n"
-               "      field: <field: 3>\n"
-               "      field: <field: 4>\n"
+               "      field: < field: 1 >\n"
+               "      field: < field: 2 >\n"
+               "      field: < field: 3 >\n"
+               "      field: < field: 4 >\n"
                "      field: ok\n"
                "    >\n"
                "  }\n"
                ">\n"
                "field: OK,\n"
-               "field_c <field <field <>>>");
+               "field_c < field < field <> > >");
 
   verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
                "head_id: 1\n"
-               "data <key: value>");
+               "data < key: value >");
 
   verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
                "head_id: 1\n"
-               "data <key: value>\n"
+               "data < key: value >\n"
                "tail_id: 2");
 
   verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
                "head_id: 1\n"
-               "data <key: value>\n"
+               "data < key: value >\n"
                "data { key: value }");
 
   verifyFormat("app {\n"
                "  app_id: 'com.javax.swing.salsa.latino'\n"
                "  head_id: 1\n"
-               "  data <key: value>\n"
+               "  data < key: value >\n"
                "}");
 
   verifyFormat("app: {\n"
                "  app_id: 'com.javax.swing.salsa.latino'\n"
                "  head_id: 1\n"
-               "  data <key: value>\n"
+               "  data < key: value >\n"
                "}");
 
   verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
                "headheadheadheadheadhead_id: 1\n"
                "product_data { product { 1 } }");
 
   verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
                "headheadheadheadheadhead_id: 1\n"
-               "product_data <product { 1 }>");
+               "product_data < product { 1 } >");
 
   verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
                "headheadheadheadheadhead_id: 1\n"
-               "product_data <product <1>>");
+               "product_data < product < 1 > >");
 
   verifyFormat("app <\n"
                "  app_id: 'com.javax.swing.salsa.latino'\n"
                "  headheadheadheadheadhead_id: 1\n"
-               "  product_data <product { 1 }>\n"
+               "  product_data < product { 1 } >\n"
                ">");
 
   verifyFormat("dcccwrnfioeruvginerurneitinfo {\n"
                "  exte3nsionrnfvui { key: value }\n"
                "}");
 }
 
 TEST_F(FormatTestTextProto, DiscardsUnbreakableTailIfCanBreakAfter) {