[rename] support renaming class member.

[ioeric]

Lgtm. Nice!

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

251

Is there a test case for this?

unittests/Rename/RenameMemberTest.cpp

206

Maybe add one more field in the test?

[hokein]

• remove the code of handling template class methods as it is fixed in another patch.
• address review comments.

[hokein]

I have refined the patch based on the D39241. Please take another look.

[ioeric]

Still lgtm.

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

if (isInUSRSet(Decl)) {
                            /*Specifier=*/nullptr,
                            /*IgnorePrefixQualifers=*/true};
         RenameInfos.push_back(Info);
       }
     }
     return true;
   }
 
+  bool VisitMemberExpr(const MemberExpr *Expr) {
+    const NamedDecl *Decl = Expr->getFoundDecl();
+    auto StartLoc = Expr->getMemberLoc();
+    auto EndLoc = Expr->getMemberLoc();
+    if (isInUSRSet(Decl)) {
+      RenameInfos.push_back({StartLoc, EndLoc,
+                            /*FromDecl=*/nullptr,
+                            /*Context=*/nullptr,
+                            /*Specifier=*/nullptr,
+                            /*IgnorePrefixQualifiers=*/true});
+      return true;
+    }
+
+    // A class template method could have multiple AST nodes (each class
+    // specialization has its own instantiated method). Only the USR of the
+    // template method would be added to the USR set, so we need to retrieve the
+    // method template from which the member function of the class template
+    // specialization was instantiated, and check whether it is in the USR set.
+    if (const auto *FD = llvm::dyn_cast<FunctionDecl>(Decl)) {
+      if (const auto *T = FD->getInstantiatedFromMemberFunction()) {
+        if (isInUSRSet(T)) {
+          RenameInfos.push_back({StartLoc, EndLoc,
+                                 /*FromDecl=*/nullptr,
+                                 /*Context=*/nullptr,
+                                 /*Specifier=*/nullptr,
+                                 /*IgnorePrefixQualifiers=*/true});
+        }
+      }
+    }
+    return true;
+  }
+
+  bool VisitCXXConstructorDecl(const CXXConstructorDecl *CD) {
+    // Fix the constructor initializer when renaming class members.
+    for (const auto *Initializer : CD->inits()) {
+      // Ignore implicit initializers.
+      if (!Initializer->isWritten())
+        continue;
+
+      if (const FieldDecl *FD = Initializer->getMember()) {
+        if (isInUSRSet(FD)) {
+          auto Loc = Initializer->getSourceLocation();
+          RenameInfos.push_back({Loc, Loc,
+                                 /*FromDecl=*/nullptr,
+                                 /*Context=*/nullptr,
+                                 /*Specifier=*/nullptr,
+                                 /*IgnorePrefixQualifiers=*/true});
+        }
+      }
+    }
+    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();
     }
 

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
   RenameEnumTest.cpp
   RenameAliasTest.cpp
+  RenameMemberTest.cpp
   RenameFunctionTest.cpp
   )
 
 target_link_libraries(ClangRenameTests
   clangAST
   clangASTMatchers
   clangBasic
   clangFormat
   clangFrontend
   clangRewrite
   clangTooling
   clangToolingCore
   clangToolingRefactor
   )

unittests/Rename/RenameMemberTest.cpp

