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 708fbefb6ee1747cdea3a9d270d38ec31c74e50a..a0b63514574ed1e30a6e68daeeb61d64c61741d8 100644 (file)
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -46,6 +46,12 @@ struct FormatStyle {
   /// \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 51db69c5ae65dc1371ea2b70770b5dfa7e563d9c..ebb49498da1b8d4797bb785434416b17e4e03194 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/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<UnwrappedLine>::iterator I = UnwrappedLines.begin(),
                                               E = UnwrappedLines.end();

diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index 425e15b81f99dddaf41f5670a42b2616e2066a1b..ebebece7eb435622fb580beefdb2b26ec9764572 100644 (file)
--- 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 @@ 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/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h
index 3fbc73fd75b9a04cf4ec586646f7b6f3dc6e64a2..03dda9957da12f44e24ab4a605c8b21d12b4fe61 100644 (file)
--- 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 @@ 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/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index c385e353d6930c11b25be40c3ba6f652b69d55f7..c8416cb3529135f607d6763ab6317c34e3a0c3a3 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/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