[clang-format] FixNamespaceComments does not understand namespace aliases

[MyDeveloperDay]

Summary

https://github.com/llvm/llvm-project/issues/35876

Ensure a namespace alias doesn't get incorrectly identifier as a namespace

namespace nn {}
void f() {
  namespace n = nn;
  {
    int i;
    int j;
  } // namespace n=nn;
}

[curdeius]

clang/lib/Format/NamespaceEndCommentsFixer.cpp

27

It seems to be a good place for optional, nope?

180

Maybe just checking previous (non-comment) token to be not a semicolon would be enough?

251

As an alternative, couldn't we avoid computing the name for namespace aliases by searching the next semicolon (don't compute) or l_brace (compute)?

clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

1189

Nit: NamespaceAlias with small 's'.

[HazardyKnusperkeks]

clang/lib/Format/NamespaceEndCommentsFixer.cpp

59

Should only be equal, right?
Otherwise please add a test for semi.

[MyDeveloperDay]

Look backwards from the { rather than scanning the namespace

[curdeius]

LGTM!

[owenpan]

clang/lib/Format/NamespaceEndCommentsFixer.cpp

185–189

Something like below will do:
if (AnnotatedLines[StartLineIndex - 1]->endsWith(tok::semi))

[MyDeveloperDay]

use endsWith()

clang/lib/Format/NamespaceEndCommentsFixer.cpp

 #define DEBUG_TYPE "namespace-end-comments-fixer"
 
 namespace clang {
 namespace format {
 
 namespace {
 // Computes the name of a namespace given the namespace token.
 // Returns "" for anonymous namespace.
 std::string computeName(const FormatToken *NamespaceTok) {
   assert(NamespaceTok &&
          NamespaceTok->isOneOf(tok::kw_namespace, TT_NamespaceMacro) &&
          "expecting a namespace token");
   std::string name = "";
   const FormatToken *Tok = NamespaceTok->getNextNonComment();
   if (NamespaceTok->is(TT_NamespaceMacro)) {
     // Collects all the non-comment tokens between opening parenthesis
     // and closing parenthesis or comma.
if (NamespaceTok->is(TT_NamespaceMacro)) {
     }
 
     Tok = FirstNSTok;
     while (Tok && !Tok->is(tok::l_brace)) {
       name += Tok->TokenText;
       if (Tok->is(tok::kw_inline))
         name += " ";
       Tok = Tok->getNextNonComment();
     }
   }
   return name;
 }
 
 std::string computeEndCommentText(StringRef NamespaceName, bool AddNewline,
                                   const FormatToken *NamespaceTok,
                                   unsigned SpacesToAdd) {
   std::string text = "//";
getNamespaceToken(const AnnotatedLine *Line,
                   const SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
   if (!Line->Affected || Line->InPPDirective || !Line->startsWith(tok::r_brace))
     return nullptr;
   size_t StartLineIndex = Line->MatchingOpeningBlockLineIndex;
   if (StartLineIndex == UnwrappedLine::kInvalidIndex)
     return nullptr;
   assert(StartLineIndex < AnnotatedLines.size());
   const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First;
   if (NamespaceTok->is(tok::l_brace)) {
     // "namespace" keyword can be on the line preceding '{', e.g. in styles
     // where BraceWrapping.AfterNamespace is true.
-    if (StartLineIndex > 0)
+    if (StartLineIndex > 0) {
       NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First;
+      if (AnnotatedLines[StartLineIndex - 1]->endsWith(tok::semi))
+        return nullptr;
+    }
   }
+
   return NamespaceTok->getNamespaceToken();
 }
 
 StringRef
 getNamespaceTokenText(const AnnotatedLine *Line,
                       const SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
   const FormatToken *NamespaceTok = getNamespaceToken(Line, AnnotatedLines);
   return NamespaceTok ? NamespaceTok->TokenText : StringRef();
for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
     // comments to the semicolon tokens.
     if (RBraceTok->Next && RBraceTok->Next->is(tok::semi)) {
       EndCommentPrevTok = RBraceTok->Next;
     }
     if (StartLineIndex == SIZE_MAX)
       StartLineIndex = EndLine->MatchingOpeningBlockLineIndex;
     std::string NamespaceName = computeName(NamespaceTok);
     if (Style.CompactNamespaces) {
       if (CompactedNamespacesCount == 0)
         NamespaceTokenText = NamespaceTok->TokenText;
       if ((I + 1 < E) &&
           NamespaceTokenText ==
               getNamespaceTokenText(AnnotatedLines[I + 1], AnnotatedLines) &&
           StartLineIndex - CompactedNamespacesCount - 1 ==
               AnnotatedLines[I + 1]->MatchingOpeningBlockLineIndex &&
           !AnnotatedLines[I + 1]->First->Finalized) {
         if (hasEndComment(EndCommentPrevTok)) {

clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

EXPECT_EQ("namespace LongNamespace {\n"
             "}// namespace LongNamespace\n",
             fixNamespaceEndComments("namespace LongNamespace {\n"
                                     "int i;\n"
                                     "int j;\n"
                                     "int k;\n"
                                     "}\n",
                                     Style));
 }
+
+TEST_F(ShortNamespaceLinesTest, NamespaceAlias) {
+  auto Style = getLLVMStyle();
+
+  EXPECT_EQ("namespace n = nn;\n"
+            "{\n"
+            "  int i;\n"
+            "  int j;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace n = nn;\n"
+                                    "{\n"
+                                    "  int i;\n"
+                                    "  int j;\n"
+                                    "}\n",
+                                    Style));
+
+  EXPECT_EQ("namespace n = nn; // comment\n"
+            "{\n"
+            "  int i;\n"
+            "  int j;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace n = nn; // comment\n"
+                                    "{\n"
+                                    "  int i;\n"
+                                    "  int j;\n"
+                                    "}\n",
+                                    Style));
+
+  EXPECT_EQ("namespace n = nn; /* comment */\n"
+            "{\n"
+            "  int i;\n"
+            "  int j;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace n = nn; /* comment */\n"
+                                    "{\n"
+                                    "  int i;\n"
+                                    "  int j;\n"
+                                    "}\n",
+                                    Style));
+
+  EXPECT_EQ(
+      "namespace n = nn; /* comment */ /* comment2 */\n"
+      "{\n"
+      "  int i;\n"
+      "  int j;\n"
+      "}\n",
+      fixNamespaceEndComments("namespace n = nn; /* comment */ /* comment2 */\n"
+                              "{\n"
+                              "  int i;\n"
+                              "  int j;\n"
+                              "}\n",
+                              Style));
+
+  EXPECT_EQ("namespace n = nn; {\n"
+            "  int i;\n"
+            "  int j;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace n = nn; {\n"
+                                    "  int i;\n"
+                                    "  int j;\n"
+                                    "}\n",
+                                    Style));
+  EXPECT_EQ("int foo;\n"
+            "namespace n\n"
+            "{\n"
+            "  int i;\n"
+            "  int j;\n"
+            "}// namespace n\n",
+            fixNamespaceEndComments("int foo;\n"
+                                    "namespace n\n"
+                                    "{\n"
+                                    "  int i;\n"
+                                    "  int j;\n"
+                                    "}\n",
+                                    Style));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang