[clang-format] text protos: put entries on separate lines if there is a submessage

Summary:
This patch updates clang-format text protos to put entries of a submessage into separate lines if the submessage contains at least two entries and contains at least one submessage entry.

For example, the entries here are kept on separate lines even if putting them on a single line would be under the column limit:
```
message: {
  entry: 1
  submessage: { key: value }
}
```

Messages containing a single submessage or several scalar entries can still be put on one line if they fit:
```
message { submessage { key: value } }
message { x: 1 y: 2 z: 3 }
```

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D46757

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@334401 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Format/TokenAnnotator.cpp

@@ -2924,6 +2924,74 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
   if (Right.is(TT_ProtoExtensionLSquare))
     return true;
 
+  // In text proto instances if a submessage contains at least 2 entries and at
+  // least one of them is a submessage, like A { ... B { ... } ... },
+  // put all of the entries of A on separate lines by forcing the selector of
+  // the submessage B to be put on a newline.
+  //
+  // Example: these can stay on one line:
+  // a { scalar_1: 1 scalar_2: 2 }
+  // a { b { key: value } }
+  //
+  // and these entries need to be on a new line even if putting them all in one
+  // line is under the column limit:
+  // a {
+  //   scalar: 1
+  //   b { key: value }
+  // }
+  //
+  // We enforce this by breaking before a submessage field that has previous
+  // siblings, *and* breaking before a field that follows a submessage field.
+  //
+  // Be careful to exclude the case  [proto.ext] { ... } since the `]` is
+  // the TT_SelectorName there, but we don't want to break inside the brackets.
+  // We ensure elsewhere that extensions are always on their own line.
+  if ((Style.Language == FormatStyle::LK_Proto ||
+       Style.Language == FormatStyle::LK_TextProto) &&
+      Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
+    // Look for the scope opener after selector in cases like:
+    // selector { ...
+    // selector: { ...
+    FormatToken *LBrace =
+        Right.Next->is(tok::colon) ? Right.Next->Next : Right.Next;
+    if (LBrace &&
+        // The scope opener is one of {, [, <:
+        // selector { ... }
+        // selector [ ... ]
+        // selector < ... >
+        //
+        // In case of selector { ... }, the l_brace is TT_DictLiteral.
+        // In case of an empty selector {}, the l_brace is not TT_DictLiteral,
+        // so we check for immediately following r_brace.
+        ((LBrace->is(tok::l_brace) &&
+          (LBrace->is(TT_DictLiteral) ||
+           (LBrace->Next && LBrace->Next->is(tok::r_brace)))) ||
+         LBrace->is(TT_ArrayInitializerLSquare) || LBrace->is(tok::less))) {
+      // If Left.ParameterCount is 0, then this submessage entry is not the
+      // first in its parent submessage, and we want to break before this entry.
+      // If Left.ParameterCount is greater than 0, then its parent submessage
+      // might contain 1 or more entries and we want to break before this entry
+      // if it contains at least 2 entries. We deal with this case later by
+      // detecting and breaking before the next entry in the parent submessage.
+      if (Left.ParameterCount == 0)
+        return true;
+      // However, if this submessage is the first entry in its parent
+      // submessage, Left.ParameterCount might be 1 in some cases.
+      // We deal with this case later by detecting an entry
+      // following a closing paren of this submessage.
+    }
+    
+    // If this is an entry immediately following a submessage, it will be
+    // preceded by a closing paren of that submessage, like in:
+    //     left---.  .---right
+    //            v  v
+    // sub: { ... } key: value
+    // If there was a comment between `}` an `key` above, then `key` would be
+    // put on a new line anyways.
+    if (Left.isOneOf(tok::r_brace, tok::greater, tok::r_square))
+      return true;
+  }
+
   return false;
 }
 

unittests/Format/FormatTestProto.cpp

@@ -493,5 +493,136 @@ TEST_F(FormatTestProto, AcceptsOperatorAsKeyInOptions) {
                "};");
 }
 
+TEST_F(FormatTestProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+  Style.ColumnLimit = 60;
+  // The column limit allows for the keys submessage to be put on 1 line, but we
+  // break it since it contains a submessage an another entry.
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "    sub <>\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "    sub {}\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub {}\n"
+               "    sub: <>\n"
+               "    sub: []\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub { msg: 1 }\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub: { msg: 1 }\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub < msg: 1 >\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub: [ msg: 1 ]\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: <\n"
+               "    item: 'aaaaaaaaaaa'\n"
+               "    sub: [ 1, 2 ]\n"
+               "  >\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub {}\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub: []\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub <>\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub { key: value }\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub: [ 1, 2 ]\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub < sub_2: {} >\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: data\n"
+               "    sub: [ 1, 2 ]\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    item: data\n"
+               "    sub < sub_2: {} >\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  sub: {\n"
+               "    key: valueeeeeeee\n"
+               "    keys: {\n"
+               "      sub: [ 1, 2 ]\n"
+               "      item: 'aaaaaaaaaaaaaaaa'\n"
+               "    }\n"
+               "  }\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang

unittests/Format/FormatTestRawStrings.cpp

@@ -199,12 +199,6 @@ TEST_F(FormatTestRawStrings, ReformatsShortRawStringsOnSingleLine) {
       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",
-      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(
@@ -220,6 +214,18 @@ std::string s = R"pb(
                    getRawStringPbStyleWithColumns(40)));
 }
 
