From e265a8885761ebae6bbf501fc31d5b85141f8ff1 Mon Sep 17 00:00:00 2001 From: Daniel Jasper <djasper@google.com> Date: Sun, 19 Jan 2014 09:04:08 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@199580 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Format/Format.h | 9 +++- lib/Format/Format.cpp | 25 +++++++++- lib/Format/TokenAnnotator.cpp | 17 +++++-- unittests/Format/CMakeLists.txt | 1 + unittests/Format/FormatTest.cpp | 9 ---- unittests/Format/FormatTestProto.cpp | 72 ++++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 15 deletions(-) create mode 100644 unittests/Format/FormatTestProto.cpp diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h index 71fb6339db6..7a09b1b088c 100644 --- a/include/clang/Format/Format.h +++ b/include/clang/Format/Format.h @@ -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(); diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index a69570a1571..79e243b9074 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -39,6 +39,7 @@ template <> struct ScalarEnumerationTraits<FormatStyle::LanguageKind> { 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, FormatStyle::LanguageKind Language, } 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(FormatStyle::LanguageKind Language) { 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; } diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 45d60726dcb..b1f414f8dd7 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/lib/Format/TokenAnnotator.cpp @@ -1164,9 +1164,12 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line, 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(const AnnotatedLine &Line, 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(const AnnotatedLine &Line, // 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; } diff --git a/unittests/Format/CMakeLists.txt b/unittests/Format/CMakeLists.txt index 89c7055d5a3..14fc22d05c0 100644 --- a/unittests/Format/CMakeLists.txt +++ b/unittests/Format/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS add_clang_unittest(FormatTests FormatTest.cpp FormatTestJS.cpp + FormatTestProto.cpp ) target_link_libraries(FormatTests diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index cff28d456ce..e43d17fa05c 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -7854,15 +7854,6 @@ TEST_F(FormatTest, FormatsWithWebKitStyle) { 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" diff --git a/unittests/Format/FormatTestProto.cpp b/unittests/Format/FormatTestProto.cpp new file mode 100644 index 00000000000..d102c6567c1 --- /dev/null +++ b/unittests/Format/FormatTestProto.cpp @@ -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 -- GitLab