r334401 - [clang-format] text protos: put entries on separate lines if there is a submessage
Krasimir Georgiev via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 11 05:53:25 PDT 2018
Author: krasimir
Date: Mon Jun 11 05:53:25 2018
New Revision: 334401
URL: http://llvm.org/viewvc/llvm-project?rev=334401&view=rev
Log:
[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
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=334401&r1=334400&r2=334401&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Jun 11 05:53:25 2018
@@ -2924,6 +2924,74 @@ bool TokenAnnotator::mustBreakBefore(con
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;
}
Modified: cfe/trunk/unittests/Format/FormatTestProto.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=334401&r1=334400&r2=334401&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp Mon Jun 11 05:53:25 2018
@@ -493,5 +493,136 @@ TEST_F(FormatTestProto, AcceptsOperatorA
"};");
}
+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
Modified: cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestRawStrings.cpp?rev=334401&r1=334400&r2=334401&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestRawStrings.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestRawStrings.cpp Mon Jun 11 05:53:25 2018
@@ -199,12 +199,6 @@ TEST_F(FormatTestRawStrings, ReformatsSh
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(
Modified: cfe/trunk/unittests/Format/FormatTestTextProto.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestTextProto.cpp?rev=334401&r1=334400&r2=334401&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestTextProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestTextProto.cpp Mon Jun 11 05:53:25 2018
@@ -171,19 +171,50 @@ TEST_F(FormatTestTextProto, SupportsAngl
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"
" field_c: 1\n"
" field_d: 12.5\n"
@@ -242,7 +273,10 @@ TEST_F(FormatTestTextProto, SupportsAngl
" 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, BreaksAfterB
">");
}
+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
More information about the cfe-commits
mailing list