[change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

[ioeric]

Summary

when replacing symbol references in moved namespaces, trying to make the replace
name as short as possible by considering UsingDecl (i.e. UsingShadow) and
UsingDirectiveDecl (i.e. using namespace decl).

[hokein]

change-namespace/ChangeNamespace.cpp

583

Yeah, it works for most cases, but it can not guarantee that they are still in the same DeclContext after changing to new namespace.

For example, the following case where an using-decl is used in the namespace being renamed, we change b::c to d::e, although DeclContext d of callexpr f() is a nested DeclContext of b (also DeclContext of using a::f), but this assumption is wrong after changing namespace because we keep using a::f in its original namespace.

namespace a { void f(); }

namespace b {
using a::f;
namespace c {
void d() { f(); }
}  // c
} // b

Not sure whether we should do this in our tool. I suspect there might be a lot of corner cases.

[ioeric]

change-namespace/ChangeNamespace.cpp

583

I thought using decls in old namespaces should be moved with along with namespaces. If this is the case, the moving of using decls is unexpected (looking into this), but this patch is handling this correctly if using decls are not moved.

[ioeric]

change-namespace/ChangeNamespace.cpp

583

Ahh, I was wrong. using a::f should not be moved. Hmm, I think we can solve or at least workaround this. But I still think we should support shortening namespace specifier based on using decls; it is quite not nice to add long specifiers if there are already using decls present.

[ioeric]

• Ignore using decls in old ns but not the inner-most old ns.

[ioeric]

Still missing one case... working on a fix.

[ioeric]

• Add a missing test case.

[ioeric]

change-namespace/ChangeNamespace.cpp

583

This should be fixed with the new matcher.

[ioeric]

ping.

[hokein]

change-namespace/ChangeNamespace.cpp

270

leading? looks like you are removing trailing ;

277

Ignoring using-decls in Prefix namespace-decl doesn't work perfectly. The same example:

namespace a { void f(); }

namespace b {
using a::f;
namespace c {
void d() { f(); }
} 
}

When changing b::c to b::e, the using a::f; will be excluded by this filter. As a result, a:: will be added to f().

583

OK, let's try to make it work perfectly.

[ioeric]

• Ignore using decls in old ns but not the inner-most old ns.
• Add a missing test case.
• Addressed comments

change-namespace/ChangeNamespace.cpp

277

Nice catch! Fixed and explained in the new code.

[ioeric]

• Get rid of redundant PrefixRef

[hokein]

change-namespace/ChangeNamespace.cpp

233

I will create two variables for SM.getSpellingLoc(D->getLocation()) and SM.getSpellingLoc(Loc) to avoid redundant function call.

570

Shouldn't we check whether the using namespace decl is visible here?

[ioeric]

• Addressed comments.

[hokein]

One more comment, otherwise looks good.

change-namespace/ChangeNamespace.cpp

275

Using an invalid name - is not an elegant solution to me. Is it possible to avoid it?
Maybe we can explicitly specify IsVisibleInNewNs using the code like:

Optional<ast_matchers::internal::Matcher<NamedDecl>> IsVisibleInNewNs = IsInMovedNs;
if (!DiffOldNsSplitted.empty() )  {
  std::string Prefix = ... 
  IsVisibleInNewNs = anyOf(*IsVisibleInNewNs, unless(hasAncestor(namespaceDecl(hasName(Prefix));
}

[ioeric]

change-namespace/ChangeNamespace.cpp

275

As per offline discussion, this seems to be impossible.

[hokein]

LGTM with one nit.

change-namespace/ChangeNamespace.cpp

275

OK, then add a comment explicitly specifying that "-" is used as an invalid name.

I think the code can be simplified as:

std::string Prefix = "-";
if (!DiffOldNsSplitted.empty()) {
  Prefix =  (StringRef(FullOldNs).drop_back(DiffOldNamespace.size()) + DiffOldNsSplitted.front()).str();
}

[ioeric]

• Addressed comment.

change-namespace/ChangeNamespace.h

void moveOldNamespace(const ast_matchers::MatchFinder::MatchResult &Result,
                         const NamespaceDecl *NsDecl);
 
   void moveClassForwardDeclaration(
       const ast_matchers::MatchFinder::MatchResult &Result,
       const CXXRecordDecl *FwdDecl);
 
   void replaceQualifiedSymbolInDeclContext(
       const ast_matchers::MatchFinder::MatchResult &Result,
-      const Decl *DeclContext, SourceLocation Start, SourceLocation End,
-      llvm::StringRef DeclName);
+      const DeclContext *DeclContext, SourceLocation Start, SourceLocation End,
+      const NamedDecl *FromDecl);
 
   void fixTypeLoc(const ast_matchers::MatchFinder::MatchResult &Result,
                   SourceLocation Start, SourceLocation End, TypeLoc Type);
 
   void fixUsingShadowDecl(const ast_matchers::MatchFinder::MatchResult &Result,
                           const UsingDecl *UsingDeclaration);
 
   // Information about moving an old namespace.
private:
   // in `onEndOfTranslationUnit`).
   std::map<std::string, std::vector<MoveNamespace>> MoveNamespaces;
   // Information about forward declaration insertions grouped by files.
   // A class forward declaration is not moved, so it will be deleted from the
   // moved code block and inserted back into the old namespace. The insertion
   // will be done after removing the code from the old namespace and before
   // 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;
 };
 
 } // namespace change_namespace
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CHANGE_NAMESPACE_CHANGENAMESPACE_H

change-namespace/ChangeNamespace.cpp

 // 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) {
   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();
+    return DeclName;
 
   while (!DeclName.consume_front((NsName + "::").str())) {
     const auto Pos = NsName.find_last_of(':');
     if (Pos == llvm::StringRef::npos)
       return DeclName;
     assert(Pos > 0);
     NsName = NsName.substr(0, Pos - 1);
   }
while (!NsSplitted.empty()) {
     Code = ("namespace " + NsSplitted.back() + " {\n" + Code +
             "} // namespace " + NsSplitted.back() + "\n")
                .str();
     NsSplitted.pop_back();
   }
   return Code;
 }
 
