[clang-format] Add a new flag FixNamespaceComments to FormatStyle

Summary:
This patch enables namespace end comments under a new flag FixNamespaceComments,
which is enabled for the LLVM and Google styles.

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

Differential Revision: https://reviews.llvm.org/D30405

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@296632 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/docs/ClangFormatStyleOptions.rst b/docs/ClangFormatStyleOptions.rst
index 3f76da6dca32c7d072ce3b8404312cf5223d99b4..1876744ebe8602a38e3275359a0accb3737f1fc6 100644 (file)
--- a/docs/ClangFormatStyleOptions.rst
+++ b/docs/ClangFormatStyleOptions.rst
@@ -471,6 +471,10 @@ the configuration (without a prefix: ``Auto``).
   NOTE: This is an experimental flag, that might go away or be renamed. Do
   not use this in config files, etc. Use at your own risk.
 
+**FixNamespaceComments** (``bool``)
+  If ``true``, clang-format adds missing namespace end comments and
+  fixes invalid existing ones.
+
 **ForEachMacros** (``std::vector<std::string>``)
   A vector of macros that should be interpreted as foreach loops
   instead of as function calls.
@@ -561,6 +565,9 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+**JavaScriptWrapImports** (``bool``)
+  Whether to wrap JavaScript import/export statements.
+
 **KeepEmptyLinesAtTheStartOfBlocks** (``bool``)
   If true, empty lines at the start of blocks are kept.
 
@@ -573,7 +580,7 @@ the configuration (without a prefix: ``Auto``).
     Do not use.
 
   * ``LK_Cpp`` (in configuration: ``Cpp``)
-    Should be used for C, C++, ObjectiveC, ObjectiveC++.
+    Should be used for C, C++.
 
   * ``LK_Java`` (in configuration: ``Java``)
     Should be used for Java.
@@ -581,6 +588,9 @@ the configuration (without a prefix: ``Auto``).
   * ``LK_JavaScript`` (in configuration: ``JavaScript``)
     Should be used for JavaScript.
 
