Skip to content

Commit 8b98f55

Browse files
krasimirggkrasimirgg
committedJun 11, 2018
[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 llvm-svn: 334401
·
llvmorg-21-initllvmorg-7.0.0-rc1
1 parent 5f8ede4 commit 8b98f55

File tree

4 files changed

+362
-16
lines changed

4 files changed

+362
-16
lines changed
 

‎clang/lib/Format/TokenAnnotator.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2924,6 +2924,74 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
29242924
if (Right.is(TT_ProtoExtensionLSquare))
29252925
return true;
29262926

2927+
// In text proto instances if a submessage contains at least 2 entries and at
2928+
// least one of them is a submessage, like A { ... B { ... } ... },
2929+
// put all of the entries of A on separate lines by forcing the selector of
2930+
// the submessage B to be put on a newline.
2931+
//
2932+
// Example: these can stay on one line:
2933+
// a { scalar_1: 1 scalar_2: 2 }
2934+
// a { b { key: value } }
2935+
//
2936+
// and these entries need to be on a new line even if putting them all in one
2937+
// line is under the column limit:
2938+
// a {
2939+
// scalar: 1
2940+
// b { key: value }
2941+
// }
2942+
//
2943+
// We enforce this by breaking before a submessage field that has previous
2944+
// siblings, *and* breaking before a field that follows a submessage field.
2945+
//
2946+
// Be careful to exclude the case [proto.ext] { ... } since the `]` is
2947+
// the TT_SelectorName there, but we don't want to break inside the brackets.
2948+
// We ensure elsewhere that extensions are always on their own line.
2949+
if ((Style.Language == FormatStyle::LK_Proto ||
2950+
Style.Language == FormatStyle::LK_TextProto) &&
2951+
Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
2952+
// Look for the scope opener after selector in cases like:
2953+
// selector { ...
2954+
// selector: { ...
2955+
FormatToken *LBrace =
2956+
Right.Next->is(tok::colon) ? Right.Next->Next : Right.Next;
2957+
if (LBrace &&
2958+
// The scope opener is one of {, [, <:
2959+
// selector { ... }
2960+
// selector [ ... ]
2961+
// selector < ... >
2962+
//
2963+
// In case of selector { ... }, the l_brace is TT_DictLiteral.
2964+
// In case of an empty selector {}, the l_brace is not TT_DictLiteral,
2965+
// so we check for immediately following r_brace.
2966+
((LBrace->is(tok::l_brace) &&
2967+
(LBrace->is(TT_DictLiteral) ||
2968+
(LBrace->Next && LBrace->Next->is(tok::r_brace)))) ||
2969+
LBrace->is(TT_ArrayInitializerLSquare) || LBrace->is(tok::less))) {
2970+
// If Left.ParameterCount is 0, then this submessage entry is not the
2971+
// first in its parent submessage, and we want to break before this entry.
2972+
// If Left.ParameterCount is greater than 0, then its parent submessage
2973+
// might contain 1 or more entries and we want to break before this entry
2974+
// if it contains at least 2 entries. We deal with this case later by
2975+
// detecting and breaking before the next entry in the parent submessage.
2976+
if (Left.ParameterCount == 0)
2977+
return true;
2978+
// However, if this submessage is the first entry in its parent
2979+
// submessage, Left.ParameterCount might be 1 in some cases.
2980+
// We deal with this case later by detecting an entry
2981+
// following a closing paren of this submessage.
2982+
}
2983+
2984+
// If this is an entry immediately following a submessage, it will be
2985+
// preceded by a closing paren of that submessage, like in:
2986+
// left---. .---right
2987+
// v v
2988+
// sub: { ... } key: value
2989+
// If there was a comment between `}` an `key` above, then `key` would be
2990+
// put on a new line anyways.
2991+
if (Left.isOneOf(tok::r_brace, tok::greater, tok::r_square))
2992+
return true;
2993+
}
2994+
29272995
return false;
29282996
}
29292997

‎clang/unittests/Format/FormatTestProto.cpp

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,5 +493,136 @@ TEST_F(FormatTestProto, AcceptsOperatorAsKeyInOptions) {
493493
"};");
494494
}
495495

