Skip to content

Commit 7052ce6

Browse files
committed
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. llvm-svn: 199580
1 parent 2e3166a commit 7052ce6

File tree

6 files changed

+118
-15
lines changed

6 files changed

+118
-15
lines changed

clang/include/clang/Format/Format.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ struct FormatStyle {
3939
/// Should be used for C, C++, ObjectiveC, ObjectiveC++.
4040
LK_Cpp,
4141
/// Should be used for JavaScript.
42-
LK_JavaScript
42+
LK_JavaScript,
43+
/// Should be used for Protocol Buffers
44+
/// (https://developers.google.com/protocol-buffers/).
45+
LK_Proto
4346
};
4447

4548
/// \brief Language, this format style is targeted at.
@@ -361,6 +364,10 @@ FormatStyle getGoogleStyle();
361364
/// http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml.
362365
FormatStyle getGoogleJSStyle();
363366

367+
/// \brief Returns a format style complying with Google's Protocol Buffer style:
368+
/// https://developers.google.com/protocol-buffers/docs/style.
369+
FormatStyle getGoogleProtoStyle();
370+
364371
/// \brief Returns a format style complying with Chromium's style guide:
365372
/// http://www.chromium.org/developers/coding-style.
366373
FormatStyle getChromiumStyle();

clang/lib/Format/Format.cpp

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ template <> struct ScalarEnumerationTraits<FormatStyle::LanguageKind> {
3939
static void enumeration(IO &IO, FormatStyle::LanguageKind &Value) {
4040
IO.enumCase(Value, "Cpp", FormatStyle::LK_Cpp);
4141
IO.enumCase(Value, "JavaScript", FormatStyle::LK_JavaScript);
42+
IO.enumCase(Value, "Proto", FormatStyle::LK_Proto);
4243
}
4344
};
4445

@@ -323,6 +324,13 @@ FormatStyle getGoogleJSStyle() {
323324
return GoogleJSStyle;
324325
}
325326

327+
FormatStyle getGoogleProtoStyle() {
328+
FormatStyle GoogleProtoStyle = getGoogleStyle();
329+
GoogleProtoStyle.Language = FormatStyle::LK_Proto;
330+
GoogleProtoStyle.AllowShortFunctionsOnASingleLine = false;
331+
return GoogleProtoStyle;
332+
}
333+
326334
FormatStyle getChromiumStyle() {
327335
FormatStyle ChromiumStyle = getGoogleStyle();
328336
ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
@@ -379,8 +387,16 @@ bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language,
379387
} else if (Name.equals_lower("mozilla")) {
380388
*Style = getMozillaStyle();
381389
} else if (Name.equals_lower("google")) {
382-
*Style = Language == FormatStyle::LK_JavaScript ? getGoogleJSStyle()
383-
: getGoogleStyle();
390+
switch (Language) {
391+
case FormatStyle::LK_JavaScript:
392+
*Style = getGoogleJSStyle();
393+
break;
394+
case FormatStyle::LK_Proto:
395+
*Style = getGoogleProtoStyle();
396+
break;
397+
default:
398+
*Style = getGoogleStyle();
399+
}
384400
} else if (Name.equals_lower("webkit")) {
385401
*Style = getWebKitStyle();
386402
} else if (Name.equals_lower("gnu")) {
@@ -1350,6 +1366,8 @@ static StringRef getLanguageName(FormatStyle::LanguageKind Language) {
13501366
return "C++";
13511367
case FormatStyle::LK_JavaScript:
13521368
return "JavaScript";
1369+
case FormatStyle::LK_Proto:
1370+
return "Proto";
13531371
default:
13541372
return "Unknown";
13551373
}
@@ -1698,6 +1716,9 @@ const char *StyleOptionHelpDescription =
16981716
static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
16991717
if (FileName.endswith_lower(".js")) {
17001718
return FormatStyle::LK_JavaScript;
1719+
} else if (FileName.endswith_lower(".proto") ||
1720+
FileName.endswith_lower(".protodevel")) {
1721+
return FormatStyle::LK_Proto;
17011722
}
17021723
return FormatStyle::LK_Cpp;
17031724
}

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,9 +1164,12 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
11641164
return 0;
11651165
if (Left.is(tok::comma))
11661166
return 1;
1167-
if (Right.is(tok::l_square) && Right.Type != TT_ObjCMethodExpr)
1168-
return 250;
1169-
1167+
if (Right.is(tok::l_square)) {
1168+
if (Style.Language == FormatStyle::LK_Proto)
1169+
return 1;
1170+
if (Right.Type != TT_ObjCMethodExpr)
1171+
return 250;
1172+
}
11701173
if (Right.Type == TT_StartOfName || Right.is(tok::kw_operator)) {
11711174
if (Line.First->is(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
11721175
return 3;
@@ -1250,6 +1253,10 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
12501253
bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
12511254
const FormatToken &Left,
12521255
const FormatToken &Right) {
1256+
if (Style.Language == FormatStyle::LK_Proto) {
1257+
if (Right.is(tok::l_paren) && Left.TokenText == "returns")
1258+
return true;
1259+
}
12531260
if (Right.is(tok::hashhash))
12541261
return Left.is(tok::hash);
12551262
if (Left.isOneOf(tok::hashhash, tok::hash))
@@ -1452,6 +1459,10 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
14521459
// deliberate choice and might have aligned the contents of the string
14531460
// literal accordingly. Thus, we try keep existing line breaks.
14541461
return Right.NewlinesBefore > 0;
1462+
} else if (Right.Previous->is(tok::l_brace) &&
1463+
Style.Language == FormatStyle::LK_Proto) {
1464+
// Don't enums onto single lines in protocol buffers.
1465+
return true;
14551466
}
14561467
return false;
14571468
}

clang/unittests/Format/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
55
add_clang_unittest(FormatTests
66
FormatTest.cpp
77
FormatTestJS.cpp
8+
FormatTestProto.cpp
89
)
910

1011
target_link_libraries(FormatTests

clang/unittests/Format/FormatTest.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7854,15 +7854,6 @@ TEST_F(FormatTest, FormatsWithWebKitStyle) {
78547854
Style));
78557855
}
78567856

