[change-namespace] Don't match a function call/ref multiple times.

Summary:
Previously, the matcher matches a function call/ref multiple times, one
for each decl ancestor. This might cause problems. For example, in the following
case, `func()` would be matched once (with namespace context) before using decl is
seen and once after using decl is seeing, which would result in different conflicting
replacements as the first match would replace `func` with "ns::func" as it doesn't
know about the using decl.

```
namespace x {
namespace {
using ::ns::func;
void f() { func(); }
}
}
```

Switching from `hasDescendant` matching to `hasAncestor` matching solves the
problem.

Reviewers: hokein

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D44517

llvm-svn: 327629

2 files changed, 45 insertions, 9 deletions

diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.cpp b/clang-tools-extra/change-namespace/ChangeNamespace.cpp
index 68adada9948..35c321aed79 100644
--- a/clang-tools-extra/change-namespace/ChangeNamespace.cpp
+++ b/clang-tools-extra/change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@ void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
                                 hasAncestor(namespaceDecl(isAnonymous())),
                                 hasAncestor(cxxRecordDecl()))),
                    hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-                              callExpr(callee(FuncMatcher)).bind("call"),
-                              declRefExpr(to(FuncMatcher.bind("func_decl")))
-                                  .bind("func_ref")))),
-                          IsInMovedNs, unless(isImplicit()))
-                         .bind("dc"),
-                     this);
+  Finder->addMatcher(
+      expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+                 unless(hasAncestor(isImplicit())),
+                 anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+                       declRefExpr(to(FuncMatcher.bind("func_decl")))
+                           .bind("func_ref")))),
+      this);
 
   auto GlobalVarMatcher = varDecl(
       hasGlobalStorage(), hasParent(namespaceDecl()),
diff --git a/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp b/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp
index 440a3fe0b0f..917f7b02e97 100644
--- a/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ b/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@ TEST_F(ChangeNamespaceTest, CommentsBeforeMovedClass) {
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
                      "class Glob {};\n"
+                     "void GFunc() {}\n"
                      "}\n"
                      "using glob::Glob;\n"
+                     "using glob::GFunc;\n"
                      "namespace na {\n"
                      "namespace nb {\n"
-                     "void f() { Glob g; }\n"
+                     "void f() { Glob g; GFunc(); }\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
 
   std::string Expected = "namespace glob {\n"
                          "class Glob {};\n"
+                         "void GFunc() {}\n"
                          "}\n"
                          "using glob::Glob;\n"
+                         "using glob::GFunc;\n"
                          "\n"
                          "namespace x {\n"
                          "namespace y {\n"
-                         "void f() { Glob g; }\n"
+                         "void f() { Glob g; GFunc(); }\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+  std::string Code = "namespace util {\n"
+                     "class Util {};\n"
+                     "void func() {}\n"
+                     "}\n"
+                     "namespace na {\n"
+                     "namespace nb {\n"
+                     "namespace {\n"
+                     "using ::util::Util;\n"
+                     "using ::util::func;\n"
+                     "void f() { Util u; func(); }\n"
+                     "}\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace util {\n"
+                         "class Util {};\n"
+                         "void func() {}\n"
+                         "} // namespace util\n"
+                         "\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "namespace {\n"
+                         "using ::util::Util;\n"
+                         "using ::util::func;\n"
+                         "void f() { Util u; func(); }\n"
+                         "}\n"
                          "} // namespace y\n"
                          "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));