[change-namespace] Fixed a bug in getShortestQualifiedNameInNamespace.

[ioeric]

• Remove redundant variable.

[ioeric]

Ping

[hokein]

change-namespace/ChangeNamespace.cpp

176 ↗

(On Diff #73116)

Mightbe add a small doc saying NsName is a global namespace if it is empty. (If I misunderstand the code).

187 ↗

(On Diff #73116)

I think the statement doesn't compile here, since consume_front return a bool. It should be if (DeclName.consume_front((NsName + "::")))?

Looks like we can also put this judge into the above while statement?

[ioeric]

• Address review comments.

change-namespace/ChangeNamespace.cpp

187 ↗

(On Diff #73116)

Note that str() is called on (NsName + "::") instead of consume_front. But you are right, we can put the check into the while loop.

[hokein]

LGTM.

change-namespace/ChangeNamespace.cpp

187 ↗

(On Diff #73116)

Oh, I see. Sorry for misreading the code.

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

tooling::Replacement createInsertion(SourceLocation Loc,
   }
   Loc = SM.getSpellingLoc(Loc);
   return tooling::Replacement(SM, Loc, 0, InsertText);
 }
 
 // Returns the shortest qualified name for declaration `DeclName` in the
 // namespace `NsName`. For example, if `DeclName` is "a::b::X" and `NsName`
 // is "a::c::d", then "b::X" will be returned.
+// \param DeclName A fully qualified name, "::a::b::X" or "a::b::X".
+// \param NsName A fully qualified name, "::a::b" or "a::b". Global namespace
+//        will have empty name.
 std::string getShortestQualifiedNameInNamespace(llvm::StringRef DeclName,
                                                 llvm::StringRef NsName) {
-  llvm::SmallVector<llvm::StringRef, 4> DeclNameSplitted;
-  DeclName.split(DeclNameSplitted, "::");
-  if (DeclNameSplitted.size() == 1)
-    return DeclName;
-  const auto UnqualifiedName = DeclNameSplitted.back();
-  while (true) {
+  DeclName = DeclName.ltrim(':');
+  NsName = NsName.ltrim(':');
+  // If `DeclName` is a global variable, we prepend "::" to it if it is not in
+  // the global namespace.
+  if (DeclName.find(':') == llvm::StringRef::npos)
+    return NsName.empty() ? DeclName.str() : ("::" + DeclName).str();
+
+  while (!DeclName.consume_front((NsName + "::").str())) {
     const auto Pos = NsName.find_last_of(':');
     if (Pos == llvm::StringRef::npos)
       return DeclName;
-    const auto Prefix = NsName.substr(0, Pos - 1);
-    if (DeclName.startswith(Prefix))
-      return (Prefix + "::" + UnqualifiedName).str();
-    NsName = Prefix;
+    assert(Pos > 0);
+    NsName = NsName.substr(0, Pos - 1);
   }
   return DeclName;
 }
 
 std::string wrapCodeInNamespace(StringRef NestedNs, std::string Code) {
   if (Code.back() != '\n')
     Code += "\n";
   llvm::SmallVector<StringRef, 4> NsSplitted;

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

std::string Expected = "namespace na {\n"
                          "\n"
                          "namespace nc {\n"
                          "class A {};\n"
                          "} // namespace nc\n"
                          "} // namespace na\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, MoveIntoAnotherNestedNamespaceWithRef) {
+  NewNamespace = "na::nc";
+  std::string Code = "namespace na {\n"
+                     "class A {};\n"
+                     "namespace nb {\n"
+                     "class X { A a; };\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+                         "class A {};\n"
+                         "\n"
+                         "namespace nc {\n"
+                         "class X { A a; };\n"
+                         "} // namespace nc\n"
+                         "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, SimpleMoveNestedNamespace) {
   NewNamespace = "na::x::y";
   std::string Code = "namespace na {\n"
                      "class A {};\n"
                      "namespace nb {\n"
                      "class B {};\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";