+TEST_F(FormatTestRawStrings, BreaksShortRawStringsWhenNeeded) {
+  // The raw string contains multiple submessage entries, so break for
+  // readability.
+  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)));
+}
+
 TEST_F(FormatTestRawStrings, BreaksRawStringsExceedingColumnLimit) {
   expect_eq(R"test(
 P p = TPPPPPPPPPPPPPPP(

unittests/Format/FormatTestTextProto.cpp

@@ -171,17 +171,48 @@ TEST_F(FormatTestTextProto, SupportsAngleBracketMessageFields) {
   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: <>,\n"
+               "  field_c: OK\n"
+               ">");
+
+  verifyFormat("msg_field <\n"
+               "  field_a { field_b: 1 },\n"
+               "  field_c: < f_d: 2 >\n"
+               ">");
+
+  verifyFormat("msg_field: <\n"
+               "  field_a: OK\n"
+               "  field_b: <>,\n"
+               "  field_c: OK\n"
+               ">");
+
+  verifyFormat("msg_field: <\n"
+               "  field_a { field_b: 1 },\n"
+               "  field_c: < fd_d: 2 >\n"
+               ">");
+
+  verifyFormat("field_a: \"OK\",\n"
+               "msg_field: < field_b: 123 >,\n"
+               "field_c: {}");
+
+  verifyFormat("field_a < field_b: 1 >,\n"
+               "msg_fid: < fiel_b: 123 >,\n" 
+               "field_c <>");
+
+  verifyFormat("field_a < field_b: 1 >\n"
+               "msg_fied: < field_b: 123 >\n"
+               "field_c <>");
+
+  verifyFormat("field <\n"
+               "  field < field: <> >,\n"
+               "  field <>\n"
+               ">\n"
+               "field: < field: 1 >");
+
   verifyFormat("msg_field <\n"
                "  field_a: OK\n"
                "  field_b: \"OK\"\n"
@@ -242,7 +273,10 @@ TEST_F(FormatTestTextProto, SupportsAngleBracketMessageFields) {
                "  field_d: ok\n"
                "}");
 
-  verifyFormat("field_a: < f1: 1, f2: <> >\n"
+  verifyFormat("field_a: <\n"
+               "  f1: 1,\n"
+               "  f2: <>\n"
+               ">\n"
                "field_b <\n"
                "  field_b1: <>\n"
                "  field_b2: ok,\n"
@@ -529,5 +563,112 @@ TEST_F(FormatTestTextProto, BreaksAfterBraceFollowedByClosingBraceOnNextLine) {
                ">");
 }
 
+TEST_F(FormatTestTextProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+  Style.ColumnLimit = 60;
+  // The column limit allows for the keys submessage to be put on 1 line, but we
+  // break it since it contains a submessage an another entry.
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "  sub <>\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "  sub {}\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub {}\n"
+               "  sub: <>\n"
+               "  sub: []\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub { msg: 1 }\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub: { msg: 1 }\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub < msg: 1 >\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub: [ msg: 1 ]\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: <\n"
+               "  item: 'aaaaaaaaaaa'\n"
+               "  sub: [ 1, 2 ]\n"
+               ">");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub {}\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub: []\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub <>\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub { key: value }\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub: [ 1, 2 ]\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  sub < sub_2: {} >\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: data\n"
+               "  sub: [ 1, 2 ]\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("key: valueeeeeeee\n"
+               "keys: {\n"
+               "  item: data\n"
+               "  sub < sub_2: {} >\n"
+               "  item: 'aaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("sub: {\n"
+               "  key: valueeeeeeee\n"
+               "  keys: {\n"
+               "    sub: [ 1, 2 ]\n"
+               "    item: 'aaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("sub: {\n"
+               "  key: 1\n"
+               "  sub: {}\n"
+               "}\n"
+               "# comment\n");
+  verifyFormat("sub: {\n"
+               "  key: 1\n"
+               "  # comment\n"
+               "  sub: {}\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang