[clang-rename] Don't add prefix qualifiers to the declaration and definition of the...

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: klimek, cfe-commits, arphaman

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@315452 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp b/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
index 43b03caa2ed2925f0878d8e2989d9be97b513722..76a173f4b49eb947ca93a039e62fbf7fa9320705 100644 (file)
--- a/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ b/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -160,13 +160,14 @@ public:
     const Decl *Context;
     // The nested name being replaced (can be nullptr).
     const NestedNameSpecifier *Specifier;
+    // Determine whether the prefix qualifiers of the NewName should be ignored.
+    // Normally, we set it to true for the symbol declaration and definition to
+    // avoid adding prefix qualifiers.
+    // For example, if it is true and NewName is "a::b::foo", then the symbol
+    // occurrence which the RenameInfo points to will be renamed to "foo".
+    bool IgnorePrefixQualifers;
   };
 
-  // FIXME: Currently, prefix qualifiers will be added to the renamed symbol
-  // definition (e.g. "class Foo {};" => "class b::Bar {};" when renaming
-  // "a::Foo" to "b::Bar").
-  // For renaming declarations/definitions, prefix qualifiers should be filtered
-  // out.
   bool VisitNamedDecl(const NamedDecl *Decl) {
     // UsingDecl has been handled in other place.
     if (llvm::isa<UsingDecl>(Decl))
@@ -180,8 +181,12 @@ public:
       return true;
 
     if (isInUSRSet(Decl)) {
-      RenameInfo Info = {Decl->getLocation(), Decl->getLocation(), nullptr,
-                         nullptr, nullptr};
+      RenameInfo Info = {Decl->getLocation(),
+                         Decl->getLocation(),
+                         /*FromDecl=*/nullptr,
+                         /*Context=*/nullptr,
+                         /*Specifier=*/nullptr,
+                         /*IgnorePrefixQualifers=*/true};
       RenameInfos.push_back(Info);
     }
     return true;
@@ -191,8 +196,11 @@ public:
     const NamedDecl *Decl = Expr->getFoundDecl();
     if (isInUSRSet(Decl)) {
       RenameInfo Info = {Expr->getSourceRange().getBegin(),
-                         Expr->getSourceRange().getEnd(), Decl,
-                         getClosestAncestorDecl(*Expr), Expr->getQualifier()};
+                         Expr->getSourceRange().getEnd(),
+                         Decl,
+                         getClosestAncestorDecl(*Expr),
+                         Expr->getQualifier(),
+                         /*IgnorePrefixQualifers=*/false};
       RenameInfos.push_back(Info);
     }
 
@@ -220,8 +228,10 @@ public:
       if (isInUSRSet(TargetDecl)) {
         RenameInfo Info = {NestedLoc.getBeginLoc(),
                            EndLocationForType(NestedLoc.getTypeLoc()),
-                           TargetDecl, getClosestAncestorDecl(NestedLoc),
-                           NestedLoc.getNestedNameSpecifier()->getPrefix()};
+                           TargetDecl,
+                           getClosestAncestorDecl(NestedLoc),
+                           NestedLoc.getNestedNameSpecifier()->getPrefix(),
+                           /*IgnorePrefixQualifers=*/false};
         RenameInfos.push_back(Info);
       }
     }
@@ -265,9 +275,12 @@ public:
         if (!ParentTypeLoc.isNull() &&
             isInUSRSet(getSupportedDeclFromTypeLoc(ParentTypeLoc)))
           return true;
-        RenameInfo Info = {StartLocationForType(Loc), EndLocationForType(Loc),
-                           TargetDecl, getClosestAncestorDecl(Loc),
-                           GetNestedNameForType(Loc)};
+        RenameInfo Info = {StartLocationForType(Loc),
+                           EndLocationForType(Loc),
+                           TargetDecl,
+                           getClosestAncestorDecl(Loc),
+                           GetNestedNameForType(Loc),
+                           /*IgnorePrefixQualifers=*/false};
         RenameInfos.push_back(Info);
         return true;
       }
@@ -293,11 +306,13 @@ public:
             llvm::isa<ElaboratedType>(ParentTypeLoc.getType()))
           TargetLoc = ParentTypeLoc;
         RenameInfo Info = {
-            StartLocationForType(TargetLoc), EndLocationForType(TargetLoc),
+            StartLocationForType(TargetLoc),
+            EndLocationForType(TargetLoc),
             TemplateSpecType->getTemplateName().getAsTemplateDecl(),
             getClosestAncestorDecl(
                 ast_type_traits::DynTypedNode::create(TargetLoc)),
-            GetNestedNameForType(TargetLoc)};
+            GetNestedNameForType(TargetLoc),
+            /*IgnorePrefixQualifers=*/false};
         RenameInfos.push_back(Info);
       }
     }
