[clang-rename] Add function unit tests.

[hokein]

Summary

Also contain a fix:

• Fix a false positive of renaming a using shadow function declaration.

[ioeric]

Lg

[hokein]

Update, and add one unfixed testcase.

[hokein]

Sorry, I thought I have run all the tests. The previous version broke one test in RenameClass. We still need the workaround for the std::function<void(T)> special case. I added it back, and also added a corresponding test case for renaming function, but it is currently not supported and disabled, I think it is fine now, as it is a rare case IMO.

Please take a look again.

[ioeric]

Still LGTM

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

462

The current behavior looks fine to me, but I'm not sure if this is a right FIXME? Basically, I think we want to be able to get the real DeclContext for the symbol reference here right?

[hokein]

Remove the FIXME.

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

462

I'm fine on removing it, as this is somewhat mentioned above. Done.

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

if (isInUSRSet(Decl)) {
                          /*IgnorePrefixQualifers=*/true};
       RenameInfos.push_back(Info);
     }
     return true;
   }
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
     const NamedDecl *Decl = Expr->getFoundDecl();
+    // Get the underlying declaration of the shadow declaration introduced by a
+    // using declaration.
+    if (auto* UsingShadow = llvm::dyn_cast<UsingShadowDecl>(Decl)) {
+      Decl = UsingShadow->getTargetDecl();
+    }
+
     if (isInUSRSet(Decl)) {
       RenameInfo Info = {Expr->getSourceRange().getBegin(),
                          Expr->getSourceRange().getEnd(),
                          Decl,
                          getClosestAncestorDecl(*Expr),
                          Expr->getQualifier(),
                          /*IgnorePrefixQualifers=*/false};
       RenameInfos.push_back(Info);
for (const auto &RenameInfo : Finder.getRenameInfos()) {
     std::string ReplacedName = NewName.str();
     if (RenameInfo.IgnorePrefixQualifers) {
       // Get the name without prefix qualifiers from NewName.
       size_t LastColonPos = NewName.find_last_of(':');
       if (LastColonPos != std::string::npos)
         ReplacedName = NewName.substr(LastColonPos + 1);
     } else {
       if (RenameInfo.FromDecl && RenameInfo.Context) {
-        if (!llvm::isa<clang::TranslationUnitDecl>(
-                RenameInfo.Context->getDeclContext())) {
-          ReplacedName = tooling::replaceNestedName(
-              RenameInfo.Specifier, RenameInfo.Context->getDeclContext(),
-              RenameInfo.FromDecl,
-              NewName.startswith("::") ? NewName.str()
-                                       : ("::" + NewName).str());
-        }
-      }
       // If the NewName contains leading "::", add it back.
       if (NewName.startswith("::") && NewName.substr(2) == ReplacedName)
         ReplacedName = NewName.str();
     }
     Replace(RenameInfo.Begin, RenameInfo.End, ReplacedName);
   }
 
   // Hanlde using declarations explicitly as "using a::Foo" don't trigger
   // typeLoc for "a::Foo".
   for (const auto *Using : Finder.getUsingDecls())
     Replace(Using->getLocStart(), Using->getLocEnd(), "using " + NewName.str());
 
   return AtomicChanges;
 }
 
 } // end namespace tooling
 } // end namespace clang

unittests/Rename/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   support
   )
 
 # We'd like clang/unittests/Tooling/RewriterTestContext.h in the test.
 include_directories(${CLANG_SOURCE_DIR})
 
 add_clang_unittest(ClangRenameTests
   RenameClassTest.cpp
+  RenameFunctionTest.cpp
   )
 
 target_link_libraries(ClangRenameTests
   clangAST
   clangASTMatchers
   clangBasic
   clangFormat
   clangFrontend
   clangRewrite
   clangTooling
   clangToolingCore
   clangToolingRefactor
   )

unittests/Rename/RenameClassTest.cpp