+// Returns true if \p D is a nested DeclContext in \p Context
+bool isNestedDeclContext(const DeclContext *D, const DeclContext *Context) {
+  while (D) {
+    if (D == Context)
+      return true;
+    D = D->getParent();
+  }
+  return false;
+}
+
+// Returns true if \p D is visible at \p Loc with DeclContext \p DeclCtx.
+bool isDeclVisibleAtLocation(const SourceManager &SM, const Decl *D,
+                             const DeclContext *DeclCtx, SourceLocation Loc) {
+  return SM.isBeforeInTranslationUnit(SM.getSpellingLoc(D->getLocation()),
+                                      SM.getSpellingLoc(Loc)) &&
+         (SM.getFileID(SM.getSpellingLoc(D->getLocation())) ==
+          SM.getFileID(SM.getSpellingLoc(Loc))) &&
+         isNestedDeclContext(DeclCtx, D->getDeclContext());
+}
+
 } // anonymous namespace
 
 ChangeNamespaceTool::ChangeNamespaceTool(
     llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern,
     std::map<std::string, tooling::Replacements> *FileToReplacements,
     llvm::StringRef FallbackStyle)
     : FallbackStyle(FallbackStyle), FileToReplacements(*FileToReplacements),
       OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')),
while (!OldNsSplitted.empty() && !NewNsSplitted.empty() &&
     OldNsSplitted.erase(OldNsSplitted.begin());
     NewNsSplitted.erase(NewNsSplitted.begin());
   }
   DiffOldNamespace = joinNamespaces(OldNsSplitted);
   DiffNewNamespace = joinNamespaces(NewNsSplitted);
 }
 
 void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  // Match old namespace blocks.
   std::string FullOldNs = "::" + OldNamespace;