496+
TEST_F(FormatTestProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
497+
FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
498+
Style.ColumnLimit = 60;
499+
// The column limit allows for the keys submessage to be put on 1 line, but we
500+
// break it since it contains a submessage an another entry.
501+
verifyFormat("option (MyProto.options) = {\n"
502+
" key: valueeeeeeee\n"
503+
" keys: {\n"
504+
" item: 'aaaaaaaaaaaaaaaa'\n"
505+
" sub <>\n"
506+
" }\n"
507+
"}");
508+
verifyFormat("option (MyProto.options) = {\n"
509+
" key: valueeeeeeee\n"
510+
" keys: {\n"
511+
" item: 'aaaaaaaaaaaaaaaa'\n"
512+
" sub {}\n"
513+
" }\n"
514+
"}");
515+
verifyFormat("option (MyProto.options) = {\n"
516+
" key: valueeeeeeee\n"
517+
" keys: {\n"
518+
" sub {}\n"
519+
" sub: <>\n"
520+
" sub: []\n"
521+
" }\n"
522+
"}");
523+
verifyFormat("option (MyProto.options) = {\n"
524+
" key: valueeeeeeee\n"
525+
" keys: {\n"
526+
" item: 'aaaaaaaaaaa'\n"
527+
" sub { msg: 1 }\n"
528+
" }\n"
529+
"}");
530+
verifyFormat("option (MyProto.options) = {\n"
531+
" key: valueeeeeeee\n"
532+
" keys: {\n"
533+
" item: 'aaaaaaaaaaa'\n"
534+
" sub: { msg: 1 }\n"
535+
" }\n"
536+
"}");
537+
verifyFormat("option (MyProto.options) = {\n"
538+
" key: valueeeeeeee\n"
539+
" keys: {\n"
540+
" item: 'aaaaaaaaaaa'\n"
541+
" sub < msg: 1 >\n"
542+
" }\n"
543+
"}");
544+
verifyFormat("option (MyProto.options) = {\n"
545+
" key: valueeeeeeee\n"
546+
" keys: {\n"
547+
" item: 'aaaaaaaaaaa'\n"
548+
" sub: [ msg: 1 ]\n"
549+
" }\n"
550+
"}");
551+
verifyFormat("option (MyProto.options) = {\n"
552+
" key: valueeeeeeee\n"
553+
" keys: <\n"
554+
" item: 'aaaaaaaaaaa'\n"
555+
" sub: [ 1, 2 ]\n"
556+
" >\n"
557+
"}");
558+
verifyFormat("option (MyProto.options) = {\n"
559+
" key: valueeeeeeee\n"
560+
" keys: {\n"
561+
" sub {}\n"
562+
" item: 'aaaaaaaaaaaaaaaa'\n"
563+
" }\n"
564+
"}");
565+
verifyFormat("option (MyProto.options) = {\n"
566+
" key: valueeeeeeee\n"
567+
" keys: {\n"
568+
" sub: []\n"
569+
" item: 'aaaaaaaaaaaaaaaa'\n"
570+
" }\n"
571+
"}");
572+
verifyFormat("option (MyProto.options) = {\n"
573+
" key: valueeeeeeee\n"
574+
" keys: {\n"
575+
" sub <>\n"
576+
" item: 'aaaaaaaaaaaaaaaa'\n"
577+
" }\n"
578+
"}");
579+
verifyFormat("option (MyProto.options) = {\n"
580+
" key: valueeeeeeee\n"
581+
" keys: {\n"
582+
" sub { key: value }\n"
583+
" item: 'aaaaaaaaaaaaaaaa'\n"
584+
" }\n"
585+
"}");
586+
verifyFormat("option (MyProto.options) = {\n"
587+
" key: valueeeeeeee\n"
588+
" keys: {\n"
589+
" sub: [ 1, 2 ]\n"
590+
" item: 'aaaaaaaaaaaaaaaa'\n"
591+
" }\n"
592+
"}");
593+
verifyFormat("option (MyProto.options) = {\n"
594+
" key: valueeeeeeee\n"
595+
" keys: {\n"
596+
" sub < sub_2: {} >\n"
597+
" item: 'aaaaaaaaaaaaaaaa'\n"
598+
" }\n"
599+
"}");
600+
verifyFormat("option (MyProto.options) = {\n"
601+
" key: valueeeeeeee\n"
602+
" keys: {\n"
603+
" item: data\n"
604+
" sub: [ 1, 2 ]\n"
605+
" item: 'aaaaaaaaaaaaaaaa'\n"
606+
" }\n"
607+
"}");
608+
verifyFormat("option (MyProto.options) = {\n"
609+
" key: valueeeeeeee\n"
610+
" keys: {\n"
611+
" item: data\n"
612+
" sub < sub_2: {} >\n"
613+
" item: 'aaaaaaaaaaaaaaaa'\n"
614+
" }\n"
615+
"}");
616+
verifyFormat("option (MyProto.options) = {\n"
617+
" sub: {\n"
618+
" key: valueeeeeeee\n"
619+
" keys: {\n"
620+
" sub: [ 1, 2 ]\n"
621+
" item: 'aaaaaaaaaaaaaaaa'\n"
622+
" }\n"
623+
" }\n"
624+
"}");
625+
}
626+
496627
} // end namespace tooling
497628
} // end namespace clang