7857-
TEST_F(FormatTest, FormatsProtocolBufferDefinitions) {
7858-
// It seems that clang-format can format protocol buffer definitions
7859-
// (see https://code.google.com/p/protobuf/).
7860-
verifyFormat("message SomeMessage {\n"
7861-
" required int32 field1 = 1;\n"
7862-
" optional string field2 = 2 [default = \"2\"]\n"
7863-
"}");
7864-
}
7865-
78667857
TEST_F(FormatTest, FormatsLambdas) {
78677858
verifyFormat("int c = [b]() mutable {\n"
78687859
" return [&b] { return b++; }();\n"
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
//===- unittest/Format/FormatTestProto.cpp --------------------------------===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#define DEBUG_TYPE "format-test"
11+
12+
#include "FormatTestUtils.h"
13+
#include "clang/Format/Format.h"
14+
#include "llvm/Support/Debug.h"
15+
#include "gtest/gtest.h"
16+
17+
namespace clang {
18+
namespace format {
19+
20+
class FormatTestProto : public ::testing::Test {
21+
protected:
22+
static std::string format(llvm::StringRef Code, unsigned Offset,
23+
unsigned Length, const FormatStyle &Style) {
24+
DEBUG(llvm::errs() << "---\n");
25+
DEBUG(llvm::errs() << Code << "\n\n");
26+
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
27+
tooling::Replacements Replaces = reformat(Style, Code, Ranges);
28+
std::string Result = applyAllReplacements(Code, Replaces);
29+
EXPECT_NE("", Result);
30+
DEBUG(llvm::errs() << "\n" << Result << "\n\n");
31+
return Result;
32+
}
33+
34+
static std::string format(llvm::StringRef Code) {
35+
return format(Code, 0, Code.size(), getGoogleProtoStyle());
36+
}
37+
38+
static void verifyFormat(llvm::StringRef Code) {
39+
EXPECT_EQ(Code.str(), format(test::messUp(Code)));
40+
}
41+
};
42+
43+
TEST_F(FormatTestProto, FormatsMessages) {
44+
verifyFormat("message SomeMessage {\n"
45+
" required int32 field1 = 1;\n"
46+
"}");
47+
verifyFormat("message SomeMessage {\n"
48+
" required int32 field1 = 1;\n"
49+
" optional string field2 = 2 [default = \"2\"]\n"
50+
"}");
51+
}
52+
53+
TEST_F(FormatTestProto, FormatsEnums) {
54+
verifyFormat("enum Type {\n"
55+
" UNKNOWN = 0;\n"
56+
" TYPE_A = 1;\n"
57+
" TYPE_B = 2;\n"
58+
"};");
59+
}
60+
61+
TEST_F(FormatTestProto, UnderstandsReturns) {
62+
verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
63+
}
64+
65+
TEST_F(FormatTestProto, MessageFieldAttributes) {
66+
verifyFormat("optional string test = 1 [default = \"test\"];");
67+
verifyFormat("optional LongMessageType long_proto_field = 1\n"
68+
" [default = REALLY_REALLY_LONG_CONSTANT_VALUE];");
69+
}
70+
71+
} // end namespace tooling
72+
} // end namespace clang

0 commit comments

Comments
 (0)