+  // Prefix is the outer-most namespace in DiffOldNamespace. For example, if the
+  // OldNamespace is "a::b::c" and DiffOldNamespace is "b::c", then Prefix will
+  // be "a::b". Declarations in this namespace will not be visible in the new
+  // namespace.
+  llvm::SmallVector<llvm::StringRef, 4> DiffOldNsSplitted;
+  llvm::StringRef(DiffOldNamespace).split(DiffOldNsSplitted, "::");
+  std::string Prefix =
+      (StringRef(FullOldNs).drop_back(DiffOldNamespace.size()) +
+       (DiffOldNsSplitted.empty() ? "-" : DiffOldNsSplitted.front()))
+          .str();
+  StringRef PrefixRef = Prefix;
+  auto IsInMovedNs =
+      allOf(hasAncestor(namespaceDecl(hasName(FullOldNs)).bind("ns_decl")),
+            isExpansionInFileMatching(FilePattern));
+  auto IsVisibleInNewNs = anyOf(
+      IsInMovedNs, unless(hasAncestor(namespaceDecl(hasName(PrefixRef)))));
+  // Match using declarations.
+  Finder->addMatcher(
+      usingDecl(isExpansionInFileMatching(FilePattern), IsVisibleInNewNs)
+          .bind("using"),
+      this);
+  // Match using namespace declarations.
+  Finder->addMatcher(usingDirectiveDecl(isExpansionInFileMatching(FilePattern),
+                                        IsVisibleInNewNs)
+                         .bind("using_namespace"),
+                     this);
+
+  // Match old namespace blocks.
   Finder->addMatcher(
       namespaceDecl(hasName(FullOldNs), isExpansionInFileMatching(FilePattern))
           .bind("old_ns"),
       this);
 
-  auto IsInMovedNs =
-      allOf(hasAncestor(namespaceDecl(hasName(FullOldNs)).bind("ns_decl")),
-            isExpansionInFileMatching(FilePattern));
-
   // Match forward-declarations in the old namespace.
   Finder->addMatcher(
       cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition())), IsInMovedNs)
           .bind("fwd_decl"),
       this);
 
   // Match references to types that are not defined in the old namespace.
   // Forward-declarations in the old namespace are also matched since they will
Finder->addMatcher(
                                      unless(templateSpecializationType())))))),
                            hasParent(nestedNameSpecifierLoc()))),
               hasAncestor(decl().bind("dc")))
           .bind("type"),
       this);
 
   // Types in `UsingShadowDecl` is not matched by `typeLoc` above, so we need to
   // special case it.
-  Finder->addMatcher(
-      usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl())).bind("using_decl"),
+  Finder->addMatcher(usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl()))
+                         .bind("using_with_shadow"),
                      this);
 
   // Handle types in nested name specifier.
   Finder->addMatcher(nestedNameSpecifierLoc(
                          hasAncestor(decl(IsInMovedNs).bind("dc")),
                          loc(nestedNameSpecifier(specifiesType(
                              hasDeclaration(DeclMatcher.bind("from_decl"))))))
                          .bind("nested_specifier_loc"),
                      this);
void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   Finder->addMatcher(declRefExpr(IsInMovedNs, hasAncestor(decl().bind("dc")),
                                  to(GlobalVarMatcher.bind("var_decl")))
                          .bind("var_ref"),
                      this);
 }
 
 void ChangeNamespaceTool::run(
     const ast_matchers::MatchFinder::MatchResult &Result) {
-  if (const auto *NsDecl = Result.Nodes.getNodeAs<NamespaceDecl>("old_ns")) {
+  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 *NsDecl =
+                 Result.Nodes.getNodeAs<NamespaceDecl>("old_ns")) {
     moveOldNamespace(Result, NsDecl);
   } else if (const auto *FwdDecl =
                  Result.Nodes.getNodeAs<CXXRecordDecl>("fwd_decl")) {
     moveClassForwardDeclaration(Result, FwdDecl);
-  } else if (const auto *UsingDeclaration =
-                 Result.Nodes.getNodeAs<UsingDecl>("using_decl")) {
-    fixUsingShadowDecl(Result, UsingDeclaration);
+  } else if (const auto *UsingWithShadow =
+                 Result.Nodes.getNodeAs<UsingDecl>("using_with_shadow")) {
+    fixUsingShadowDecl(Result, UsingWithShadow);
   } else if (const auto *Specifier =
                  Result.Nodes.getNodeAs<NestedNameSpecifierLoc>(
                      "nested_specifier_loc")) {
     SourceLocation Start = Specifier->getBeginLoc();
     SourceLocation End = EndLocationForType(Specifier->getTypeLoc());
     fixTypeLoc(Result, Start, End, Specifier->getTypeLoc());
   } else if (const auto *TLoc = Result.Nodes.getNodeAs<TypeLoc>("type")) {
     fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc),
                *TLoc);
