[change-namespace] Enhance detection of conflicting namespaces.

[ioeric]

Summary

For example:

namespace util { class Base; }

namespace new {
namespace util { class Internal; }
}

namespace old {
util::Base b1;
}

When changing old:: to new::, util:: in namespace "new::" will conflict
with "new::util::" unless a leading "::" is added.

[hokein]

LGTM

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

 //===-- ChangeNamespace.cpp - Change namespace implementation -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include "ChangeNamespace.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace change_namespace {
 
 namespace {
bool isDeclVisibleAtLocation(const SourceManager &SM, const Decl *D,
   SourceLocation DeclLoc = SM.getSpellingLoc(D->getBeginLoc());
   Loc = SM.getSpellingLoc(Loc);
   return SM.isBeforeInTranslationUnit(DeclLoc, Loc) &&
          (SM.getFileID(DeclLoc) == SM.getFileID(Loc) &&
           isNestedDeclContext(DeclCtx, D->getDeclContext()));
 }
 
 // Given a qualified symbol name, returns true if the symbol will be
-// incorrectly qualified without leading "::".
-bool conflictInNamespace(llvm::StringRef QualifiedSymbol,
+// incorrectly qualified without leading "::". For example, a symbol
+// "nx::ny::Foo" in namespace "na::nx::ny" without leading "::"; a symbol
+// "util::X" in namespace "na" can potentially conflict with "na::util" (if this
+// exists).
+bool conflictInNamespace(const ASTContext &AST, llvm::StringRef QualifiedSymbol,
                          llvm::StringRef Namespace) {
   auto SymbolSplitted = splitSymbolName(QualifiedSymbol.trim(":"));
   assert(!SymbolSplitted.empty());
   SymbolSplitted.pop_back();  // We are only interested in namespaces.
 
-  if (SymbolSplitted.size() > 1 && !Namespace.empty()) {
+  if (SymbolSplitted.size() >= 1 && !Namespace.empty()) {
+    auto SymbolTopNs = SymbolSplitted.front();
     auto NsSplitted = splitSymbolName(Namespace.trim(":"));
     assert(!NsSplitted.empty());
-    // We do not check the outermost namespace since it would not be a conflict
-    // if it equals to the symbol's outermost namespace and the symbol name
-    // would have been shortened.
+
+    auto LookupDecl = [&AST](const Decl &Scope,
+                             llvm::StringRef Name) -> const NamedDecl * {
+      const auto *DC = llvm::dyn_cast<DeclContext>(&Scope);
+      if (!DC)
+        return nullptr;
+      auto LookupRes = DC->lookup(DeclarationName(&AST.Idents.get(Name)));
+      if (LookupRes.empty())
+        return nullptr;
+      return LookupRes.front();
+    };
+    // We do not check the outermost namespace since it would not be a
+    // conflict if it equals to the symbol's outermost namespace and the
+    // symbol name would have been shortened.
+    const NamedDecl *Scope =
+        LookupDecl(*AST.getTranslationUnitDecl(), NsSplitted.front());
     for (auto I = NsSplitted.begin() + 1, E = NsSplitted.end(); I != E; ++I) {
-      if (*I == SymbolSplitted.front())
+      if (*I == SymbolTopNs) // Handles "::ny" in "::nx::ny" case.
         return true;
+      // Handles "::util" and "::nx::util" conflicts.
+      if (Scope) {
+        if (LookupDecl(*Scope, SymbolTopNs))
+          return true;
+        Scope = LookupDecl(*Scope, *I);
+      }
     }
+    if (Scope && LookupDecl(*Scope, SymbolTopNs))
+      return true;
   }
   return false;
 }
 
 AST_MATCHER(EnumDecl, isScoped) {
     return Node.isScoped();
 }
 
if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) {
             FromDecl->getQualifiedNameAsString()) {
           ReplaceName = FromDecl->getNameAsString();
           Matched = true;
           break;
         }
       }
     }
   }
+  bool Conflict = conflictInNamespace(DeclCtx->getParentASTContext(),
+                                      ReplaceName, NewNamespace);
   // 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 ||
+  // old namespace, we don't create replacement unless there can be ambiguity.
+  if ((NestedName == ReplaceName && !Conflict) ||
       (NestedName.startswith("::") && NestedName.drop_front(2) == ReplaceName))
     return;
   // If the reference need to be fully-qualified, add a leading "::" unless
   // NewNamespace is the global namespace.
-  if (ReplaceName == FromDeclName && !NewNamespace.empty() &&
-      conflictInNamespace(ReplaceName, NewNamespace))
+  if (ReplaceName == FromDeclName && !NewNamespace.empty() && Conflict)
     ReplaceName = "::" + ReplaceName;
   addReplacementOrDie(Start, End, ReplaceName, *Result.SourceManager,
                       &FileToReplacements);
 }
 
 // Replace the [Start, End] of `Type` with the shortest qualified name when the
 // `Type` is in `NewNamespace`.
 void ChangeNamespaceTool::fixTypeLoc(

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

std::string Expected = "namespace a{\n"
                          "  D d;\n"
                          "  a::Base<D> b;\n"
                          "  b.f();\n"
                          "}\n"
                          "}  // namespace e\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, FullyQualifyConflictNamespace) {
+  std::string Code =
+      "namespace x { namespace util { class Some {}; } }\n"
+      "namespace x { namespace y {namespace base { class Base {}; } } }\n"
+      "namespace util { class Status {}; }\n"
+      "namespace base { class Base {}; }\n"
+      "namespace na {\n"
+      "namespace nb {\n"
+      "void f() {\n"
+      "  util::Status s1; x::util::Some s2;\n"
+      "  base::Base b1; x::y::base::Base b2;\n"
+      "}\n"
+      "} // namespace nb\n"
+      "} // namespace na\n";
+
+  std::string Expected =
+      "namespace x { namespace util { class Some {}; } }\n"
+      "namespace x { namespace y {namespace base { class Base {}; } } }\n"
+      "namespace util { class Status {}; }\n"
+      "namespace base { class Base {}; }\n"
+      "\n"
+      "namespace x {\n"
+      "namespace y {\n"
+      "void f() {\n"
+      "  ::util::Status s1; util::Some s2;\n"
+      "  ::base::Base b1; base::Base b2;\n"
+      "}\n"
+      "} // namespace y\n"
+      "} // namespace x\n";
+
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 } // anonymous namespace
 } // namespace change_namespace
 } // namespace clang