+//===-- ClangMemberTests.cpp - unit tests for renaming class members ------===//
+//
+//                     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 RenameMemberTest : public ClangRenameTest {
+public:
+  RenameMemberTest() {
+    AppendToHeader(R"(
+        struct NA {
+          void Foo();
+          void NotFoo();
+          static void SFoo();
+          static void SNotFoo();
+          int Moo;
+        };
+        struct A {
+          virtual void Foo();
+          void NotFoo();
+          static void SFoo();
+          static void SNotFoo();
+          int Moo;
+          int NotMoo;
+        };
+        struct B : public A {
+          void Foo() override;
+        };
+        template <typename T> struct TA {
+          T* Foo();
+          T* NotFoo();
+          static T* SFoo();
+          static T* NotSFoo();
+        };
+        template <typename T> struct TB : public TA<T> {};
+        namespace ns {
+          template <typename T> struct TA {
+            T* Foo();
+            T* NotFoo();
+            static T* SFoo();
+            static T* NotSFoo();
+          };
+          template <typename T> struct TB : public TA<T> {};
+          struct A {
+            void Foo();
+            void NotFoo();
+            static void SFoo();
+            static void SNotFoo();
+          };
+          struct B : public A {};
+          struct C {
+            template <class T>
+            void SFoo(const T& t) {}
+            template <class T>
+            void Foo() {}
+          };
+        })");
+  }
+};
+
+INSTANTIATE_TEST_CASE_P(
+    DISABLED_RenameTemplateMethodTest, RenameMemberTest,
+    testing::ValuesIn(std::vector<Case>({
+        // FIXME: support renaming static template methods for template classs.
+        //
+        // A template static class method of a template class could have more
+        // than one AST node (each class specialization has its own instantiated
+        // method). The current rename library only finds the NamedDecl of the
+        // template method for given qualified name. To support this, we need
+        // to record all NamedDecls (including the instantiated ones) which the
+        // static class method corresponds to.
+        {"void f() { TA<int>::SFoo(); }", "void f() { TA<int>::SBar(); }",
+         "TA::SFoo", "TA::SBar"},
+        {"void f() { ns::TA<int>::SFoo(); }",
+         "void f() { ns::TA<int>::SBar(); }", "ns::TA::SFoo", "ns::TA::SBar"},
+        {"void f() { TB<int>::SFoo(); }", "void f() { TA<int>::SBar(); }",
+         "TA::SFoo", "TA::SBar"},
+        {"void f() { ns::TB<int>::SFoo(); }",
+         "void f() { ns::TA<int>::SBar(); }", "ns::TA::SFoo", "ns::TA::SBar"},
+    })), );
+
+INSTANTIATE_TEST_CASE_P(
+    RenameMemberTest, RenameMemberTest,
+    testing::ValuesIn(std::vector<Case>({
+        // Normal methods and fields.
+        {"void f() { A a; a.Foo(); }", "void f() { A a; a.Bar(); }", "A::Foo",
+         "A::Bar"},
+        {"void f() { ns::A a; a.Foo(); }", "void f() { ns::A a; a.Bar(); }",
+         "ns::A::Foo", "ns::A::Bar"},
+        {"void f() { A a; int x = a.Moo; }", "void f() { A a; int x = a.Meh; }",
+         "A::Moo", "A::Meh"},
+        {"void f() { B b; b.Foo(); }", "void f() { B b; b.Bar(); }", "B::Foo",
+         "B::Bar"},
+        {"void f() { ns::B b; b.Foo(); }", "void f() { ns::B b; b.Bar(); }",
+         "ns::A::Foo", "ns::A::Bar"},
+        {"void f() { B b; int x = b.Moo; }", "void f() { B b; int x = b.Meh; }",
+         "A::Moo", "A::Meh"},
+
+        // Static methods.
+        {"void f() { A::SFoo(); }", "void f() { A::SBar(); }", "A::SFoo",
+         "A::SBar"},
+        {"void f() { ns::A::SFoo(); }", "void f() { ns::A::SBar(); }",
+         "ns::A::SFoo", "ns::A::SBar"},
+
+        // Templated methods.
+        {"void f() { TA<int> a; a.Foo(); }", "void f() { TA<int> a; a.Bar(); }",
+         "TA::Foo", "TA::Bar"},
+        {"void f() { ns::TA<int> a; a.Foo(); }",
+         "void f() { ns::TA<int> a; a.Bar(); }", "ns::TA::Foo", "ns::TA::Bar"},
+        {"void f() { TB<int> b; b.Foo(); }", "void f() { TB<int> b; b.Bar(); }",
+         "TA::Foo", "TA::Bar"},
+        {"void f() { ns::TB<int> b; b.Foo(); }",
+         "void f() { ns::TB<int> b; b.Bar(); }", "ns::TA::Foo", "ns::TA::Bar"},
+        {"void f() { ns::C c; int x; c.SFoo(x); }",
+         "void f() { ns::C c; int x; c.SBar(x); }", "ns::C::SFoo",
+         "ns::C::SBar"},
+        {"void f() { ns::C c; c.Foo<int>(); }",
+         "void f() { ns::C c; c.Bar<int>(); }", "ns::C::Foo", "ns::C::Bar"},
+
+        // Pointers to methods.
+        {"void f() { auto p = &A::Foo; }", "void f() { auto p = &A::Bar; }",
+         "A::Foo", "A::Bar"},
+        {"void f() { auto p = &A::SFoo; }", "void f() { auto p = &A::SBar; }",
+         "A::SFoo", "A::SBar"},
+        {"void f() { auto p = &B::Foo; }", "void f() { auto p = &B::Bar; }",
+         "B::Foo", "B::Bar"},
+        {"void f() { auto p = &ns::A::Foo; }",
+         "void f() { auto p = &ns::A::Bar; }", "ns::A::Foo", "ns::A::Bar"},
+        {"void f() { auto p = &ns::A::SFoo; }",
+         "void f() { auto p = &ns::A::SBar; }", "ns::A::SFoo", "ns::A::SBar"},
+        {"void f() { auto p = &ns::C::SFoo<int>; }",
+         "void f() { auto p = &ns::C::SBar<int>; }", "ns::C::SFoo",
+         "ns::C::SBar"},
+
+        // These methods are not declared or overrided in the subclass B, we
+        // have to use the qualified name with parent class A to identify them.
+        {"void f() { auto p = &ns::B::Foo; }",
+         "void f() { auto p = &ns::B::Bar; }", "ns::A::Foo", "ns::B::Bar"},
+        {"void f() { B::SFoo(); }", "void f() { B::SBar(); }", "A::SFoo",
+         "B::SBar"},
+        {"void f() { ns::B::SFoo(); }", "void f() { ns::B::SBar(); }",
+         "ns::A::SFoo", "ns::B::SBar"},
+        {"void f() { auto p = &B::SFoo; }", "void f() { auto p = &B::SBar; }",
+         "A::SFoo", "B::SBar"},
+        {"void f() { auto p = &ns::B::SFoo; }",
+         "void f() { auto p = &ns::B::SBar; }", "ns::A::SFoo", "ns::B::SBar"},
+    })), );
+
+TEST_P(RenameMemberTest, RenameMembers) {
+  auto Param = GetParam();
+  assert(!Param.OldName.empty());
+  assert(!Param.NewName.empty());
+  std::string Actual =
+      runClangRenameOnCode(Param.Before, Param.OldName, Param.NewName);
+  CompareSnippets(Param.After, Actual);
+}
+
+TEST_F(RenameMemberTest, RenameMemberInsideClassMethods) {
+  std::string Before = R"(
+      struct X {
+        int Moo;
+        void Baz() { Moo = 1; }
+      };)";
+  std::string Expected = R"(
+      struct X {
+        int Meh;
+        void Baz() { Meh = 1; }
+      };)";
+  std::string After = runClangRenameOnCode(Before, "X::Moo", "Y::Meh");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameMemberTest, RenameMethodInsideClassMethods) {
+  std::string Before = R"(
+      struct X {
+        void Foo() {}
+        void Baz() { Foo(); }
+      };)";
+  std::string Expected = R"(
+      struct X {
+        void Bar() {}
+        void Baz() { Bar(); }
+      };)";
+  std::string After = runClangRenameOnCode(Before, "X::Foo", "X::Bar");
+  CompareSnippets(Expected, After);
+}
+
+TEST_F(RenameMemberTest, RenameCtorInitializer) {
+  std::string Before = R"(
+      class X {
+      public:
+       X();
+       A a;
+      };
+
+      X::X():a() {}
+      )";
+  std::string Expected = R"(
+      class X {
+      public:
+       X();
+       A bar;
+      };
+
+      X::X():bar() {}
+      )";
+  std::string After = runClangRenameOnCode(Before, "X::a", "X::bar");
+  CompareSnippets(Expected, After);
+}
+
+} // anonymous namespace
+} // namespace test
+} // namespace clang_rename
+} // namesdpace clang