r199580 - clang-format: Better support and testing for protocol buffers.

Daniel Jasper djasper at google.com
Sun Jan 19 01:04:08 PST 2014


Author: djasper
Date: Sun Jan 19 03:04:08 2014
New Revision: 199580

URL: http://llvm.org/viewvc/llvm-project?rev=199580&view=rev
Log:
clang-format: Better support and testing for protocol buffers.

With this patch, there is dedicated testing for protocol buffers
(https://developers.google.com/protocol-buffers/).

Also some minor tweaks formatting tweaks.

Added:
    cfe/trunk/unittests/Format/FormatTestProto.cpp
Modified:
    cfe/trunk/include/clang/Format/Format.h
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/lib/Format/TokenAnnotator.cpp
    cfe/trunk/unittests/Format/CMakeLists.txt
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/include/clang/Format/Format.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=199580&r1=199579&r2=199580&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Sun Jan 19 03:04:08 2014
@@ -39,7 +39,10 @@ struct FormatStyle {
     /// Should be used for C, C++, ObjectiveC, ObjectiveC++.
     LK_Cpp,
     /// Should be used for JavaScript.
-    LK_JavaScript
+    LK_JavaScript,
+    /// Should be used for Protocol Buffers
+    /// (https://developers.google.com/protocol-buffers/).
+    LK_Proto
   };
 
   /// \brief Language, this format style is targeted at.
@@ -361,6 +364,10 @@ FormatStyle getGoogleStyle();
 /// http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml.
 FormatStyle getGoogleJSStyle();
 
+/// \brief Returns a format style complying with Google's Protocol Buffer style:
+/// https://developers.google.com/protocol-buffers/docs/style.
+FormatStyle getGoogleProtoStyle();
+
 /// \brief Returns a format style complying with Chromium's style guide:
 /// http://www.chromium.org/developers/coding-style.
 FormatStyle getChromiumStyle();

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=199580&r1=199579&r2=199580&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Sun Jan 19 03:04:08 2014
@@ -39,6 +39,7 @@ template <> struct ScalarEnumerationTrai
   static void enumeration(IO &IO, FormatStyle::LanguageKind &Value) {
     IO.enumCase(Value, "Cpp", FormatStyle::LK_Cpp);
     IO.enumCase(Value, "JavaScript", FormatStyle::LK_JavaScript);
+    IO.enumCase(Value, "Proto", FormatStyle::LK_Proto);
   }
 };
 
@@ -323,6 +324,13 @@ FormatStyle getGoogleJSStyle() {
   return GoogleJSStyle;
 }
 
