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
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169520 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index 708fbef..a0b6351 100644
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -46,6 +46,12 @@
/// \brief Split two consecutive closing '>' by a space, i.e. use
/// A<A<int> > instead of A<A<int>>.
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/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 51db69c..ebb4949 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -57,6 +57,7 @@
LLVMStyle.PointerAndReferenceBindToType = false;
LLVMStyle.AccessModifierOffset = -2;
LLVMStyle.SplitTemplateClosingGreater = true;
+ LLVMStyle.IndentCaseLabels = false;
return LLVMStyle;
}
@@ -67,6 +68,7 @@
GoogleStyle.PointerAndReferenceBindToType = true;
GoogleStyle.AccessModifierOffset = -1;
GoogleStyle.SplitTemplateClosingGreater = false;
+ GoogleStyle.IndentCaseLabels = true;
return GoogleStyle;
}
@@ -760,7 +762,7 @@
}
tooling::Replacements format() {
- UnwrappedLineParser Parser(Lex, SourceMgr, *this);
+ UnwrappedLineParser Parser(Style, Lex, SourceMgr, *this);
StructuralError = Parser.parse();
for (std::vector<UnwrappedLine>::iterator I = UnwrappedLines.begin(),
E = UnwrappedLines.end();
diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index 425e15b..ebebece 100644
--- a/lib/Format/UnwrappedLineParser.cpp
+++ b/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 @@
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 @@
}
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::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 @@
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/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h
index 3fbc73f..03dda99 100644
--- a/lib/Format/UnwrappedLineParser.h
+++ b/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 @@
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 @@
private:
bool parseLevel();
- bool parseBlock();
+ bool parseBlock(unsigned AddLevels = 1);
void parsePPDirective();
void parseComment();
void parseStatement();
@@ -97,6 +99,7 @@
void parseLabel();
void parseCaseLabel();
void parseSwitch();
+ void parseNamespace();
void parseAccessSpecifier();
void parseEnum();
void addUnwrappedLine();
@@ -111,6 +114,7 @@
FormatToken FormatTok;
bool GreaterStashed;
+ const FormatStyle &Style;
Lexer &Lex;
SourceManager &SourceMgr;
IdentifierTable IdentTable;
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index c385e35..c8416cb 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -59,6 +59,10 @@
}
};
+//===----------------------------------------------------------------------===//
+// Basic function tests.
+//===----------------------------------------------------------------------===//
+
TEST_F(FormatTest, DoesNotChangeCorrectlyFormatedCode) {
EXPECT_EQ(";", format(";"));
}
@@ -78,6 +82,61 @@
EXPECT_EQ("{\n {\n {\n }\n }\n}", format("{{{}}}"));
}
+TEST_F(FormatTest, FormatsNestedCall) {
+ verifyFormat("Method(f1, f2(f3));");
+ verifyFormat("Method(f1(f2, f3()));");
+}
+
+
+//===----------------------------------------------------------------------===//
+// Tests for control statements.
+//===----------------------------------------------------------------------===//
+
+TEST_F(FormatTest, FormatIfWithoutCompountStatement) {
+ verifyFormat("if (true)\n f();\ng();");
+ verifyFormat("if (a)\n if (b)\n if (c)\n g();\nh();");
+ verifyFormat("if (a)\n if (b) {\n f();\n }\ng();");
+ EXPECT_EQ("if (a)\n // comment\n f();", format("if(a)\n// comment\nf();"));
+}
+
+TEST_F(FormatTest, ParseIfElse) {
+ verifyFormat("if (true)\n"
+ " if (true)\n"
+ " if (true)\n"
+ " f();\n"
+ " else\n"
+ " g();\n"
+ " else\n"
+ " h();\n"
+ "else\n"
+ " i();");
+ verifyFormat("if (true)\n"
+ " if (true)\n"
+ " if (true) {\n"
+ " if (true)\n"
+ " f();\n"
+ " } else {\n"
+ " g();\n"
+ " }\n"
+ " else\n"
+ " h();\n"
+ "else {\n"
+ " i();\n"
+ "}");
+}
+
+TEST_F(FormatTest, ElseIf) {
+ verifyFormat("if (a) {\n"
+ "} else if (b) {\n"
+ "}");
+ verifyFormat("if (a)\n"
+ " f();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else\n"
+ " h();");
+}
+
TEST_F(FormatTest, FormatsForLoop) {
verifyFormat(
"for (int VeryVeryLongLoopVariable = 0; VeryVeryLongLoopVariable < 10;\n"
@@ -103,111 +162,16 @@
"}");
}
-TEST_F(FormatTest, FormatsNestedCall) {
- verifyFormat("Method(f1, f2(f3));");
- verifyFormat("Method(f1(f2, f3()));");
+TEST_F(FormatTest, FormatsDoWhile) {
+ verifyFormat("do {\n"
+ " do_something();\n"
+ "} while (something());");
+ verifyFormat("do\n"
+ " do_something();\n"
+ "while (something());");
}
-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"
- "}");
-}
-
-TEST_F(FormatTest, FormatIfWithoutCompountStatement) {
- verifyFormat("if (true)\n f();\ng();");
- verifyFormat("if (a)\n if (b)\n if (c)\n g();\nh();");
- verifyFormat("if (a)\n if (b) {\n f();\n }\ng();");
- EXPECT_EQ("if (a)\n // comment\n f();", format("if(a)\n// comment\nf();"));
-}
-
-TEST_F(FormatTest, ParseIfThenElse) {
- verifyFormat("if (true)\n"
- " if (true)\n"
- " if (true)\n"
- " f();\n"
- " else\n"
- " g();\n"
- " else\n"
- " h();\n"
- "else\n"
- " i();");
- verifyFormat("if (true)\n"
- " if (true)\n"
- " if (true) {\n"
- " if (true)\n"
- " f();\n"
- " } else {\n"
- " g();\n"
- " }\n"
- " else\n"
- " h();\n"
- "else {\n"
- " i();\n"
- "}");
-}
-
-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"
- "}");
- verifyFormat("if (a)\n"
- " f();\n"
- "else if (b)\n"
- " g();\n"
- "else\n"
- " 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, SwitchStatement) {
+TEST_F(FormatTest, FormatsSwitchStatement) {
verifyFormat("switch (x) {\n"
"case 1:\n"
" f();\n"
@@ -228,9 +192,29 @@
"}");
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 @@
"some_other_code();");
}
-TEST_F(FormatTest, DerivedClass) {
+
+//===----------------------------------------------------------------------===//
+// 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, FormatsDerivedClass) {
verifyFormat("class A : public B {\n"
"};");
}
-TEST_F(FormatTest, DoWhile) {
- verifyFormat("do {\n"
- " do_something();\n"
- "} while (something());");
- verifyFormat("do\n"
- " do_something();\n"
- "while (something());");
-}
-
-TEST_F(FormatTest, Enum) {
+TEST_F(FormatTest, FormatsEnum) {
verifyFormat("enum {\n"
" Zero,\n"
" One = 1,\n"
@@ -275,6 +294,53 @@
"};");
}
+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 @@
" \"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 @@
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, 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