Handle adding new nested namespace in old namespace.

[ioeric]

• Added a test case with type references.

[hokein]

LGTM with two nits.

change-namespace/ChangeNamespace.cpp

475 ↗

(On Diff #77380)

missing a trailing ..

477 ↗

(On Diff #77380)

How about SourceLocation InsertionLoc = Start; so that you don't need else statement below?

[ioeric]

• Address comments.

[ioeric]

• Merged origin/master

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

SourceLocation EndLocationForType(TypeLoc TLoc) {
   if (TLoc.getTypeLocClass() == TypeLoc::TemplateSpecialization)
     return TLoc.castAs<TemplateSpecializationTypeLoc>()
         .getLAngleLoc()
         .getLocWithOffset(-1);
   return TLoc.getEndLoc();
 }
 
 // Returns the containing namespace of `InnerNs` by skipping `PartialNsName`.
-// If the `InnerNs` does not have `PartialNsName` as suffix, nullptr is
-// returned.
+// If the `InnerNs` does not have `PartialNsName` as suffix, or `PartialNsName`
+// is empty, nullptr is returned.
 // For example, if `InnerNs` is "a::b::c" and `PartialNsName` is "b::c", then
 // the NamespaceDecl of namespace "a" will be returned.
 const NamespaceDecl *getOuterNamespace(const NamespaceDecl *InnerNs,
                                        llvm::StringRef PartialNsName) {
+  if (!InnerNs || PartialNsName.empty())
+    return nullptr;
   const auto *CurrentContext = llvm::cast<DeclContext>(InnerNs);
   const auto *CurrentNs = InnerNs;
   llvm::SmallVector<llvm::StringRef, 4> PartialNsNameSplitted;
-  PartialNsName.split(PartialNsNameSplitted, "::");
+  PartialNsName.split(PartialNsNameSplitted, "::", /*MaxSplit=*/-1,
+                      /*KeepEmpty=*/false);
   while (!PartialNsNameSplitted.empty()) {
     // Get the inner-most namespace in CurrentContext.
     while (CurrentContext && !llvm::isa<NamespaceDecl>(CurrentContext))
       CurrentContext = CurrentContext->getParent();
     if (!CurrentContext)
       return nullptr;
     CurrentNs = llvm::cast<NamespaceDecl>(CurrentContext);
     if (PartialNsNameSplitted.back() != CurrentNs->getNameAsString())
void ChangeNamespaceTool::moveOldNamespace(
   MoveNs.Offset = R.getOffset();
   MoveNs.Length = R.getLength();
 
   // Insert the new namespace after `DiffOldNamespace`. For example, if
   // `OldNamespace` is "a::b::c" and `NewNamespace` is `a::x::y`, then
   // "x::y" will be inserted inside the existing namespace "a" and after "a::b".
   // `OuterNs` is the first namespace in `DiffOldNamespace`, e.g. "namespace b"
   // in the above example.
-  // FIXME: consider the case where DiffOldNamespace is empty.
+  // If there is no outer namespace (i.e. DiffOldNamespace is empty), the new
+  // namespace will be a nested namespace in the old namespace.
   const NamespaceDecl *OuterNs = getOuterNamespace(NsDecl, DiffOldNamespace);
+  SourceLocation InsertionLoc = Start;
+  if (OuterNs) {
     SourceLocation LocAfterNs =
         getStartOfNextLine(OuterNs->getRBraceLoc(), *Result.SourceManager,
                            Result.Context->getLangOpts());
     assert(LocAfterNs.isValid() &&
            "Failed to get location after DiffOldNamespace");
+    InsertionLoc = LocAfterNs;
+  }
   MoveNs.InsertionOffset = Result.SourceManager->getFileOffset(
-      Result.SourceManager->getSpellingLoc(LocAfterNs));
-
+      Result.SourceManager->getSpellingLoc(InsertionLoc));
   MoveNs.FID = Result.SourceManager->getFileID(Start);
   MoveNs.SourceMgr = Result.SourceManager;
   MoveNamespaces[R.getFilePath()].push_back(MoveNs);
 }
 
 // Removes a class forward declaration from the code in the moved namespace and
 // creates an `InsertForwardDeclaration` to insert the forward declaration back
 // into the old namespace after moving code from the old namespace to the new

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

std::string Expected = "\n\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "class A {};\n"
                          "} // namespace y\n"
                          "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, NewNsNestedInOldNs) {
+  NewNamespace = "na::nb::nc";
+  std::string Code = "namespace na {\n"
+                     "namespace nb {\n"
+                     "class A {};\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+                         "namespace nb {\n"
+                         "namespace nc {\n"
+                         "class A {};\n"
+                         "} // namespace nc\n"
+                         "\n"
+                         "} // namespace nb\n"
+                         "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NewNsNestedInOldNsWithRefs) {
+  NewNamespace = "na::nb::nc";
+  std::string Code = "namespace na {\n"
+                     "class A {};\n"
+                     "namespace nb {\n"
+                     "class B {};\n"
+                     "class C {};\n"
+                     "void f() { A a; B b; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+                         "class A {};\n"
+                         "namespace nb {\n"
+                         "namespace nc {\n"
+                         "class B {};\n"
+                         "class C {};\n"
+                         "void f() { A a; B b; }\n"
+                         "} // namespace nc\n"
+                         "\n"
+                         "} // namespace nb\n"
+                         "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, SimpleMoveIntoAnotherNestedNamespace) {
   NewNamespace = "na::nc";
   std::string Code = "namespace na {\n"
                      "namespace nb {\n"
                      "class A {};\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
   std::string Expected = "namespace na {\n"