‎clang/unittests/Format/FormatTestRawStrings.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,6 @@ TEST_F(FormatTestRawStrings, ReformatsShortRawStringsOnSingleLine) {
199199
format(
200200
R"test(P p = TP(R"pb(item_1:1 item_2:2)pb");)test",
201201
getRawStringPbStyleWithColumns(40)));
202-
expect_eq(
203-
R"test(P p = TP(R"pb(item_1 < 1 > item_2: { 2 })pb");)test",
204-
format(
205-
R"test(P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
206-
getRawStringPbStyleWithColumns(40)));
207-
208202
// Merge two short lines into one.
209203
expect_eq(R"test(
210204
std::string s = R"pb(
@@ -220,6 +214,18 @@ std::string s = R"pb(
220214
getRawStringPbStyleWithColumns(40)));
221215
}
222216

217+
TEST_F(FormatTestRawStrings, BreaksShortRawStringsWhenNeeded) {
218+
// The raw string contains multiple submessage entries, so break for
219+
// readability.
220+
expect_eq(R"test(
221+
P p = TP(R"pb(item_1 < 1 >
222+
item_2: { 2 })pb");)test",
223+
format(
224+
R"test(
225+
P p = TP(R"pb(item_1<1> item_2:{2})pb");)test",
226+
getRawStringPbStyleWithColumns(40)));
227+
}
228+
223229
TEST_F(FormatTestRawStrings, BreaksRawStringsExceedingColumnLimit) {
224230
expect_eq(R"test(
225231
P p = TPPPPPPPPPPPPPPP(

‎clang/unittests/Format/FormatTestTextProto.cpp

Lines changed: 151 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,17 +171,48 @@ TEST_F(FormatTestTextProto, SupportsAngleBracketMessageFields) {
171171
verifyFormat("msg_field < field_a < field_b <> > >");
172172
verifyFormat("msg_field: < field_a < field_b: <> > >");
173173
verifyFormat("msg_field < field_a: OK, field_b: \"OK\" >");
174-
verifyFormat("msg_field < field_a: OK field_b: <>, field_c: OK >");
175-
verifyFormat("msg_field < field_a { field_b: 1 }, field_c: < f_d: 2 > >");
176174
verifyFormat("msg_field: < field_a: OK, field_b: \"OK\" >");
177-
verifyFormat("msg_field: < field_a: OK field_b: <>, field_c: OK >");
178-
verifyFormat("msg_field: < field_a { field_b: 1 }, field_c: < fd_d: 2 > >");
179-
verifyFormat("field_a: \"OK\", msg_field: < field_b: 123 >, field_c: {}");
180-
verifyFormat("field_a < field_b: 1 >, msg_fid: < fiel_b: 123 >, field_c <>");
181-
verifyFormat("field_a < field_b: 1 > msg_fied: < field_b: 123 > field_c <>");
182-
verifyFormat("field < field < field: <> >, field <> > field: < field: 1 >");
183-
184175
// Multiple lines tests
176+
verifyFormat("msg_field <\n"
177+
" field_a: OK\n"
178+
" field_b: <>,\n"
179+
" field_c: OK\n"
180+
">");
181+
182+
verifyFormat("msg_field <\n"
183+
" field_a { field_b: 1 },\n"
184+
" field_c: < f_d: 2 >\n"
185+
">");
186+
187+
verifyFormat("msg_field: <\n"
188+
" field_a: OK\n"
189+
" field_b: <>,\n"
190+
" field_c: OK\n"
191+
">");
192+
193+
verifyFormat("msg_field: <\n"
194+
" field_a { field_b: 1 },\n"
195+
" field_c: < fd_d: 2 >\n"
196+
">");
197+
198+
verifyFormat("field_a: \"OK\",\n"
199+
"msg_field: < field_b: 123 >,\n"
200+
"field_c: {}");
201+
202+
verifyFormat("field_a < field_b: 1 >,\n"
203+
"msg_fid: < fiel_b: 123 >,\n"
204+
"field_c <>");
205+
206+
verifyFormat("field_a < field_b: 1 >\n"
207+
"msg_fied: < field_b: 123 >\n"
208+
"field_c <>");
209+
210+
verifyFormat("field <\n"
211+
" field < field: <> >,\n"
212+
" field <>\n"
213+
">\n"
214+
"field: < field: 1 >");
215+
185216
verifyFormat("msg_field <\n"
186217
" field_a: OK\n"
187218
" field_b: \"OK\"\n"
@@ -242,7 +273,10 @@ TEST_F(FormatTestTextProto, SupportsAngleBracketMessageFields) {
242273
" field_d: ok\n"
243274
"}");
244275

245-
verifyFormat("field_a: < f1: 1, f2: <> >\n"
276+
verifyFormat("field_a: <\n"
277+
" f1: 1,\n"
278+
" f2: <>\n"
279+
">\n"
246280
"field_b <\n"
247281
" field_b1: <>\n"
248282
" field_b2: ok,\n"
@@ -529,5 +563,112 @@ TEST_F(FormatTestTextProto, BreaksAfterBraceFollowedByClosingBraceOnNextLine) {
529563
">");
530564
}
531565

566+
TEST_F(FormatTestTextProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
567+
FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
568+
Style.ColumnLimit = 60;
569+
// The column limit allows for the keys submessage to be put on 1 line, but we
570+
// break it since it contains a submessage an another entry.
571+
verifyFormat("key: valueeeeeeee\n"
572+
"keys: {\n"
573+
" item: 'aaaaaaaaaaaaaaaa'\n"
574+
" sub <>\n"
575+
"}");
576+
verifyFormat("key: valueeeeeeee\n"
577+
"keys: {\n"
578+
" item: 'aaaaaaaaaaaaaaaa'\n"
579+
" sub {}\n"
580+
"}");
581+
verifyFormat("key: valueeeeeeee\n"
582+
"keys: {\n"
583+
" sub {}\n"
584+
" sub: <>\n"
585+
" sub: []\n"
586+
"}");
587+
verifyFormat("key: valueeeeeeee\n"
588+
"keys: {\n"
589+
" item: 'aaaaaaaaaaa'\n"
590+
" sub { msg: 1 }\n"
591+
"}");
592+
verifyFormat("key: valueeeeeeee\n"
593+
"keys: {\n"
594+
" item: 'aaaaaaaaaaa'\n"
595+
" sub: { msg: 1 }\n"
596+
"}");
597+
verifyFormat("key: valueeeeeeee\n"
598+
"keys: {\n"
599+
" item: 'aaaaaaaaaaa'\n"
600+
" sub < msg: 1 >\n"
601+
"}");
602+
verifyFormat("key: valueeeeeeee\n"
603+
"keys: {\n"
604+
" item: 'aaaaaaaaaaa'\n"
605+
" sub: [ msg: 1 ]\n"
606+
"}");
607+
verifyFormat("key: valueeeeeeee\n"
608+
"keys: <\n"
609+
" item: 'aaaaaaaaaaa'\n"
610+
" sub: [ 1, 2 ]\n"
611+
">");
612+
verifyFormat("key: valueeeeeeee\n"
613+
"keys: {\n"
614+
" sub {}\n"
615+
" item: 'aaaaaaaaaaaaaaaa'\n"
616+
"}");
617+
verifyFormat("key: valueeeeeeee\n"
618+
"keys: {\n"
619+
" sub: []\n"
620+
" item: 'aaaaaaaaaaaaaaaa'\n"
621+
"}");
622+
verifyFormat("key: valueeeeeeee\n"
623+
"keys: {\n"
624+
" sub <>\n"
625+
" item: 'aaaaaaaaaaaaaaaa'\n"
626+
"}");
627+
verifyFormat("key: valueeeeeeee\n"
628+
"keys: {\n"
629+
" sub { key: value }\n"
630+
" item: 'aaaaaaaaaaaaaaaa'\n"
631+
"}");
632+
verifyFormat("key: valueeeeeeee\n"
633+
"keys: {\n"
634+
" sub: [ 1, 2 ]\n"
635+
" item: 'aaaaaaaaaaaaaaaa'\n"
636+
"}");
637+
verifyFormat("key: valueeeeeeee\n"
638+
"keys: {\n"
639+
" sub < sub_2: {} >\n"
640+
" item: 'aaaaaaaaaaaaaaaa'\n"
641+
"}");
642+
verifyFormat("key: valueeeeeeee\n"
643+
"keys: {\n"
644+
" item: data\n"
645+
" sub: [ 1, 2 ]\n"
646+
" item: 'aaaaaaaaaaaaaaaa'\n"
647+
"}");
648+
verifyFormat("key: valueeeeeeee\n"
649+
"keys: {\n"
650+
" item: data\n"
651+
" sub < sub_2: {} >\n"
652+
" item: 'aaaaaaaaaaaaaaaa'\n"
653+
"}");
654+
verifyFormat("sub: {\n"
655+
" key: valueeeeeeee\n"
656+
" keys: {\n"
657+
" sub: [ 1, 2 ]\n"
658+
" item: 'aaaaaaaaaaaaaaaa'\n"
659+
" }\n"
660+
"}");
661+
verifyFormat("sub: {\n"
662+
" key: 1\n"
663+
" sub: {}\n"
664+
"}\n"
665+
"# comment\n");
666+
verifyFormat("sub: {\n"
667+
" key: 1\n"
668+
" # comment\n"
669+
" sub: {}\n"
670+
"}");
671+
}
672+
532673
} // end namespace tooling
533674
} // end namespace clang

0 commit comments

Comments
 (0)
Please sign in to comment.