public:
   }
 };
 
 INSTANTIATE_TEST_CASE_P(
     RenameClassTests, RenameClassTest,
     testing::ValuesIn(std::vector<Case>({
         // basic classes
         {"a::Foo f;", "b::Bar f;", "", ""},
+        {"::a::Foo f;", "::b::Bar f;", "", ""},
         {"void f(a::Foo f) {}", "void f(b::Bar f) {}", "", ""},
         {"void f(a::Foo *f) {}", "void f(b::Bar *f) {}", "", ""},
         {"a::Foo f() { return a::Foo(); }", "b::Bar f() { return b::Bar(); }",
          "", ""},
         {"namespace a {a::Foo f() { return Foo(); }}",
          "namespace a {b::Bar f() { return b::Bar(); }}", "", ""},
         {"void f(const a::Foo& a1) {}", "void f(const b::Bar& a1) {}", "", ""},
         {"void f(const a::Foo* a1) {}", "void f(const b::Bar* a1) {}", "", ""},
         {"namespace a { void f(Foo a1) {} }",
          "namespace a { void f(b::Bar a1) {} }", "", ""},
         {"void f(MACRO(a::Foo) a1) {}", "void f(MACRO(b::Bar) a1) {}", "", ""},
         {"void f(MACRO(a::Foo a1)) {}", "void f(MACRO(b::Bar a1)) {}", "", ""},
         {"a::Foo::Nested ns;", "b::Bar::Nested ns;", "", ""},
         {"auto t = a::Foo::Constant;", "auto t = b::Bar::Constant;", "", ""},
         {"a::Foo::Nested ns;", "a::Foo::Nested2 ns;", "a::Foo::Nested",
          "a::Foo::Nested2"},
 
         // use namespace and typedefs
-        {"using a::Foo; Foo gA;", "using b::Bar; b::Bar gA;", "", ""},
+        {"using a::Foo; Foo gA;", "using b::Bar; Bar gA;", "", ""},
         {"using a::Foo; void f(Foo gA) {}", "using b::Bar; void f(Bar gA) {}",
          "", ""},
         {"using a::Foo; namespace x { Foo gA; }",
          "using b::Bar; namespace x { Bar gA; }", "", ""},
         {"struct S { using T = a::Foo; T a_; };",
          "struct S { using T = b::Bar; T a_; };", "", ""},
         {"using T = a::Foo; T gA;", "using T = b::Bar; T gA;", "", ""},
         {"typedef a::Foo T; T gA;", "typedef b::Bar T; T gA;", "", ""},
testing::ValuesIn(std::vector<Case>({
          " void f() { FOO(a1); FOO(a2); }",
          "#define FOO(n) b::Bar n\n"
          " void f() { FOO(a1); FOO(a2); }",
          "", ""},
 
         // Pointer to member functions
         {"auto gA = &a::Foo::func;", "auto gA = &b::Bar::func;", "", ""},
         {"using a::Foo; auto gA = &Foo::func;",
-         "using b::Bar; auto gA = &b::Bar::func;", "", ""},
+         "using b::Bar; auto gA = &Bar::func;", "", ""},
         {"using a::Foo; namespace x { auto gA = &Foo::func; }",
          "using b::Bar; namespace x { auto gA = &Bar::func; }", "", ""},
         {"typedef a::Foo T; auto gA = &T::func;",
          "typedef b::Bar T; auto gA = &T::func;", "", ""},
         {"auto gA = &MACRO(a::Foo)::func;", "auto gA = &MACRO(b::Bar)::func;",
          "", ""},
 
         // Short match inside a namespace

unittests/Rename/RenameFunctionTest.cpp

+//===-- RenameFunctionTest.cpp - unit tests for renaming functions --------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameFunctionTest : public ClangRenameTest {
+public:
+  RenameFunctionTest() {
+    AppendToHeader(R"(
+      struct A {
+        static bool Foo();
+        static bool Spam();
+      };
+      struct B {
+        static void Same();
+        static bool Foo();
+        static int Eric(int x);
+      };
+      void Same(int x);
+      int Eric(int x);
+      namespace base {
+        void Same();
+        void ToNanoSeconds();
+        void ToInt64NanoSeconds();
+      })");
+  }
+};
+
+TEST_F(RenameFunctionTest, RefactorsAFoo) {
+  std::string Before = R"(
+      void f() {
+        A::Foo();
+        ::A::Foo();
+      })";
+  std::string Expected = R"(
+      void f() {
+        A::Bar();
+        ::A::Bar();
+      })";
+
+  std::string After = runClangRenameOnCode(Before, "A::Foo", "A::Bar");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RefactorsNonCallingAFoo) {
+  std::string Before = R"(
+      bool g(bool (*func)()) {
+        return func();
+      }
+      void f() {
+        auto *ref1 = A::Foo;
+        auto *ref2 = ::A::Foo;
+        g(A::Foo);
+      })";
+  std::string Expected = R"(
+      bool g(bool (*func)()) {
+        return func();
+      }
+      void f() {
+        auto *ref1 = A::Bar;
+        auto *ref2 = ::A::Bar;
+        g(A::Bar);
+      })";
+  std::string After = runClangRenameOnCode(Before, "A::Foo", "A::Bar");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RefactorsEric) {
+  std::string Before = R"(
+      void f() {
+        if (Eric(3)==4) ::Eric(2);
+      })";
+  std::string Expected = R"(
+      void f() {
+        if (Larry(3)==4) ::Larry(2);
+      })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RefactorsNonCallingEric) {
+  std::string Before = R"(
+        int g(int (*func)(int)) {
+          return func(1);
+        }
+        void f() {
+          auto *ref = ::Eric;
+          g(Eric);
+        })";
+  std::string Expected = R"(
+        int g(int (*func)(int)) {
+          return func(1);
+        }
+        void f() {
+          auto *ref = ::Larry;
+          g(Larry);
+        })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorBFoo) {
+  std::string Before = R"(
+      void f() {
+        B::Foo();
+      })";
+  std::string After = runClangRenameOnCode(Before, "A::Foo", "A::Bar");
+  CompareSnippets(Before, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorBEric) {
+  std::string Before = R"(
+      void f() {
+        B::Eric(2);
+      })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Before, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorCEric) {
+  std::string Before = R"(
+      namespace C { int Eric(int x); }
+      void f() {
+        if (C::Eric(3)==4) ::C::Eric(2);
+      })";
+  std::string Expected = R"(
+      namespace C { int Eric(int x); }
+      void f() {
+        if (C::Eric(3)==4) ::C::Eric(2);
+      })";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, DoesNotRefactorEricInNamespaceC) {
+  std::string Before = R"(
+       namespace C {
+       int Eric(int x);
+       void f() {
+         if (Eric(3)==4) Eric(2);
+       }
+       }  // namespace C)";
+  std::string After = runClangRenameOnCode(Before, "Eric", "Larry");
+  CompareSnippets(Before, After);
+}
+
+TEST_F(RenameFunctionTest, NamespaceQualified) {
+  std::string Before = R"(
+      void f() {
+        base::ToNanoSeconds();
+        ::base::ToNanoSeconds();
+      }
+      void g() {
+        using base::ToNanoSeconds;
+        base::ToNanoSeconds();
+        ::base::ToNanoSeconds();
+        ToNanoSeconds();
+      }
+      namespace foo {
+        namespace base {
+          void ToNanoSeconds();
+          void f() {
+            base::ToNanoSeconds();
+          }
+        }
+        void f() {
+          ::base::ToNanoSeconds();
+        }
+      })";
+  std::string Expected = R"(
+      void f() {
+        base::ToInt64NanoSeconds();
+        ::base::ToInt64NanoSeconds();
+      }
+      void g() {
+        using base::ToInt64NanoSeconds;
+        base::ToInt64NanoSeconds();
+        ::base::ToInt64NanoSeconds();
+        ToInt64NanoSeconds();
+      }
+      namespace foo {
+        namespace base {
+          void ToNanoSeconds();
+          void f() {
+            base::ToNanoSeconds();
+          }
+        }
+        void f() {
+          ::base::ToInt64NanoSeconds();
+        }
+      })";
+  std::string After = runClangRenameOnCode(Before, "base::ToNanoSeconds",
+                                           "base::ToInt64NanoSeconds");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RenameFunctionDecls) {
+  std::string Before = R"(
+      namespace na {
+        void X();
+        void X() {}
+      })";
+  std::string Expected = R"(
+      namespace na {
+        void Y();
+        void Y() {}
+      })";
+  std::string After = runClangRenameOnCode(Before, "na::X", "na::Y");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RenameOutOfLineFunctionDecls) {
+  std::string Before = R"(
+      namespace na {
+        void X();
+      }
+      void na::X() {}
+      )";
+  std::string Expected = R"(
+      namespace na {
+        void Y();
+      }
+      void na::Y() {}
+      )";
+  std::string After = runClangRenameOnCode(Before, "na::X", "na::Y");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, NewNamespaceWithoutLeadingDotDot) {
+  std::string Before = R"(
+      namespace old_ns {
+        void X();
+        void X() {}
+      }
+      // Assume that the reference is in another file.
+      void f() { old_ns::X(); }
+      namespace old_ns { void g() { X(); } }
+      namespace new_ns { void h() { ::old_ns::X(); } }
+      )";
+  std::string Expected = R"(
+      namespace old_ns {
+        void Y();
+        void Y() {}
+      }
+      // Assume that the reference is in another file.
+      void f() { new_ns::Y(); }
+      namespace old_ns { void g() { new_ns::Y(); } }
+      namespace new_ns { void h() { Y(); } }
+      )";
+  std::string After = runClangRenameOnCode(Before, "::old_ns::X", "new_ns::Y");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, NewNamespaceWithLeadingDotDot) {
+  std::string Before = R"(
+      namespace old_ns {
+        void X();
+        void X() {}
+      }
+      // Assume that the reference is in another file.
+      void f() { old_ns::X(); }
+      namespace old_ns { void g() { X(); } }
+      namespace new_ns { void h() { ::old_ns::X(); } }
+      )";
+  std::string Expected = R"(
+      namespace old_ns {
+        void Y();
+        void Y() {}
+      }
+      // Assume that the reference is in another file.
+      void f() { ::new_ns::Y(); }
+      namespace old_ns { void g() { ::new_ns::Y(); } }
+      namespace new_ns { void h() { Y(); } }
+      )";
+  std::string After =
+      runClangRenameOnCode(Before, "::old_ns::X", "::new_ns::Y");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, DontRenameSymbolsDefinedInAnonymousNamespace) {
+  std::string Before = R"(
+      namespace old_ns {
+      class X {};
+      namespace {
+        void X();
+        void X() {}
+        void f() { X(); }
+      }
+      }
+      )";
+  std::string Expected = R"(
+      namespace old_ns {
+      class Y {};
+      namespace {
+        void X();
+        void X() {}
+        void f() { X(); }
+      }
+      }
+      )";
+  std::string After =
+      runClangRenameOnCode(Before, "::old_ns::X", "::old_ns::Y");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, NewNestedNamespace) {
+  std::string Before = R"(
+      namespace old_ns {
+        void X();
+        void X() {}
+      }
+      // Assume that the reference is in another file.
+      namespace old_ns {
+      void f() { X(); }
+      }
+      )";
+  std::string Expected = R"(
+      namespace old_ns {
+        void X();
+        void X() {}
+      }
+      // Assume that the reference is in another file.
+      namespace old_ns {
+      void f() { older_ns::X(); }
+      }
+      )";
+  std::string After =
+      runClangRenameOnCode(Before, "::old_ns::X", "::old_ns::older_ns::X");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, MoveFromGlobalToNamespaceWithoutLeadingDotDot) {
+  std::string Before = R"(
+      void X();
+      void X() {}
+
+      // Assume that the reference is in another file.
+      namespace some_ns {
+      void f() { X(); }
+      }
+      )";
+  std::string Expected = R"(
+      void X();
+      void X() {}
+
+      // Assume that the reference is in another file.
+      namespace some_ns {
+      void f() { ns::X(); }
+      }
+      )";
+  std::string After =
+      runClangRenameOnCode(Before, "::X", "ns::X");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, MoveFromGlobalToNamespaceWithLeadingDotDot) {
+  std::string Before = R"(
+      void Y() {}
+
+      // Assume that the reference is in another file.
+      namespace some_ns {
+      void f() { Y(); }
+      }
+      )";
+  std::string Expected = R"(
+      void Y() {}
+
+      // Assume that the reference is in another file.
+      namespace some_ns {
+      void f() { ::ns::Y(); }
+      }
+      )";
+  std::string After =
+      runClangRenameOnCode(Before, "::Y", "::ns::Y");
+  CompareSnippets(Expected, After);
+}
+
+// FIXME: the rename of overloaded operator is not fully supported yet.
+TEST_F(RenameFunctionTest, DISABLED_DoNotRenameOverloadedOperatorCalls) {
+  std::string Before = R"(
+      namespace old_ns {
+      class T { public: int x; };
+      bool operator==(const T& lhs, const T& rhs) {
+        return lhs.x == rhs.x;
+      }
+      }  // namespace old_ns
+
+      // Assume that the reference is in another file.
+      bool f() {
+        auto eq = old_ns::operator==;
+        old_ns::T t1, t2;
+        old_ns::operator==(t1, t2);
+        return t1 == t2;
+      }
+      )";
+  std::string Expected = R"(
+      namespace old_ns {
+      class T { public: int x; };
+      bool operator==(const T& lhs, const T& rhs) {
+        return lhs.x == rhs.x;
+      }
+      }  // namespace old_ns
+
+      // Assume that the reference is in another file.
+      bool f() {
+        auto eq = new_ns::operator==;
+        old_ns::T t1, t2;
+        new_ns::operator==(t1, t2);
+        return t1 == t2;
+      }
+      )";
+  std::string After =
+      runClangRenameOnCode(Before, "old_ns::operator==", "new_ns::operator==");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, FunctionRefAsTemplate) {
+  std::string Before = R"(
+      void X();
+
+      // Assume that the reference is in another file.
+      namespace some_ns {
+      template <void (*Func)(void)>
+      class TIterator {};
+
+      template <void (*Func)(void)>
+      class T {
+      public:
+        typedef TIterator<Func> IterType;
+        using TI = TIterator<Func>;
+        void g() {
+          Func();
+          auto func = Func;
+          TIterator<Func> iter;
+        }
+      };
+
+
+      void f() { T<X> tx; tx.g(); }
+      }  // namespace some_ns
+      )";
+  std::string Expected = R"(
+      void X();
+
+      // Assume that the reference is in another file.
+      namespace some_ns {
+      template <void (*Func)(void)>
+      class TIterator {};
+
+      template <void (*Func)(void)>
+      class T {
+      public:
+        typedef TIterator<Func> IterType;
+        using TI = TIterator<Func>;
+        void g() {
+          Func();
+          auto func = Func;
+          TIterator<Func> iter;
+        }
+      };
+
+
+      void f() { T<ns::X> tx; tx.g(); }
+      }  // namespace some_ns
+      )";
+  std::string After = runClangRenameOnCode(Before, "::X", "ns::X");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameFunctionTest, RenameFunctionInUsingDecl) {
+  std::string Before = R"(
+      using base::ToNanoSeconds;
+      namespace old_ns {
+      using base::ToNanoSeconds;
+      void f() {
+        using base::ToNanoSeconds;
+      }
+      }
+      )";
+  std::string Expected = R"(
+      using base::ToInt64NanoSeconds;
+      namespace old_ns {
+      using base::ToInt64NanoSeconds;
+      void f() {
+        using base::ToInt64NanoSeconds;
+      }
+      }
+      )";
+  std::string After = runClangRenameOnCode(Before, "base::ToNanoSeconds",
+                                           "base::ToInt64NanoSeconds");
+  CompareSnippets(Expected, After);
+}
+
+} // anonymous namespace
+} // namespace test
+} // namespace clang_rename
+} // namesdpace clang