@@ -423,18 +438,26 @@ createRenameAtomicChanges(llvm::ArrayRef<std::string> USRs,
 
   for (const auto &RenameInfo : Finder.getRenameInfos()) {
     std::string ReplacedName = NewName.str();
-    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 (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();
     }
-    // If the NewName contains leading "::", add it back.
-    if (NewName.startswith("::") && NewName.substr(2) == ReplacedName)
-      ReplacedName = NewName.str();
     Replace(RenameInfo.Begin, RenameInfo.End, ReplacedName);
   }
 

diff --git a/unittests/Rename/RenameClassTest.cpp b/unittests/Rename/RenameClassTest.cpp
index 29b4594fb0a4aba6cc9c112a0916a055136dc1af..74280de7871b6e9c7c919fc75b219baa02ef0e64 100644 (file)
--- a/unittests/Rename/RenameClassTest.cpp
+++ b/unittests/Rename/RenameClassTest.cpp
@@ -469,8 +469,6 @@ TEST_F(ClangRenameTest, RenameClassWithInlineMembers) {
   CompareSnippets(Expected, After);
 }
 
-// FIXME: no prefix qualifiers being added to the class definition and
-// constructor.
 TEST_F(ClangRenameTest, RenameClassWithNamespaceWithInlineMembers) {
   std::string Before = R"(
       namespace ns {
@@ -488,9 +486,9 @@ TEST_F(ClangRenameTest, RenameClassWithNamespaceWithInlineMembers) {
     )";
   std::string Expected = R"(
       namespace ns {
-      class ns::New {
+      class New {
        public:
-        ns::New() {}
+        New() {}
         ~New() {}
 
         New* next() { return next_; }
@@ -504,8 +502,6 @@ TEST_F(ClangRenameTest, RenameClassWithNamespaceWithInlineMembers) {
   CompareSnippets(Expected, After);
 }
 
-// FIXME: no prefix qualifiers being added to the class definition and
-// constructor.
 TEST_F(ClangRenameTest, RenameClassWithNamespaceWithOutOfInlineMembers) {
   std::string Before = R"(
       namespace ns {
@@ -527,9 +523,9 @@ TEST_F(ClangRenameTest, RenameClassWithNamespaceWithOutOfInlineMembers) {
     )";
   std::string Expected = R"(
       namespace ns {
-      class ns::New {
+      class New {
        public:
-        ns::New();
+        New();
         ~New();
 
         New* next();
@@ -538,7 +534,7 @@ TEST_F(ClangRenameTest, RenameClassWithNamespaceWithOutOfInlineMembers) {
         New* next_;
       };
 
-      New::ns::New() {}
+      New::New() {}
       New::~New() {}
       New* New::next() { return next_; }
       }  // namespace ns
@@ -547,12 +543,12 @@ TEST_F(ClangRenameTest, RenameClassWithNamespaceWithOutOfInlineMembers) {
   CompareSnippets(Expected, After);
 }
 
-// FIXME: no prefix qualifiers being added to the definition.
 TEST_F(ClangRenameTest, RenameClassInInheritedConstructor) {
   // `using Base::Base;` will generate an implicit constructor containing usage
   // of `::ns::Old` which should not be matched.
   std::string Before = R"(
       namespace ns {
+      class Old;
       class Old {
         int x;
       };
@@ -574,7 +570,8 @@ TEST_F(ClangRenameTest, RenameClassInInheritedConstructor) {
       })";
   std::string Expected = R"(
       namespace ns {
-      class ns::New {
+      class New;
+      class New {
         int x;
       };
       class Base {
@@ -615,7 +612,7 @@ TEST_F(ClangRenameTest, DontRenameReferencesInImplicitFunction) {
       )";
   std::string Expected = R"(
       namespace ns {
-      class ::new_ns::New {
+      class New {
       };
       } // namespace ns
       struct S {
@@ -632,7 +629,6 @@ TEST_F(ClangRenameTest, DontRenameReferencesInImplicitFunction) {
   CompareSnippets(Expected, After);
 }
 
-// FIXME: no prefix qualifiers being adding to the definition.
 TEST_F(ClangRenameTest, ReferencesInLambdaFunctionParameters) {
   std::string Before = R"(
       template <class T>
@@ -669,7 +665,7 @@ TEST_F(ClangRenameTest, ReferencesInLambdaFunctionParameters) {
       };
 
       namespace ns {
-      class ::new_ns::New {};
+      class New {};
       void f() {
         function<void(::new_ns::New)> func;
       }