-  } else if (const auto *VarRef = Result.Nodes.getNodeAs<DeclRefExpr>("var_ref")){
+  } else if (const auto *VarRef =
+                 Result.Nodes.getNodeAs<DeclRefExpr>("var_ref")) {
     const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var_decl");
     assert(Var);
     if (Var->getCanonicalDecl()->isStaticDataMember())
       return;
-    std::string Name = Var->getQualifiedNameAsString();
     const clang::Decl *Context = Result.Nodes.getNodeAs<clang::Decl>("dc");
     assert(Context && "Empty decl context.");
     clang::SourceRange VarRefRange = VarRef->getSourceRange();
-    replaceQualifiedSymbolInDeclContext(Result, Context, VarRefRange.getBegin(),
-                                        VarRefRange.getEnd(), Name);
+    replaceQualifiedSymbolInDeclContext(
+        Result, Context->getDeclContext(), VarRefRange.getBegin(),
+        VarRefRange.getEnd(), llvm::dyn_cast<NamedDecl>(Var));
   } else {
     const auto *Call = Result.Nodes.getNodeAs<clang::CallExpr>("call");
     assert(Call != nullptr &&"Expecting callback for CallExpr.");
     const clang::FunctionDecl* Func = Call->getDirectCallee();
     assert(Func != nullptr);
     // Ignore out-of-line static methods since they will be handled by nested
     // name specifiers.
     if (Func->getCanonicalDecl()->getStorageClass() ==
             clang::StorageClass::SC_Static &&
         Func->isOutOfLine())
       return;
-    std::string Name = Func->getQualifiedNameAsString();
     const clang::Decl *Context = Result.Nodes.getNodeAs<clang::Decl>("dc");
     assert(Context && "Empty decl context.");
     clang::SourceRange CalleeRange = Call->getCallee()->getSourceRange();
-    replaceQualifiedSymbolInDeclContext(Result, Context, CalleeRange.getBegin(),
-                                        CalleeRange.getEnd(), Name);
+    replaceQualifiedSymbolInDeclContext(
+        Result, Context->getDeclContext(), CalleeRange.getBegin(),
+        CalleeRange.getEnd(), llvm::dyn_cast<NamedDecl>(Func));
   }
 }
 
 static SourceLocation getLocAfterNamespaceLBrace(const NamespaceDecl *NsDecl,
                                                  const SourceManager &SM,
                                                  const LangOptions &LangOpts) {
   std::unique_ptr<Lexer> Lex =
       getLexerStartingFromLoc(NsDecl->getLocStart(), SM, LangOpts);
void ChangeNamespaceTool::moveClassForwardDeclaration(
   const auto Insertion = createInsertion(NsDecl->decls_begin()->getLocStart(),
                                          Code, *Result.SourceManager);
   InsertForwardDeclaration InsertFwd;
   InsertFwd.InsertionOffset = Insertion.getOffset();
   InsertFwd.ForwardDeclText = Insertion.getReplacementText().str();
   InsertFwdDecls[Insertion.getFilePath()].push_back(InsertFwd);
 }
 
-// Replaces a qualified symbol that refers to a declaration `DeclName` with the
-// shortest qualified name possible when the reference is in `NewNamespace`.
-// FIXME: don't need to add redundant namespace qualifier when there is
-// UsingShadowDecl or using namespace decl.
+// Replaces a qualified symbol (in \p DeclCtx) that refers to a declaration \p
+// FromDecl with the shortest qualified name possible when the reference is in
+// `NewNamespace`.
 void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext(
-    const ast_matchers::MatchFinder::MatchResult &Result, const Decl *DeclCtx,
-    SourceLocation Start, SourceLocation End, llvm::StringRef DeclName) {
-  const auto *NsDeclContext =
-      DeclCtx->getDeclContext()->getEnclosingNamespaceContext();
+    const ast_matchers::MatchFinder::MatchResult &Result,
+    const DeclContext *DeclCtx, SourceLocation Start, SourceLocation End,
+    const NamedDecl *FromDecl) {
+  const auto *NsDeclContext = DeclCtx->getEnclosingNamespaceContext();
   const auto *NsDecl = llvm::dyn_cast<NamespaceDecl>(NsDeclContext);
   // Calculate the name of the `NsDecl` after it is moved to new namespace.
   std::string OldNs = NsDecl->getQualifiedNameAsString();
   llvm::StringRef Postfix = OldNs;
   bool Consumed = Postfix.consume_front(OldNamespace);
   assert(Consumed && "Expect OldNS to start with OldNamespace.");
   (void)Consumed;
   const std::string NewNs = (NewNamespace + Postfix).str();
 
   llvm::StringRef NestedName = Lexer::getSourceText(
       CharSourceRange::getTokenRange(
           Result.SourceManager->getSpellingLoc(Start),
           Result.SourceManager->getSpellingLoc(End)),
       *Result.SourceManager, Result.Context->getLangOpts());
   // If the symbol is already fully qualified, no change needs to be make.
   if (NestedName.startswith("::"))
     return;
+  std::string FromDeclName = FromDecl->getQualifiedNameAsString();
   std::string ReplaceName =
-      getShortestQualifiedNameInNamespace(DeclName, NewNs);
+      getShortestQualifiedNameInNamespace(FromDeclName, NewNs);
+  // Checks if there is any using namespace declarations that can shorten the
+  // qualified name.
+  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 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()) {
+        const auto *TargetDecl = UsingShadow->getTargetDecl();
+        if (TargetDecl == FromDecl) {
+          ReplaceName = FromDecl->getNameAsString();
+          Matched = true;
+          break;
+        }
+      }
+    }
+  }
   // If the new nested name in the new namespace is the same as it was in the
   // old namespace, we don't create replacement.
   if (NestedName == ReplaceName)
     return;
   auto R = createReplacement(Start, End, ReplaceName, *Result.SourceManager);
   addOrMergeReplacement(R, &FileToReplacements[R.getFilePath()]);
 }
 
