[clang-format] Support repeated field lists in protos

[krasimir]

Summary

This patch adds support for list initialization of proto repeated fields:

keys: [1, 2, 3]

[djasper]

Looks good.

lib/Format/TokenAnnotator.cpp

2355 ↗

(On Diff #134254)

This is almost a duplicate of the one above. Can we pull out a function or lambda?

[krasimir]

• Pull out check in lambda
• Address review comments

cfe/trunk/lib/Format/TokenAnnotator.cpp

if (Left->isCppStructuredBinding(Style)) {
         // or text proto extensions (in options):
         //
         // option (Aaa.options) = {
         //   [type.type/type] {
         //     key: value
         //   }
         // }
         //
-        // In the first case we want to spread the contents inside the square
-        // braces; in the second we want to keep them inline.
+        // or repeated fields (in options):
+        //
+        // option (Aaa.options) = {
+        //   keys: [ 1, 2, 3 ]
+        // }
+        //
+        // In the first and the third case we want to spread the contents inside
+        // the square braces; in the second we want to keep them inline.
         Left->Type = TT_ArrayInitializerLSquare;
         if (!Left->endsSequence(tok::l_square, tok::numeric_constant,
-                                tok::equal)) {
+                                tok::equal) &&
+            !Left->endsSequence(tok::l_square, tok::colon, TT_SelectorName)) {
           Left->Type = TT_ProtoExtensionLSquare;
           BindingIncrease = 10;
         }
       } else if (!CppArrayTemplates && Parent &&
                  Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser, tok::at,
                                  tok::comma, tok::l_paren, tok::l_square,
                                  tok::question, tok::colon, tok::kw_return,
                                  // Should only be relevant to JavaScript:
return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
            (!Right.isOneOf(TT_PointerOrReference, TT_ArraySubscriptLSquare,
                            tok::l_paren) &&
             (Style.PointerAlignment != FormatStyle::PAS_Right &&
              !Line.IsMultiVariableDeclStmt) &&
             Left.Previous &&
             !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon));
   if (Right.is(tok::star) && Left.is(tok::l_paren))
     return false;
+  const auto SpaceRequiredForArrayInitializerLSquare =
+      [](const FormatToken &LSquareTok, const FormatStyle &Style) {
+        return Style.SpacesInContainerLiterals ||
+               ((Style.Language == FormatStyle::LK_Proto ||
+                 Style.Language == FormatStyle::LK_TextProto) &&
+                !Style.Cpp11BracedListStyle &&
+                LSquareTok.endsSequence(tok::l_square, tok::colon,
+                                        TT_SelectorName));
+      };
   if (Left.is(tok::l_square))
-    return (Left.is(TT_ArrayInitializerLSquare) &&
-            Style.SpacesInContainerLiterals && Right.isNot(tok::r_square)) ||
+    return (Left.is(TT_ArrayInitializerLSquare) && Right.isNot(tok::r_square) &&
+            SpaceRequiredForArrayInitializerLSquare(Left, Style)) ||
            (Left.isOneOf(TT_ArraySubscriptLSquare,
                          TT_StructuredBindingLSquare) &&
             Style.SpacesInSquareBrackets && Right.isNot(tok::r_square));
   if (Right.is(tok::r_square))
     return Right.MatchingParen &&
-           ((Style.SpacesInContainerLiterals &&
-             Right.MatchingParen->is(TT_ArrayInitializerLSquare)) ||
+           ((Right.MatchingParen->is(TT_ArrayInitializerLSquare) &&
+             SpaceRequiredForArrayInitializerLSquare(*Right.MatchingParen,
+                                                     Style)) ||
             (Style.SpacesInSquareBrackets &&
              Right.MatchingParen->isOneOf(TT_ArraySubscriptLSquare,
                                           TT_StructuredBindingLSquare)));
   if (Right.is(tok::l_square) &&
       !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
                      TT_DesignatedInitializerLSquare,
                      TT_StructuredBindingLSquare) &&
       !Left.isOneOf(tok::numeric_constant, TT_DictLiteral))

