From 578fdd8968503056dc4bd8d7664281cd3e3595e0 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Thu, 6 Dec 2012 18:03:27 +0000 Subject: [PATCH] Clang-format: IndentCaseLabels option, proper namespace handling Summary: + tests arranged in groups, as their number is already quite large. Reviewers: djasper, klimek Reviewed By: djasper CC: cfe-commits Differential Revision: http://llvm-reviews.chandlerc.com/D185 llvm-svn: 169520 --- clang/include/clang/Format/Format.h | 6 + clang/lib/Format/Format.cpp | 4 +- clang/lib/Format/UnwrappedLineParser.cpp | 37 ++-- clang/lib/Format/UnwrappedLineParser.h | 8 +- clang/unittests/Format/FormatTest.cpp | 289 +++++++++++++++++++------------ 5 files changed, 220 insertions(+), 124 deletions(-) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 708fbef..a0b6351 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -46,6 +46,12 @@ struct FormatStyle { /// \brief Split two consecutive closing '>' by a space, i.e. use /// A > instead of A>. bool SplitTemplateClosingGreater; + + /// \brief Indent case labels one level from the switch statement. + /// + /// When false, use the same indentation level as for the switch statement. + /// Switch statement body is always indented one level more than case labels. + bool IndentCaseLabels; }; /// \brief Returns a format style complying with the LLVM coding standards: diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 51db69c..ebb4949 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -57,6 +57,7 @@ FormatStyle getLLVMStyle() { LLVMStyle.PointerAndReferenceBindToType = false; LLVMStyle.AccessModifierOffset = -2; LLVMStyle.SplitTemplateClosingGreater = true; + LLVMStyle.IndentCaseLabels = false; return LLVMStyle; } @@ -67,6 +68,7 @@ FormatStyle getGoogleStyle() { GoogleStyle.PointerAndReferenceBindToType = true; GoogleStyle.AccessModifierOffset = -1; GoogleStyle.SplitTemplateClosingGreater = false; + GoogleStyle.IndentCaseLabels = true; return GoogleStyle; } @@ -760,7 +762,7 @@ public: } tooling::Replacements format() { - UnwrappedLineParser Parser(Lex, SourceMgr, *this); + UnwrappedLineParser Parser(Style, Lex, SourceMgr, *this); StructuralError = Parser.parse(); for (std::vector::iterator I = UnwrappedLines.begin(), E = UnwrappedLines.end(); diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 425e15b..ebebece 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -22,9 +22,11 @@ namespace clang { namespace format { -UnwrappedLineParser::UnwrappedLineParser(Lexer &Lex, SourceManager &SourceMgr, +UnwrappedLineParser::UnwrappedLineParser(const FormatStyle &Style, Lexer &Lex, + SourceManager &SourceMgr, UnwrappedLineConsumer &Callback) : GreaterStashed(false), + Style(Style), Lex(Lex), SourceMgr(SourceMgr), IdentTable(Lex.getLangOpts()), @@ -62,20 +64,16 @@ bool UnwrappedLineParser::parseLevel() { return Error; } -bool UnwrappedLineParser::parseBlock() { +bool UnwrappedLineParser::parseBlock(unsigned AddLevels) { assert(FormatTok.Tok.is(tok::l_brace) && "'{' expected"); nextToken(); - // FIXME: Remove this hack to handle namespaces. - bool IsNamespace = Line.Tokens[0].Tok.is(tok::kw_namespace); - addUnwrappedLine(); - if (!IsNamespace) - ++Line.Level; + Line.Level += AddLevels; parseLevel(); - if (!IsNamespace) - --Line.Level; + Line.Level -= AddLevels; + // FIXME: Add error handling. if (!FormatTok.Tok.is(tok::r_brace)) return true; @@ -114,6 +112,9 @@ void UnwrappedLineParser::parseStatement() { } switch (FormatTok.Tok.getKind()) { + case tok::kw_namespace: + parseNamespace(); + return; case tok::kw_public: case tok::kw_protected: case tok::kw_private: @@ -224,6 +225,18 @@ void UnwrappedLineParser::parseIfThenElse() { } } +void UnwrappedLineParser::parseNamespace() { + assert(FormatTok.Tok.is(tok::kw_namespace) && "'namespace' expected"); + nextToken(); + if (FormatTok.Tok.is(tok::identifier)) + nextToken(); + if (FormatTok.Tok.is(tok::l_brace)) { + parseBlock(0); + addUnwrappedLine(); + } + // FIXME: Add error handling. +} + void UnwrappedLineParser::parseForOrWhileLoop() { assert((FormatTok.Tok.is(tok::kw_for) || FormatTok.Tok.is(tok::kw_while)) && "'for' or 'while' expected"); @@ -290,13 +303,13 @@ void UnwrappedLineParser::parseSwitch() { nextToken(); parseParens(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(); + parseBlock(Style.IndentCaseLabels ? 2 : 1); addUnwrappedLine(); } else { addUnwrappedLine(); - ++Line.Level; + Line.Level += (Style.IndentCaseLabels ? 2 : 1); parseStatement(); - --Line.Level; + Line.Level -= (Style.IndentCaseLabels ? 2 : 1); } } diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index 3fbc73f..03dda99 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -21,6 +21,7 @@ #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" namespace clang { @@ -78,7 +79,8 @@ public: class UnwrappedLineParser { public: - UnwrappedLineParser(Lexer &Lex, SourceManager &SourceMgr, + UnwrappedLineParser(const FormatStyle &Style, Lexer &Lex, + SourceManager &SourceMgr, UnwrappedLineConsumer &Callback); /// Returns true in case of a structural error. @@ -86,7 +88,7 @@ public: private: bool parseLevel(); - bool parseBlock(); + bool parseBlock(unsigned AddLevels = 1); void parsePPDirective(); void parseComment(); void parseStatement(); @@ -97,6 +99,7 @@ private: void parseLabel(); void parseCaseLabel(); void parseSwitch(); + void parseNamespace(); void parseAccessSpecifier(); void parseEnum(); void addUnwrappedLine(); @@ -111,6 +114,7 @@ private: FormatToken FormatTok; bool GreaterStashed; + const FormatStyle &Style; Lexer &Lex; SourceManager &SourceMgr; IdentifierTable IdentTable; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index c385e35..c8416cb 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -59,6 +59,10 @@ protected: } }; +//===----------------------------------------------------------------------===// +// Basic function tests. +//===----------------------------------------------------------------------===// + TEST_F(FormatTest, DoesNotChangeCorrectlyFormatedCode) { EXPECT_EQ(";", format(";")); } @@ -78,48 +82,15 @@ TEST_F(FormatTest, FormatsNestedBlockStatements) { EXPECT_EQ("{\n {\n {\n }\n }\n}", format("{{{}}}")); } -TEST_F(FormatTest, FormatsForLoop) { - verifyFormat( - "for (int VeryVeryLongLoopVariable = 0; VeryVeryLongLoopVariable < 10;\n" - " ++VeryVeryLongLoopVariable)\n" - " ;"); - verifyFormat("for (;;)\n" - " f();"); - verifyFormat("for (;;) {\n" - "}"); - verifyFormat("for (;;) {\n" - " f();\n" - "}"); -} - -TEST_F(FormatTest, FormatsWhileLoop) { - verifyFormat("while (true) {\n}"); - verifyFormat("while (true)\n" - " f();"); - verifyFormat("while () {\n" - "}"); - verifyFormat("while () {\n" - " f();\n" - "}"); -} - TEST_F(FormatTest, FormatsNestedCall) { verifyFormat("Method(f1, f2(f3));"); verifyFormat("Method(f1(f2, f3()));"); } -TEST_F(FormatTest, FormatsAwesomeMethodCall) { - verifyFormat( - "SomeLongMethodName(SomeReallyLongMethod(CallOtherReallyLongMethod(\n" - " parameter, parameter, parameter)), SecondLongCall(parameter));"); -} -TEST_F(FormatTest, FormatsFunctionDefinition) { - verifyFormat("void f(int a, int b, int c, int d, int e, int f, int g," - " int h, int j, int f,\n" - " int c, int ddddddddddddd) {\n" - "}"); -} +//===----------------------------------------------------------------------===// +// Tests for control statements. +//===----------------------------------------------------------------------===// TEST_F(FormatTest, FormatIfWithoutCompountStatement) { verifyFormat("if (true)\n f();\ng();"); @@ -128,7 +99,7 @@ TEST_F(FormatTest, FormatIfWithoutCompountStatement) { EXPECT_EQ("if (a)\n // comment\n f();", format("if(a)\n// comment\nf();")); } -TEST_F(FormatTest, ParseIfThenElse) { +TEST_F(FormatTest, ParseIfElse) { verifyFormat("if (true)\n" " if (true)\n" " if (true)\n" @@ -154,30 +125,6 @@ TEST_F(FormatTest, ParseIfThenElse) { "}"); } -TEST_F(FormatTest, UnderstandsSingleLineComments) { - EXPECT_EQ("// line 1\n// line 2\nvoid f() {\n}\n", - format("// line 1\n// line 2\nvoid f() {}\n")); - - EXPECT_EQ("void f() {\n // Doesn't do anything\n}", - format("void f() {\n// Doesn't do anything\n}")); - - EXPECT_EQ("int i // This is a fancy variable\n = 5;", - format("int i // This is a fancy variable\n= 5;")); - - verifyFormat("f(/*test=*/ true);"); -} - -TEST_F(FormatTest, DoesNotBreakSemiAfterClassDecl) { - verifyFormat("class A {\n};"); -} - -TEST_F(FormatTest, BreaksAsHighAsPossible) { - verifyFormat( - "if ((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaaaaaaaaaaaaa) ||\n" - " (bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && bbbbbbbbbbbbbbbbbbbbbbbbbb))\n" - " f();"); -} - TEST_F(FormatTest, ElseIf) { verifyFormat("if (a) {\n" "} else if (b) {\n" @@ -190,24 +137,41 @@ TEST_F(FormatTest, ElseIf) { " h();"); } -TEST_F(FormatTest, UnderstandsAccessSpecifiers) { - verifyFormat("class A {\n" - "public:\n" - "protected:\n" - "private:\n" - " void f() {\n" - " }\n" - "};"); - verifyGoogleFormat("class A {\n" - " public:\n" - " protected:\n" - " private:\n" - " void f() {\n" - " }\n" - "};"); +TEST_F(FormatTest, FormatsForLoop) { + verifyFormat( + "for (int VeryVeryLongLoopVariable = 0; VeryVeryLongLoopVariable < 10;\n" + " ++VeryVeryLongLoopVariable)\n" + " ;"); + verifyFormat("for (;;)\n" + " f();"); + verifyFormat("for (;;) {\n" + "}"); + verifyFormat("for (;;) {\n" + " f();\n" + "}"); +} + +TEST_F(FormatTest, FormatsWhileLoop) { + verifyFormat("while (true) {\n}"); + verifyFormat("while (true)\n" + " f();"); + verifyFormat("while () {\n" + "}"); + verifyFormat("while () {\n" + " f();\n" + "}"); +} + +TEST_F(FormatTest, FormatsDoWhile) { + verifyFormat("do {\n" + " do_something();\n" + "} while (something());"); + verifyFormat("do\n" + " do_something();\n" + "while (something());"); } -TEST_F(FormatTest, SwitchStatement) { +TEST_F(FormatTest, FormatsSwitchStatement) { verifyFormat("switch (x) {\n" "case 1:\n" " f();\n" @@ -228,9 +192,29 @@ TEST_F(FormatTest, SwitchStatement) { "}"); verifyFormat("switch (test)\n" " ;"); + verifyGoogleFormat("switch (x) {\n" + " case 1:\n" + " f();\n" + " break;\n" + " case kFoo:\n" + " case ns::kBar:\n" + " case kBaz:\n" + " break;\n" + " default:\n" + " g();\n" + " break;\n" + "}"); + verifyGoogleFormat("switch (x) {\n" + " case 1: {\n" + " f();\n" + " break;\n" + " }\n" + "}"); + verifyGoogleFormat("switch (test)\n" + " ;"); } -TEST_F(FormatTest, Labels) { +TEST_F(FormatTest, FormatsLabels) { verifyFormat("void f() {\n" " some_code();\n" "test_label:\n" @@ -246,21 +230,56 @@ TEST_F(FormatTest, Labels) { "some_other_code();"); } -TEST_F(FormatTest, DerivedClass) { - verifyFormat("class A : public B {\n" + +//===----------------------------------------------------------------------===// +// Tests for comments. +//===----------------------------------------------------------------------===// + +TEST_F(FormatTest, UnderstandsSingleLineComments) { + EXPECT_EQ("// line 1\n// line 2\nvoid f() {\n}\n", + format("// line 1\n// line 2\nvoid f() {}\n")); + + EXPECT_EQ("void f() {\n // Doesn't do anything\n}", + format("void f() {\n// Doesn't do anything\n}")); + + EXPECT_EQ("int i // This is a fancy variable\n = 5;", + format("int i // This is a fancy variable\n= 5;")); + + verifyFormat("f(/*test=*/ true);"); +} + + +//===----------------------------------------------------------------------===// +// Tests for classes, namespaces, etc. +//===----------------------------------------------------------------------===// + +TEST_F(FormatTest, DoesNotBreakSemiAfterClassDecl) { + verifyFormat("class A {\n};"); +} + +TEST_F(FormatTest, UnderstandsAccessSpecifiers) { + verifyFormat("class A {\n" + "public:\n" + "protected:\n" + "private:\n" + " void f() {\n" + " }\n" "};"); + verifyGoogleFormat("class A {\n" + " public:\n" + " protected:\n" + " private:\n" + " void f() {\n" + " }\n" + "};"); } -TEST_F(FormatTest, DoWhile) { - verifyFormat("do {\n" - " do_something();\n" - "} while (something());"); - verifyFormat("do\n" - " do_something();\n" - "while (something());"); +TEST_F(FormatTest, FormatsDerivedClass) { + verifyFormat("class A : public B {\n" + "};"); } -TEST_F(FormatTest, Enum) { +TEST_F(FormatTest, FormatsEnum) { verifyFormat("enum {\n" " Zero,\n" " One = 1,\n" @@ -275,6 +294,53 @@ TEST_F(FormatTest, Enum) { "};"); } +TEST_F(FormatTest, FormatsNamespaces) { + verifyFormat("namespace some_namespace {\n" + "class A {\n" + "};\n" + "void f() {\n" + " f();\n" + "}\n" + "}"); + verifyFormat("namespace {\n" + "class A {\n" + "};\n" + "void f() {\n" + " f();\n" + "}\n" + "}"); + verifyFormat("using namespace some_namespace;\n" + "class A {\n" + "};\n" + "void f() {\n" + " f();\n" + "}"); +} + +//===----------------------------------------------------------------------===// +// Line break tests. +//===----------------------------------------------------------------------===// + +TEST_F(FormatTest, FormatsFunctionDefinition) { + verifyFormat("void f(int a, int b, int c, int d, int e, int f, int g," + " int h, int j, int f,\n" + " int c, int ddddddddddddd) {\n" + "}"); +} + +TEST_F(FormatTest, FormatsAwesomeMethodCall) { + verifyFormat( + "SomeLongMethodName(SomeReallyLongMethod(CallOtherReallyLongMethod(\n" + " parameter, parameter, parameter)), SecondLongCall(parameter));"); +} + +TEST_F(FormatTest, BreaksAsHighAsPossible) { + verifyFormat( + "if ((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaaaaaaaaaaaaa) ||\n" + " (bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && bbbbbbbbbbbbbbbbbbbbbbbbbb))\n" + " f();"); +} + TEST_F(FormatTest, BreaksDesireably) { verifyFormat("if (aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaa) ||\n" " aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaa) ||\n" @@ -315,6 +381,27 @@ TEST_F(FormatTest, AlignsStringLiterals) { " \"looooooooooooooooooooooooooooooooooooooooooooooooong literal\");"); } +TEST_F(FormatTest, AlignsPipes) { + verifyFormat( + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); + verifyFormat( + "aaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaa\n" + " << aaaaaaaaaaaaaaaaaaaa;"); + verifyFormat( + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " << aaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); + verifyFormat( + "llvm::outs() << \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\n" + " \"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\"\n" + " << \"ccccccccccccccccccccccccccccccccccccccccccccccccc\";"); + verifyFormat( + "aaaaaaaa << (aaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); +} + TEST_F(FormatTest, UnderstandsEquals) { verifyFormat( "aaaaaaaaaaaaaaaaa =\n" @@ -401,6 +488,11 @@ TEST_F(FormatTest, HandlesIncludeDirectives) { EXPECT_EQ("#include \"string.h\"\n", format("#include \"string.h\"\n")); } + +//===----------------------------------------------------------------------===// +// Error recovery tests. +//===----------------------------------------------------------------------===// + //TEST_F(FormatTest, IncorrectDerivedClass) { // verifyFormat("public B {\n" // "};"); @@ -443,26 +535,5 @@ TEST_F(FormatTest, IncorrectCodeErrorDetection) { } -TEST_F(FormatTest, AlignsPipes) { - verifyFormat( - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); - verifyFormat( - "aaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaa\n" - " << aaaaaaaaaaaaaaaaaaaa;"); - verifyFormat( - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " << aaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); - verifyFormat( - "llvm::outs() << \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\n" - " \"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\"\n" - " << \"ccccccccccccccccccccccccccccccccccccccccccccccccc\";"); - verifyFormat( - "aaaaaaaa << (aaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" - " << aaaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); -} - } // end namespace tooling } // end namespace clang -- 2.7.4