void ChangeNamespaceTool::fixTypeLoc(
   const auto *FromDecl = Result.Nodes.getNodeAs<NamedDecl>("from_decl");
   // `hasDeclaration` gives underlying declaration, but if the type is
   // a typedef type, we need to use the typedef type instead.
   if (auto *Typedef = Type.getType()->getAs<TypedefType>())
     FromDecl = Typedef->getDecl();
 
   const Decl *DeclCtx = Result.Nodes.getNodeAs<Decl>("dc");
   assert(DeclCtx && "Empty decl context.");
-  replaceQualifiedSymbolInDeclContext(Result, DeclCtx, Start, End,
-                                      FromDecl->getQualifiedNameAsString());
+  replaceQualifiedSymbolInDeclContext(Result, DeclCtx->getDeclContext(), Start,
+                                      End, FromDecl);
 }
 
 void ChangeNamespaceTool::fixUsingShadowDecl(
     const ast_matchers::MatchFinder::MatchResult &Result,
     const UsingDecl *UsingDeclaration) {
   SourceLocation Start = UsingDeclaration->getLocStart();
   SourceLocation End = UsingDeclaration->getLocEnd();
   if (Start.isInvalid() || End.isInvalid()) return;

unittests/change-namespace/ChangeNamespaceTests.cpp

std::string Code = "class GLOB {};\n"
                      "namespace nc {\n"
                      "class SAME {};\n"
                      "}\n"
                      "namespace nd {\n"
                      "class SAME {};\n"
                      "}\n"
                      "namespace nb {\n"
                      "using nc::SAME;\n"
-                     "using YO = nc::SAME;\n"
-                     "typedef nc::SAME IDENTICAL;\n"
+                     "using YO = nd::SAME;\n"
+                     "typedef nd::SAME IDENTICAL;\n"
                      "void f(nd::SAME Same) {}\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
 
   std::string Expected = "class GLOB {};\n"
                          "using BLOG = GLOB;\n"
                          "namespace na {\n"
                          "namespace nc {\n"
                          "class SAME {};\n"
                          "}\n"
                          "namespace nd {\n"
                          "class SAME {};\n"
                          "}\n"
                          "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "using ::na::nc::SAME;\n"
-                         "using YO = na::nc::SAME;\n"
-                         "typedef na::nc::SAME IDENTICAL;\n"
+                         "using YO = na::nd::SAME;\n"
+                         "typedef na::nd::SAME IDENTICAL;\n"
                          "void f(na::nd::SAME Same) {}\n"
                          "} // namespace y\n"
                          "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
-TEST_F(ChangeNamespaceTest, UsingShadowDeclInFunction) {
-  std::string Code = "namespace glob {\n"
-                     "class Glob {};\n"
-                     "}\n"
-                     "namespace na {\n"
-                     "namespace nb {\n"
-                     "void f() {\n"
-                     "  using glob::Glob;\n"
-                     "  Glob g;\n"
-                     "}\n"
-                     "} // namespace nb\n"
-                     "} // namespace na\n";
-
-  // FIXME: don't add namespace qualifier when there is UsingShadowDecl.
-  std::string Expected = "namespace glob {\n"
-                         "class Glob {};\n"
-                         "}\n"
-                         "\n"
-                         "namespace x {\n"
-                         "namespace y {\n"
-                         "void f() {\n"
-                         "  using ::glob::Glob;\n"
-                         "  glob::Glob g;\n"
-                         "}\n"
-                         "} // namespace y\n"
-                         "} // namespace x\n";
-  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
-}
-
-TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
-  std::string Code = "namespace glob {\n"
-                     "class Glob {};\n"
-                     "}\n"
-                     "using glob::Glob;\n"
-                     "namespace na {\n"
-                     "namespace nb {\n"
-                     "void f() { Glob g; }\n"
-                     "} // namespace nb\n"
-                     "} // namespace na\n";
-
-  // FIXME: don't add namespace qualifier when there is UsingShadowDecl.
-  std::string Expected = "namespace glob {\n"
-                         "class Glob {};\n"
-                         "}\n"
-                         "using glob::Glob;\n"
-                         "\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, UsingNamespace) {
-  std::string Code = "namespace glob {\n"
-                     "class Glob {};\n"
-                     "}\n"
-                     "using namespace glob;\n"
-                     "namespace na {\n"
-                     "namespace nb {\n"
-                     "void f() { Glob g; }\n"
-                     "} // namespace nb\n"
-                     "} // namespace na\n";
-
-  // FIXME: don't add namespace qualifier when there is "using namespace" decl.
-  std::string Expected = "namespace glob {\n"
-                         "class Glob {};\n"
-                         "}\n"
-                         "using namespace glob;\n"
-                         "\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, TypeInNestedNameSpecifier) {
   std::string Code =
       "namespace na {\n"
       "class C_A {\n"
       "public:\n"
       "  class Nested {\n"
       "    public:\n"
       "      static int NestedX;\n"
std::string Expected = "\n"
                          "  // Private comments.\n"
                          "  int a;\n"
                          "};\n"
                          "} // namespace y\n"
                          "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "using glob::Glob;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() { Glob g; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "using glob::Glob;\n"
+                         "\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, UsingNamespaceInGlobal) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "using namespace glob;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() { Glob g; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "using namespace glob;\n"
+                         "\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, 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"
+                     "} // namespace na\n"
+                     "using glob::Glob;\n"
+                     "using namespace glob;\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void f() { glob::Glob g; }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n"
+                         "using glob::Glob;\n"
+                         "using namespace glob;\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingNamespaceAndUsingShadowInGlobal) {
+  std::string Code = "namespace glob1 {\n"
+                     "namespace glob2 {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "}\n"
+                     "using glob1::glob2::Glob;\n"
+                     "using namespace glob1;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() { Glob g; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob1 {\n"
+                         "namespace glob2 {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "}\n"
+                         "using glob1::glob2::Glob;\n"
+                         "using namespace glob1;\n"
+                         "\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, UsingAliasInGlobal) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "using GLB = glob::Glob;\n"
+                     "using BLG = glob::Glob;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() { GLB g; BLG blg; }\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "using GLB = glob::Glob;\n"
+                         "using BLG = glob::Glob;\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void f() { GLB g; BLG blg; }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclAndMovedNamespace) {
+  std::string Code = "namespace na { class C_A {};\n }\n"
+                     "using na::C_A;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "class C_X {\n"
+                     "public:\n"
+                     "  C_A a;\n"
+                     "};\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na { class C_A {};\n }\n"
+                         "using na::C_A;\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "class C_X {\n"
+                         "public:\n"
+                         "  C_A a;\n"
+                         "};\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingNamespaceDeclAndMovedNamespace) {
+  std::string Code = "namespace na { class C_A {};\n }\n"
+                     "using namespace na;\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "class C_X {\n"
+                     "public:\n"
+                     "  C_A ca;\n"
+                     "};\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na { class C_A {};\n }\n"
+                         "using namespace na;\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "class C_X {\n"
+                         "public:\n"
+                         "  C_A ca;\n"
+                         "};\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInFunction) {
+  std::string Code = "namespace glob {\n"
+                     "class Glob {};\n"
+                     "}\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() {\n"
+                     "  using glob::Glob;\n"
+                     "  Glob g;\n"
+                     "}\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+                         "class Glob {};\n"
+                         "}\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void f() {\n"
+                         "  using ::glob::Glob;\n"
+                         "  Glob g;\n"
+                         "}\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInClass) {
+  std::string Code = "namespace na { class C_A {};\n }\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "void f() {\n"
+                     "  using na::CA;\n"
+                     "  CA ca;\n"
+                     "}\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na { class C_A {};\n }\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void f() {\n"
+                         "  using na::CA;\n"
+                         "  CA ca;\n"
+                         "}\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingDeclInMovedNamespace) {
+  std::string Code = "namespace nx { void f(); }\n"
+                     "namespace na {\n"
+                     "using nx::f;\n"
+                     "namespace nb {\n"
+                     "void d() { f(); }\n"
+                     "} // nb\n"
+                     "} // na\n";
+
+  std::string Expected = "namespace nx { void f(); }\n"
+                         "namespace na {\n"
+                         "using nx::f;\n"
+                         "\n"
+                         "} // na\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void d() { nx::f(); }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingDeclInMovedNamespaceNotNested) {
+  OldNamespace = "na";
+  std::string Code = "namespace nx { void f(); }\n"
+                     "namespace na {\n"
+                     "using ::nx::f;\n"
+                     "void d() { f(); }\n"
+                     "} // na\n";
+
+  std::string Expected = "namespace nx { void f(); }\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "using ::nx::f;\n"
+                         "void d() { f(); }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingDeclInMovedNamespaceMultiNested) {
+  OldNamespace = "a::b::c::d";
+  NewNamespace = "a::b::x::y";
+  std::string Code = "namespace nx { void f(); void g(); }\n"
+                     "namespace a {\n"
+                     "namespace b {\n"
+                     "using ::nx::f;\n"
+                     "namespace c {\n"
+                     "using ::nx::g;\n"
+                     "namespace d {\n"
+                     "void d() { f(); g(); }\n"
+                     "} // d\n"
+                     "} // c\n"
+                     "} // b\n"
+                     "} // a\n";
+
+  std::string Expected = "namespace nx { void f(); void g(); }\n"
+                         "namespace a {\n"
+                         "namespace b {\n"
+                         "using ::nx::f;\n"
+                         "namespace c {\n"
+                         "using ::nx::g;\n"
+                         "\n"
+                         "} // c\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "void d() { f(); nx::g(); }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n"
+                         "} // b\n"
+                         "} // a\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingDeclInTheParentOfOldNamespace) {
+  OldNamespace = "nb::nc";
+  NewNamespace = "nb::nd";
+  std::string Code = "namespace na { class A {}; }\n"
+                     "namespace nb {\n"
+                     "using na::A;\n"
+                     "namespace nc {\n"
+                     "void d() { A a; }\n"
+                     "} // nc\n"
+                     "} // nb\n";
+
+  std::string Expected = "namespace na { class A {}; }\n"
+                         "namespace nb {\n"
+                         "using na::A;\n"
+                         "\n"
+                         "namespace nd {\n"
+                         "void d() { A a; }\n"
+                         "} // namespace nd\n"
+                         "} // nb\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 } // anonymous namespace
 } // namespace change_namespace
 } // namespace clang