cfe/trunk/unittests/Format/FormatTestProto.cpp

 }
 
 TEST_F(FormatTestProto, NoSpaceAfterPercent) {
   verifyFormat("option (MyProto.options) = {\n"
                "  key: %lld\n"
                "};");
 }
 
+TEST_F(FormatTestProto, FormatsRepeatedListInitializersInOptions) {
+  verifyFormat("option (MyProto.options) = {\n"
+               "  key: item\n"
+               "  keys: [\n"
+               "    'ala',\n"
+               "    'bala',\n"
+               "    'porto',\n"
+               "    'kala',\n"
+               "    'too',\n"
+               "    'long',\n"
+               "    'long',\n"
+               "    'long'\n"
+               "  ]\n"
+               "  key: [ item ]\n"
+               "  msg {\n"
+               "    key: item\n"
+               "    keys: [\n"
+               "      'ala',\n"
+               "      'bala',\n"
+               "      'porto',\n"
+               "      'kala',\n"
+               "      'too',\n"
+               "      'long',\n"
+               "      'long'\n"
+               "    ]\n"
+               "  }\n"
+               "  key: value\n"
+               "};");
+}
+
 } // end namespace tooling
 } // end namespace clang

cfe/trunk/unittests/Format/FormatTestTextProto.cpp

static std::string format(llvm::StringRef Code, unsigned Offset,
     std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
     tooling::Replacements Replaces = reformat(Style, Code, Ranges);
     auto Result = applyAllReplacements(Code, Replaces);
     EXPECT_TRUE(static_cast<bool>(Result));
     DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
     return *Result;
   }
 
-  static std::string format(llvm::StringRef Code) {
-    FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
-    Style.ColumnLimit = 60; // To make writing tests easier.
+  static std::string format(llvm::StringRef Code, const FormatStyle &Style) {
     return format(Code, 0, Code.size(), Style);
   }
 
+  static void verifyFormat(llvm::StringRef Code, const FormatStyle &Style) {
+    EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+
   static void verifyFormat(llvm::StringRef Code) {
-    EXPECT_EQ(Code.str(), format(test::messUp(Code)));
+    FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+    Style.ColumnLimit = 60; // To make writing tests easier.
+    verifyFormat(Code, Style);
   }
 };
 
 TEST_F(FormatTestTextProto, KeepsTopLevelEntriesFittingALine) {
   verifyFormat("field_a: OK field_b: OK field_c: OK field_d: OK field_e: OK");
 }
 
 TEST_F(FormatTestTextProto, SupportsMessageFields) {
verifyFormat("[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/\n"
                "    keyyyyyyyyyyyyyy: valuuuuuuuuuuuuuuuuuuuuuuuuue\n"
                "  }\n"
                "}");
 }
 
 TEST_F(FormatTestTextProto, NoSpaceAfterPercent) {
   verifyFormat("key: %d");
 }
+
+TEST_F(FormatTestTextProto, FormatsRepeatedListInitializers) {
+  verifyFormat("keys: []");
+  verifyFormat("keys: [ 1 ]");
+  verifyFormat("keys: [ 'ala', 'bala' ]");
+  verifyFormat("keys:\n"
+               "    [ 'ala', 'bala', 'porto', 'kala', 'too', 'long', 'ng' ]");
+  verifyFormat("key: item\n"
+               "keys: [\n"
+               "  'ala',\n"
+               "  'bala',\n"
+               "  'porto',\n"
+               "  'kala',\n"
+               "  'too',\n"
+               "  'long',\n"
+               "  'long',\n"
+               "  'long'\n"
+               "]\n"
+               "key: item\n"
+               "msg {\n"
+               "  key: item\n"
+               "  keys: [\n"
+               "    'ala',\n"
+               "    'bala',\n"
+               "    'porto',\n"
+               "    'kala',\n"
+               "    'too',\n"
+               "    'long',\n"
+               "    'long'\n"
+               "  ]\n"
+               "}\n"
+               "key: value"
+               );
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+  Style.ColumnLimit = 60; // To make writing tests easier.
+  Style.Cpp11BracedListStyle = true;
+  verifyFormat("keys: [1]", Style);
+}
 } // end namespace tooling
 } // end namespace clang