+FormatStyle getGoogleProtoStyle() {
+  FormatStyle GoogleProtoStyle = getGoogleStyle();
+  GoogleProtoStyle.Language = FormatStyle::LK_Proto;
+  GoogleProtoStyle.AllowShortFunctionsOnASingleLine = false;
+  return GoogleProtoStyle;
+}
+
 FormatStyle getChromiumStyle() {
   FormatStyle ChromiumStyle = getGoogleStyle();
   ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
@@ -379,8 +387,16 @@ bool getPredefinedStyle(StringRef Name,
   } else if (Name.equals_lower("mozilla")) {
     *Style = getMozillaStyle();
   } else if (Name.equals_lower("google")) {
-    *Style = Language == FormatStyle::LK_JavaScript ? getGoogleJSStyle()
-                                                    : getGoogleStyle();
+    switch (Language) {
+    case FormatStyle::LK_JavaScript:
+      *Style = getGoogleJSStyle();
+      break;
+    case FormatStyle::LK_Proto:
+      *Style = getGoogleProtoStyle();
+      break;
+    default:
+      *Style = getGoogleStyle();
+    }
   } else if (Name.equals_lower("webkit")) {
     *Style = getWebKitStyle();
   } else if (Name.equals_lower("gnu")) {
@@ -1350,6 +1366,8 @@ static StringRef getLanguageName(FormatS
     return "C++";
   case FormatStyle::LK_JavaScript:
     return "JavaScript";
+  case FormatStyle::LK_Proto:
+    return "Proto";
   default:
     return "Unknown";
   }
@@ -1698,6 +1716,9 @@ const char *StyleOptionHelpDescription =
 static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
   if (FileName.endswith_lower(".js")) {
     return FormatStyle::LK_JavaScript;
+  } else if (FileName.endswith_lower(".proto") ||
+             FileName.endswith_lower(".protodevel")) {
+    return FormatStyle::LK_Proto;
   }
   return FormatStyle::LK_Cpp;
 }

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=199580&r1=199579&r2=199580&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sun Jan 19 03:04:08 2014
@@ -1164,9 +1164,12 @@ unsigned TokenAnnotator::splitPenalty(co
     return 0;
   if (Left.is(tok::comma))
     return 1;
-  if (Right.is(tok::l_square) && Right.Type != TT_ObjCMethodExpr)
-    return 250;
-
+  if (Right.is(tok::l_square)) {
+    if (Style.Language == FormatStyle::LK_Proto)
+      return 1;
+    if (Right.Type != TT_ObjCMethodExpr)
+      return 250;
+  }
   if (Right.Type == TT_StartOfName || Right.is(tok::kw_operator)) {
     if (Line.First->is(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
       return 3;
@@ -1250,6 +1253,10 @@ unsigned TokenAnnotator::splitPenalty(co
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
                                           const FormatToken &Left,
                                           const FormatToken &Right) {
+  if (Style.Language == FormatStyle::LK_Proto) {
+    if (Right.is(tok::l_paren) && Left.TokenText == "returns")
+      return true;
+  }
   if (Right.is(tok::hashhash))
     return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
@@ -1452,6 +1459,10 @@ bool TokenAnnotator::mustBreakBefore(con
     // deliberate choice and might have aligned the contents of the string
     // literal accordingly. Thus, we try keep existing line breaks.
     return Right.NewlinesBefore > 0;
+  } else if (Right.Previous->is(tok::l_brace) &&
+             Style.Language == FormatStyle::LK_Proto) {
+    // Don't enums onto single lines in protocol buffers.
+    return true;
   }
   return false;
 }

Modified: cfe/trunk/unittests/Format/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/CMakeLists.txt?rev=199580&r1=199579&r2=199580&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Format/CMakeLists.txt Sun Jan 19 03:04:08 2014
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_unittest(FormatTests
   FormatTest.cpp
   FormatTestJS.cpp
+  FormatTestProto.cpp
   )
 
 target_link_libraries(FormatTests

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=199580&r1=199579&r2=199580&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Sun Jan 19 03:04:08 2014
@@ -7854,15 +7854,6 @@ TEST_F(FormatTest, FormatsWithWebKitStyl
                    Style));
 }
 
-TEST_F(FormatTest, FormatsProtocolBufferDefinitions) {
-  // It seems that clang-format can format protocol buffer definitions
-  // (see https://code.google.com/p/protobuf/).
-  verifyFormat("message SomeMessage {\n"
-               "  required int32 field1 = 1;\n"
-               "  optional string field2 = 2 [default = \"2\"]\n"
-               "}");
-}
-
 TEST_F(FormatTest, FormatsLambdas) {
   verifyFormat("int c = [b]() mutable {\n"
                "  return [&b] { return b++; }();\n"

Added: cfe/trunk/unittests/Format/FormatTestProto.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=199580&view=auto
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestProto.cpp (added)
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp Sun Jan 19 03:04:08 2014
@@ -0,0 +1,72 @@
+//===- unittest/Format/FormatTestProto.cpp --------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#define DEBUG_TYPE "format-test"
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace format {
+
+class FormatTestProto : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+                            unsigned Length, const FormatStyle &Style) {
+    DEBUG(llvm::errs() << "---\n");
+    DEBUG(llvm::errs() << Code << "\n\n");
+    std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
+    tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+    std::string Result = applyAllReplacements(Code, Replaces);
+    EXPECT_NE("", Result);
+    DEBUG(llvm::errs() << "\n" << Result << "\n\n");
+    return Result;
+  }
+
+  static std::string format(llvm::StringRef Code) {
+    return format(Code, 0, Code.size(), getGoogleProtoStyle());
+  }
+
+  static void verifyFormat(llvm::StringRef Code) {
+    EXPECT_EQ(Code.str(), format(test::messUp(Code)));
+  }
+};
+
+TEST_F(FormatTestProto, FormatsMessages) {
+  verifyFormat("message SomeMessage {\n"
+               "  required int32 field1 = 1;\n"
+               "}");
+  verifyFormat("message SomeMessage {\n"
+               "  required int32 field1 = 1;\n"
+               "  optional string field2 = 2 [default = \"2\"]\n"
+               "}");
+}
+
+TEST_F(FormatTestProto, FormatsEnums) {
+  verifyFormat("enum Type {\n"
+               "  UNKNOWN = 0;\n"
+               "  TYPE_A = 1;\n"
+               "  TYPE_B = 2;\n"
+               "};");
+}
+
+TEST_F(FormatTestProto, UnderstandsReturns) {
+  verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
+}
+
+TEST_F(FormatTestProto, MessageFieldAttributes) {
+  verifyFormat("optional string test = 1 [default = \"test\"];");
+  verifyFormat("optional LongMessageType long_proto_field = 1\n"
+               "    [default = REALLY_REALLY_LONG_CONSTANT_VALUE];");
+}
+
+} // end namespace tooling
+} // end namespace clang





More information about the cfe-commits mailing list