+  * ``LK_ObjC`` (in configuration: ``ObjC``)
+    Should be used for Objective-C, Objective-C++.
+
   * ``LK_Proto`` (in configuration: ``Proto``)
     Should be used for Protocol Buffers
     (https://developers.google.com/protocol-buffers/).
@@ -755,6 +765,9 @@ the configuration (without a prefix: ``Auto``).
   * ``UT_ForIndentation`` (in configuration: ``ForIndentation``)
     Use tabs only for indentation.
 
+  * ``UT_ForContinuationAndIndentation`` (in configuration: ``ForContinuationAndIndentation``)
+    Use tabs only for line continuation and indentation.
+
   * ``UT_Always`` (in configuration: ``Always``)
     Use tabs whenever we need to fill whitespace that spans at least from
     one tab stop to the next one.

diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index bce68f8554283517081e4bf6e3ac28044404855e..f11f3b8dfd04c2ac6f93aac3b6de0337cadebf04 100644 (file)
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -349,6 +349,10 @@ struct FormatStyle {
   /// not use this in config files, etc. Use at your own risk.
   bool ExperimentalAutoDetectBinPacking;
 
+  /// \brief If ``true``, clang-format adds missing namespace end comments and
+  /// fixes invalid existing ones.
+  bool FixNamespaceComments;
+
   /// \brief A vector of macros that should be interpreted as foreach loops
   /// instead of as function calls.
   ///
@@ -679,6 +683,7 @@ struct FormatStyle {
            DisableFormat == R.DisableFormat &&
            ExperimentalAutoDetectBinPacking ==
                R.ExperimentalAutoDetectBinPacking &&
+           FixNamespaceComments == R.FixNamespaceComments &&
            ForEachMacros == R.ForEachMacros &&
            IncludeCategories == R.IncludeCategories &&
            IndentCaseLabels == R.IndentCaseLabels &&

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 28e997787147c12c53214fa358640ad1c65cec72..9910e4316bc399635dc19311f0791f0182f0b5aa 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -308,6 +308,7 @@ template <> struct MappingTraits<FormatStyle> {
     IO.mapOptional("DisableFormat", Style.DisableFormat);
     IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
+    IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
     IO.mapOptional("ForEachMacros", Style.ForEachMacros);
     IO.mapOptional("IncludeCategories", Style.IncludeCategories);
     IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
@@ -529,6 +530,7 @@ FormatStyle getLLVMStyle() {
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
+  LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
   LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
   LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");
@@ -676,6 +678,7 @@ FormatStyle getMozillaStyle() {
   MozillaStyle.ConstructorInitializerIndentWidth = 2;
   MozillaStyle.ContinuationIndentWidth = 2;
   MozillaStyle.Cpp11BracedListStyle = false;
+  MozillaStyle.FixNamespaceComments = false;
   MozillaStyle.IndentCaseLabels = true;
   MozillaStyle.ObjCSpaceAfterProperty = true;
   MozillaStyle.ObjCSpaceBeforeProtocolList = false;
@@ -696,6 +699,7 @@ FormatStyle getWebKitStyle() {
   Style.BreakConstructorInitializersBeforeComma = true;
   Style.Cpp11BracedListStyle = false;
   Style.ColumnLimit = 0;
+  Style.FixNamespaceComments = false;
   Style.IndentWidth = 4;
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   Style.ObjCBlockIndentWidth = 4;
@@ -713,6 +717,7 @@ FormatStyle getGNUStyle() {
   Style.BreakBeforeTernaryOperators = true;
   Style.Cpp11BracedListStyle = false;
   Style.ColumnLimit = 79;
+  Style.FixNamespaceComments = false;
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   Style.Standard = FormatStyle::LS_Cpp03;
   return Style;
@@ -1829,20 +1834,32 @@ tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
     return tooling::Replacements();
   auto Env = Environment::CreateVirtualEnvironment(Code, FileName, Ranges);
 
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-      Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) {
-    JavaScriptRequoter Requoter(*Env, Expanded);
-    tooling::Replacements Requotes = Requoter.process();
-    if (!Requotes.empty()) {
-      auto NewCode = applyAllReplacements(Code, Requotes);
+  auto reformatAfterApplying = [&] (TokenAnalyzer& Fixer) {
+    tooling::Replacements Fixes = Fixer.process();
+    if (!Fixes.empty()) {
+      auto NewCode = applyAllReplacements(Code, Fixes);
       if (NewCode) {
         auto NewEnv = Environment::CreateVirtualEnvironment(
             *NewCode, FileName,
-            tooling::calculateRangesAfterReplacements(Requotes, Ranges));
+            tooling::calculateRangesAfterReplacements(Fixes, Ranges));
         Formatter Format(*NewEnv, Expanded, IncompleteFormat);
-        return Requotes.merge(Format.process());
+        return Fixes.merge(Format.process());
       }
     }
+    Formatter Format(*Env, Expanded, IncompleteFormat);
+    return Format.process();
+  };
+
+  if (Style.Language == FormatStyle::LK_Cpp &&
+      Style.FixNamespaceComments) {
+    NamespaceEndCommentsFixer CommentsFixer(*Env, Expanded);
+    return reformatAfterApplying(CommentsFixer);
+  }
+
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+      Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) {
+    JavaScriptRequoter Requoter(*Env, Expanded);
+    return reformatAfterApplying(Requoter);
   }
 
   Formatter Format(*Env, Expanded, IncompleteFormat);

diff --git a/lib/Index/CommentToXML.cpp b/lib/Index/CommentToXML.cpp
index ee066cc6d985a2202e3bb31e1eb5fb6f989ea283..08acc96c4efb6417fc163f084744ea8355885d15 100644 (file)
--- a/lib/Index/CommentToXML.cpp
+++ b/lib/Index/CommentToXML.cpp
@@ -593,9 +593,11 @@ void CommentASTToXMLConverter::formatTextOfDeclaration(
   unsigned Length = Declaration.size();
 
   bool IncompleteFormat = false;
+  format::FormatStyle Style = format::getLLVMStyle();
+  Style.FixNamespaceComments = false;
   tooling::Replacements Replaces =
-      reformat(format::getLLVMStyle(), StringDecl,
-               tooling::Range(Offset, Length), "xmldecl.xd", &IncompleteFormat);
+      reformat(Style, StringDecl, tooling::Range(Offset, Length), "xmldecl.xd",
+               &IncompleteFormat);
   auto FormattedStringDecl = applyAllReplacements(StringDecl, Replaces);
   if (static_cast<bool>(FormattedStringDecl)) {
     Declaration = *FormattedStringDecl;

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 1562bffd3f9f56a3a6d84d2c55e71945d86a1a95..30cc749624632f2970e37f832755042e89830e89 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -185,7 +185,7 @@ TEST_F(FormatTest, RemovesEmptyLines) {
   EXPECT_EQ("namespace N {\n"
             "\n"
             "int i;\n"
-            "}",
+            "}  // namespace N",
             format("namespace N {\n"
                    "\n"
                    "int    i;\n"
@@ -270,9 +270,19 @@ TEST_F(FormatTest, RemovesEmptyLines) {
                    "}"));
 
   // FIXME: This is slightly inconsistent.
+  FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle();
+  LLVMWithNoNamespaceFix.FixNamespaceComments = false;
   EXPECT_EQ("namespace {\n"
             "int i;\n"
             "}",
+            format("namespace {\n"
+                   "int i;\n"
+                   "\n"
+                   "}", LLVMWithNoNamespaceFix));
+  EXPECT_EQ("namespace {\n"
+            "int i;\n"
+            "\n"
+            "} // namespace",
             format("namespace {\n"
                    "int i;\n"
                    "\n"
@@ -1197,33 +1207,43 @@ TEST_F(FormatTest, FormatsBitfields) {
 }
 
 TEST_F(FormatTest, FormatsNamespaces) {
+  FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle();
+  LLVMWithNoNamespaceFix.FixNamespaceComments = false;
+
   verifyFormat("namespace some_namespace {\n"
                "class A {};\n"
                "void f() { f(); }\n"
-               "}");
+               "}",
+               LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
                "class A {};\n"
                "void f() { f(); }\n"
-               "}");
+               "}",
+               LLVMWithNoNamespaceFix);
   verifyFormat("inline namespace X {\n"
                "class A {};\n"
                "void f() { f(); }\n"
-               "}");
+               "}",
+               LLVMWithNoNamespaceFix);
   verifyFormat("using namespace some_namespace;\n"
                "class A {};\n"
-               "void f() { f(); }");
+               "void f() { f(); }",
+               LLVMWithNoNamespaceFix);
 
   // This code is more common than we thought; if we
   // layout this correctly the semicolon will go into
   // its own line, which is undesirable.
-  verifyFormat("namespace {};");
+  verifyFormat("namespace {};",
+               LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
                "class A {};\n"
-               "};");
+               "};",
+               LLVMWithNoNamespaceFix);
 
   verifyFormat("namespace {\n"
                "int SomeVariable = 0; // comment\n"
-               "} // namespace");
+               "} // namespace",
+               LLVMWithNoNamespaceFix);
   EXPECT_EQ("#ifndef HEADER_GUARD\n"
             "#define HEADER_GUARD\n"
             "namespace my_namespace {\n"
@@ -1235,14 +1255,16 @@ TEST_F(FormatTest, FormatsNamespaces) {
                    "   namespace my_namespace {\n"
                    "int i;\n"
                    "}    // my_namespace\n"
-                   "#endif    // HEADER_GUARD"));
+                   "#endif    // HEADER_GUARD",
+                   LLVMWithNoNamespaceFix));
 
   EXPECT_EQ("namespace A::B {\n"
             "class C {};\n"
             "}",
             format("namespace A::B {\n"
                    "class C {};\n"
-                   "}"));
+                   "}",
+                   LLVMWithNoNamespaceFix));
 
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceIndentation = FormatStyle::NI_All;
@@ -1250,14 +1272,14 @@ TEST_F(FormatTest, FormatsNamespaces) {
             "  int i;\n"
             "  namespace in {\n"
             "    int i;\n"
-            "  } // namespace\n"
-            "} // namespace",
+            "  } // namespace in\n"
+            "} // namespace out",
             format("namespace out {\n"
                    "int i;\n"
                    "namespace in {\n"
                    "int i;\n"
-                   "} // namespace\n"
-                   "} // namespace",
+                   "} // namespace in\n"
+                   "} // namespace out",
                    Style));
 
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
@@ -1265,14 +1287,14 @@ TEST_F(FormatTest, FormatsNamespaces) {
             "int i;\n"
             "namespace in {\n"
             "  int i;\n"
-            "} // namespace\n"
-            "} // namespace",
+            "} // namespace in\n"
+            "} // namespace out",
             format("namespace out {\n"
                    "int i;\n"
                    "namespace in {\n"
                    "int i;\n"
-                   "} // namespace\n"
-                   "} // namespace",
+                   "} // namespace in\n"
+                   "} // namespace out",
                    Style));
 }
 
@@ -1776,11 +1798,11 @@ TEST_F(FormatTest, MacrosWithoutTrailingSemicolon) {
   EXPECT_EQ("SOME_MACRO\n"
             "namespace {\n"
             "void f();\n"
-            "}",
+            "} // namespace",
             format("SOME_MACRO\n"
                    "  namespace    {\n"
                    "void   f(  );\n"
-                   "}"));
+                   "} // namespace"));
   // Only if the identifier contains at least 5 characters.
   EXPECT_EQ("HTTP f();", format("HTTP\nf();"));
   EXPECT_EQ("MACRO\nf();", format("MACRO\nf();"));
@@ -5498,7 +5520,7 @@ TEST_F(FormatTest, DoesNotTouchUnwrappedLinesWithErrors) {
   verifyIncompleteFormat("namespace {\n"
                          "class Foo { Foo (\n"
                          "};\n"
-                         "} // comment");
+                         "} // namespace");
 }
 
 TEST_F(FormatTest, IncorrectCodeErrorDetection) {
@@ -7853,6 +7875,7 @@ TEST_F(FormatTest, LinuxBraceBreaking) {
 TEST_F(FormatTest, MozillaBraceBreaking) {
   FormatStyle MozillaBraceStyle = getLLVMStyle();
   MozillaBraceStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla;
+  MozillaBraceStyle.FixNamespaceComments = false;
   verifyFormat("namespace a {\n"
                "class A\n"
                "{\n"
@@ -8269,6 +8292,7 @@ TEST_F(FormatTest, GNUBraceBreaking) {
 TEST_F(FormatTest, WebKitBraceBreaking) {
   FormatStyle WebKitBraceStyle = getLLVMStyle();
   WebKitBraceStyle.BreakBeforeBraces = FormatStyle::BS_WebKit;
+  WebKitBraceStyle.FixNamespaceComments = false;
   verifyFormat("namespace a {\n"
                "class A {\n"
                "  void f()\n"