[change-namespace] consider namespace aliases to shorten qualified names.

[hokein]

change-namespace/ChangeNamespace.cpp

719 ↗

(On Diff #82322)

maybe assert(StringRef(AliasQualifiedName).end_with("::"+AliasName))?

741 ↗

(On Diff #82322)

Do we also need to check FromDecl->getNameAsString().size() < ReplaceName.size() before assigning the name to ReplaceName?

unittests/change-namespace/ChangeNamespaceTests.cpp

832 ↗

(On Diff #82322)

Add a testcase for the namespace alias decl with preceding ::, e.g. namespace gl = ::glob?

[ioeric]

• addressed review comments.

[hokein]

LGTM.

clang-tools-extra/trunk/change-namespace/ChangeNamespace.h

private:
   // inserting it to the new namespace.
   std::map<std::string, std::vector<InsertForwardDeclaration>> InsertFwdDecls;
   // Records all using declarations, which can be used to shorten namespace
   // specifiers.
   llvm::SmallPtrSet<const UsingDecl *, 8> UsingDecls;
   // Records all using namespace declarations, which can be used to shorten
   // namespace specifiers.
   llvm::SmallPtrSet<const UsingDirectiveDecl *, 8> UsingNamespaceDecls;
+  // Records all namespace alias declarations, which can be used to shorten
+  // namespace specifiers.
+  llvm::SmallPtrSet<const NamespaceAliasDecl *, 8> NamespaceAliasDecls;
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need to
   // be fixed.
   llvm::SmallVector<TypeLoc, 8> BaseCtorInitializerTypeLocs;
   // Since a DeclRefExpr for a function call can be matched twice (one as
   // CallExpr and one as DeclRefExpr), we record all DeclRefExpr's that have
   // been processed so that we don't handle them twice.
   llvm::SmallPtrSet<const clang::DeclRefExpr*, 16> ProcessedFuncRefs;
 };
 
 } // namespace change_namespace
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CHANGE_NAMESPACE_CHANGENAMESPACE_H

clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp

Finder->addMatcher(
       usingDecl(isExpansionInFileMatching(FilePattern), IsVisibleInNewNs)
           .bind("using"),
       this);
   // Match using namespace declarations.
   Finder->addMatcher(usingDirectiveDecl(isExpansionInFileMatching(FilePattern),
                                         IsVisibleInNewNs)
                          .bind("using_namespace"),
                      this);
+  // Match namespace alias declarations.
+  Finder->addMatcher(namespaceAliasDecl(isExpansionInFileMatching(FilePattern),
+                                        IsVisibleInNewNs)
+                         .bind("namespace_alias"),
+                     this);
 
   // Match old namespace blocks.
   Finder->addMatcher(
       namespaceDecl(hasName(FullOldNs), isExpansionInFileMatching(FilePattern))
           .bind("old_ns"),
       this);
 
   // Match class forward-declarations in the old namespace.

 void ChangeNamespaceTool::run(
     const ast_matchers::MatchFinder::MatchResult &Result) {
   if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
     UsingDecls.insert(Using);
   } else if (const auto *UsingNamespace =
                  Result.Nodes.getNodeAs<UsingDirectiveDecl>(
                      "using_namespace")) {
     UsingNamespaceDecls.insert(UsingNamespace);
+  } else if (const auto *NamespaceAlias =
+                 Result.Nodes.getNodeAs<NamespaceAliasDecl>(
+                     "namespace_alias")) {
+    NamespaceAliasDecls.insert(NamespaceAlias);
   } else if (const auto *NsDecl =
                  Result.Nodes.getNodeAs<NamespaceDecl>("old_ns")) {
     moveOldNamespace(Result, NsDecl);
   } else if (const auto *FwdDecl =
                  Result.Nodes.getNodeAs<CXXRecordDecl>("class_fwd_decl")) {
     moveClassForwardDeclaration(Result, cast<NamedDecl>(FwdDecl));
   } else if (const auto *TemplateFwdDecl =
                  Result.Nodes.getNodeAs<ClassTemplateDecl>(
for (const auto *UsingNamespace : UsingNamespaceDecls) {
     StringRef FromDeclNameRef = FromDeclName;
     if (FromDeclNameRef.consume_front(UsingNamespace->getNominatedNamespace()
                                           ->getQualifiedNameAsString())) {
       FromDeclNameRef = FromDeclNameRef.drop_front(2);
       if (FromDeclNameRef.size() < ReplaceName.size())
         ReplaceName = FromDeclNameRef;
     }
   }
+  // Checks if there is any namespace alias declarations that can shorten the
+  // qualified name.
+  for (const auto *NamespaceAlias : NamespaceAliasDecls) {
+    if (!isDeclVisibleAtLocation(*Result.SourceManager, NamespaceAlias, DeclCtx,
+                                 Start))
+      continue;
+    StringRef FromDeclNameRef = FromDeclName;
+    if (FromDeclNameRef.consume_front(
+            NamespaceAlias->getNamespace()->getQualifiedNameAsString() +
+            "::")) {
+      std::string AliasName = NamespaceAlias->getNameAsString();
+      std::string AliasQualifiedName =
+          NamespaceAlias->getQualifiedNameAsString();
+      // We only consider namespace aliases define in the global namepspace or
+      // in namespaces that are directly visible from the reference, i.e.
+      // ancestor of the `OldNs`. Note that declarations in ancestor namespaces
+      // but not visible in the new namespace is filtered out by
+      // "IsVisibleInNewNs" matcher.
+      if (AliasQualifiedName != AliasName) {
+        // The alias is defined in some namespace.
+        assert(StringRef(AliasQualifiedName).endswith("::" + AliasName));
+        llvm::StringRef AliasNs =
+            StringRef(AliasQualifiedName).drop_back(AliasName.size() + 2);
+        if (!llvm::StringRef(OldNs).startswith(AliasNs))
+          continue;
+      }
+      std::string NameWithAliasNamespace =
+          (AliasName + "::" + FromDeclNameRef).str();
+      if (NameWithAliasNamespace.size() < ReplaceName.size())
+        ReplaceName = NameWithAliasNamespace;
+    }
+  }
   // Checks if there is any using shadow declarations that can shorten the
   // qualified name.
   bool Matched = false;
   for (const UsingDecl *Using : UsingDecls) {
     if (Matched)
       break;
     if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) {
       for (const auto *UsingShadow : Using->shadows()) {

clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

std::string Expected = "namespace glob {\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { Glob g; }\n"
                          "} // namespace y\n"
                          "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, NamespaceAliasInGlobal) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "namespace glob2 { class Glob2 {}; }\n"
+                     "namespace gl = glob;\n"
+                     "namespace gl2 = ::glob2;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() { gl::Glob g; gl2::Glob2 g2; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected =
+      "namespace glob {\n"
+      "class Glob {};\n"
+      "}\n"
+      "namespace glob2 { class Glob2 {}; }\n"
+      "namespace gl = glob;\n"
+      "namespace gl2 = ::glob2;\n"
+      "\n"
+      "namespace x {\n"
+      "namespace y {\n"
+      "void f() { gl::Glob g; gl2::Glob2 g2; }\n"
+      "} // namespace y\n"
+      "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInNamespace) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "namespace gl = glob;\n"
+                     "void f() { gl::Glob g; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "namespace gl = glob;\n"
+                         "void f() { gl::Glob g; }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInAncestorNamespace) {
+  NewNamespace = "na::nx";
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "namespace other { namespace gl = glob; }\n"
+                     "namespace na {\n"
+                     "namespace ga = glob;\n"
+                     "namespace nb {\n"
+                     "void f() { ga::Glob g; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "namespace other { namespace gl = glob; }\n"
+                         "namespace na {\n"
+                         "namespace ga = glob;\n"
+                         "\n"
+                         "namespace nx {\n"
+                         "void f() { ga::Glob g; }\n"
+                         "} // namespace nx\n"
+                         "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInOtherNamespace) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "namespace other { namespace gl = glob; }\n"
+                     "namespace na {\n"
+                     "namespace ga = glob;\n"
+                     "namespace nb {\n"
+                     "void f() { glob::Glob g; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "namespace other { namespace gl = glob; }\n"
+                         "namespace na {\n"
+                         "namespace ga = glob;\n"
+                         "\n"
+                         "} // namespace na\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void f() { glob::Glob g; }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, UsingDeclAfterReference) {
   std::string Code = "namespace glob {\n"
                      "class Glob {};\n"
                      "}\n"
                      "namespace na {\n"
                      "namespace nb {\n"
                      "void f() { glob::Glob g; }\n"
                      "} // namespace nb\n"