Index: lib/Format/TokenAnnotator.cpp =================================================================== --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -2924,6 +2924,74 @@ 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; } Index: unittests/Format/FormatTestProto.cpp =================================================================== --- unittests/Format/FormatTestProto.cpp +++ unittests/Format/FormatTestProto.cpp @@ -493,5 +493,136 @@ "};"); } +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 Index: unittests/Format/FormatTestRawStrings.cpp =================================================================== --- unittests/Format/FormatTestRawStrings.cpp +++ unittests/Format/FormatTestRawStrings.cpp @@ -199,12 +199,6 @@ 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 @@ 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( Index: unittests/Format/FormatTestTextProto.cpp =================================================================== --- unittests/Format/FormatTestTextProto.cpp +++ unittests/Format/FormatTestTextProto.cpp @@ -171,19 +171,50 @@ 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 @@ " 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, 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