Fix interaction with CompactNamespaces

Author: Galen Elias

clang/lib/Format/UnwrappedLineFormatter.cpp

@@ -361,9 +361,18 @@ class LineJoiner {
     const auto *FirstNonComment = TheLine->getFirstNonComment();
     if (!FirstNonComment)
       return 0;
+
     // FIXME: There are probably cases where we should use FirstNonComment
     // instead of TheLine->First.
 
+    if (TheLine->Last->is(tok::l_brace)) {
+      if (Style.AllowShortNamespacesOnASingleLine &&
+          TheLine->First->is(tok::kw_namespace)) {
+        if (unsigned result = tryMergeNamespace(I, E, Limit))
+          return result;
+      }
+    }
+
     if (Style.CompactNamespaces) {
       if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
         int J = 1;
@@ -421,14 +430,6 @@ class LineJoiner {
       return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
     }
 
-    if (TheLine->Last->is(tok::l_brace)) {
-      if (Style.AllowShortNamespacesOnASingleLine &&
-          TheLine->First->is(tok::kw_namespace)) {
-        if (unsigned result = tryMergeNamespace(I, E, Limit))
-          return result;
-      }
-    }
-
     // Try to merge a control statement block with left brace unwrapped.
     if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
         FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,

clang/unittests/Format/FormatTest.cpp

@@ -28315,9 +28315,11 @@ TEST_F(FormatTest, KeepFormFeed) {
 }
 
 TEST_F(FormatTest, ShortNamespacesOption) {
-  FormatStyle Style = getLLVMStyle();
-  Style.AllowShortNamespacesOnASingleLine = true;
-  Style.FixNamespaceComments = false;
+  auto BaseStyle = getLLVMStyle();
+  BaseStyle.AllowShortNamespacesOnASingleLine = true;
+  BaseStyle.FixNamespaceComments = false;
+
+  auto Style = BaseStyle;
 
   // Basic functionality.
   verifyFormat("namespace foo { class bar; }", Style);
@@ -28335,7 +28337,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
                "}",
                Style);
 
-  // Make sure code doesn't walk to far on unbalanced code.
+  // Make sure code doesn't walk too far on unbalanced code.
   verifyFormat("namespace foo {", Style);
   verifyFormat("namespace foo {\n"
                "class baz;",
@@ -28348,7 +28350,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
   verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
   verifyFormat("namespace foo {\n"
                "namespace bar { class baz; }\n"
-               "namespace quar { class quaz; }\n"
+               "namespace qux { class quux; }\n"
                "}",
                Style);
 
@@ -28390,20 +28392,32 @@ TEST_F(FormatTest, ShortNamespacesOption) {
   // No ColumnLimit, allows long nested one-liners, but also leaves multi-line
   // instances alone.
   Style.ColumnLimit = 0;
-  verifyFormat("namespace foo { namespace bar { namespace baz { namespace qux "
-               "{ class quux; } } } }",
-               Style);
+  verifyFormat(
+      "namespace foo { namespace bar { namespace baz { class qux; } } }",
+      Style);
 
   verifyNoChange("namespace foo {\n"
-                 "namespace bar {\n"
-                 "namespace baz { namespace qux { class quux; } }\n"
-                 "}\n"
+                 "namespace bar { namespace baz { class qux; } }\n"
                  "}",
                  Style);
 
+  Style = BaseStyle;
+  Style.CompactNamespaces = true;
+  verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
+
+  // If we can't merge an outer nested namespaces, but can merge an inner
+  // nested namespace, then CompactNamespaces will merge the outer namespace
+  // first, preventing the merging of the inner namespace
+  verifyFormat("namespace foo { namespace baz {\n"
+               "class qux;\n"
+               "} // comment\n"
+               "}",
+               Style);
+
   // This option doesn't really work with FixNamespaceComments and nested
   // namespaces. Code should use the concatenated namespace syntax.  e.g.
   // 'namespace foo::bar'.
+  Style = BaseStyle;
   Style.FixNamespaceComments = true;
 
   verifyFormat(