[clang-format] Discourage breaks in submessage entries, hard rule

Summary:
Currently clang-format allows this for text protos:
```
submessage:
    { key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' }
```
when it is under the column limit and when putting it all on one line exceeds the column limit.

This is not a very intuitive formatting, so I'd prefer having
```
submessage: {
  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
}
```
instead, even if it takes one line more.

This patch prevents clang-format from inserting a break between `: {` and similar cases.

Reviewers: djasper, sammccall

Reviewed By: sammccall

Subscribers: cfe-commits

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

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

diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 5dce9d248a9605caa452810b00da4f042bdd6d5e..ea957690edbe0db247421435fc2f284d490e6383 100644 (file)
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -3101,10 +3101,39 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
       !Right.isOneOf(TT_CtorInitializerColon, TT_InlineASMColon))
     return false;
   if (Left.is(tok::colon) && Left.isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
-    if ((Style.Language == FormatStyle::LK_Proto ||
-         Style.Language == FormatStyle::LK_TextProto) &&
-        !Style.AlwaysBreakBeforeMultilineStrings && Right.isStringLiteral())
-      return false;
+    if (Style.Language == FormatStyle::LK_Proto ||
+        Style.Language == FormatStyle::LK_TextProto) {
+      if (!Style.AlwaysBreakBeforeMultilineStrings && Right.isStringLiteral())
+        return false;
+      // Prevent cases like:
+      //
+      // submessage:
+      //     { key: valueeeeeeeeeeee }
+      //
+      // when the snippet does not fit into one line.
+      // Prefer:
+      //
+      // submessage: {
+      //   key: valueeeeeeeeeeee
+      // }
+      //
+      // instead, even if it is longer by one line.
+      //
+      // Note that this allows allows the "{" to go over the column limit
+      // when the column limit is just between ":" and "{", but that does
+      // not happen too often and alternative formattings in this case are
+      // not much better.
+      //
+      // The code covers the cases:
+      //
+      // submessage: { ... }
+      // submessage: < ... >
+      // repeated: [ ... ]
+      if (((Right.is(tok::l_brace) || Right.is(tok::less)) &&
+           Right.is(TT_DictLiteral)) ||
+          Right.is(TT_ArrayInitializerLSquare))
+        return false;
+    }
     return true;
   }
   if (Right.is(tok::r_square) && Right.MatchingParen &&

diff --git a/unittests/Format/FormatTestProto.cpp b/unittests/Format/FormatTestProto.cpp
index a15ba62ffceb1bd1c766d2533c487c4220443e13..d25aeb846a97c25bf625b22580e0815e53d5f2cb 100644 (file)
--- a/unittests/Format/FormatTestProto.cpp
+++ b/unittests/Format/FormatTestProto.cpp
@@ -624,5 +624,34 @@ TEST_F(FormatTestProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
                "}");
 }
 
+TEST_F(FormatTestProto, PreventBreaksBetweenKeyAndSubmessages) {
+  verifyFormat("option (MyProto.options) = {\n"
+               "  submessage: {\n"
+               "    key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  submessage {\n"
+               "    key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "  }\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  submessage: <\n"
+               "    key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "  >\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  submessage <\n"
+               "    key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "  >\n"
+               "}");
+  verifyFormat("option (MyProto.options) = {\n"
+               "  repeatedd: [\n"
+               "    'eyaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "  ]\n"
+               "}");
+}
+
+
 } // end namespace tooling
 } // end namespace clang

diff --git a/unittests/Format/FormatTestTextProto.cpp b/unittests/Format/FormatTestTextProto.cpp
index d62975feb44aaa9534507ea4590a66b03b767851..8ef4bf9a40a38fcb0f66353d1d9d7e95ae3a0f24 100644 (file)
--- a/unittests/Format/FormatTestTextProto.cpp
+++ b/unittests/Format/FormatTestTextProto.cpp
@@ -485,8 +485,15 @@ TEST_F(FormatTestTextProto, FormatsRepeatedListInitializers) {
   verifyFormat("keys: []");
   verifyFormat("keys: [ 1 ]");
   verifyFormat("keys: [ 'ala', 'bala' ]");
-  verifyFormat("keys:\n"
-               "    [ 'ala', 'bala', 'porto', 'kala', 'too', 'long', 'ng' ]");
+  verifyFormat("keys: [\n"
+               "  'ala',\n"
+               "  'bala',\n"
+               "  'porto',\n"
+               "  'kala',\n"
+               "  'too',\n"
+               "  'long',\n"
+               "  'ng'\n"
+               "]");
   verifyFormat("key: item\n"
                "keys: [\n"
                "  'ala',\n"
@@ -670,5 +677,28 @@ TEST_F(FormatTestTextProto, BreaksEntriesOfSubmessagesContainingSubmessages) {
                "}");
 }
 
+TEST_F(FormatTestTextProto, PreventBreaksBetweenKeyAndSubmessages) {
+  verifyFormat("submessage: {\n"
+               "  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("submessage {\n"
+               "  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "}");
+  verifyFormat("submessage: <\n"
+               "  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               ">");
+  verifyFormat("submessage <\n"
+               "  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               ">");
+  verifyFormat("repeatedd: [\n"
+               "  'eyaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "]");
+  // "{" is going over the column limit.
+  verifyFormat(
+      "submessageeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee: {\n"
+      "  key: 'aaaaa'\n"
+      "}");
+}
+
 } // end namespace tooling
 } // end namespace clang