[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

[Tom Praschan via Phabricator via cfe-commits]

tom-anders created this revision.
tom-anders added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/170

This patch actually consists of 2 fixes:

1. Add handling for UsingShadowDecl to canonicalRenameDecl(). This fixes the issue described in https://github.com/clangd/clangd/issues/170.

2. Avoid the "there are multiple symbols under the cursor error" by applying similar logic as in https://reviews.llvm.org/D133664. This also partly fixes https://github.com/clangd/clangd/issues/586.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135489

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -817,6 +817,18 @@
           char [[da^ta]];
         } @end
       )cpp",
+
+      // Issue 170: Rename symbol introduced by UsingDecl
+      R"cpp(
+        namespace ns { void [[f^oo]](); } 
+
+        using ns::[[f^oo]];
+
+        void f() {
+            [[f^oo]]();
+            auto p = &[[f^oo]];
+        }
+      )cpp",
   };
   llvm::StringRef NewName = "NewName";
   for (llvm::StringRef T : Tests) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -133,6 +133,10 @@
     if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember())
       return canonicalRenameDecl(OriginalVD);
   }
+  if (const auto *UD = dyn_cast<UsingShadowDecl>(D)) {
+    if (const auto* TargetDecl = UD->getTargetDecl()) 
+      return canonicalRenameDecl(TargetDecl);
+  }
   return dyn_cast<NamedDecl>(D->getCanonicalDecl());
 }
 
@@ -157,6 +161,23 @@
   return Result;
 }
 
+// For something like 
+//     namespace ns { void foo(); }
+//     void bar() { using ns::foo; f^oo(); }
+// locateDeclAt() will return a BaseUsingDecl and foo's actual declaration.
+// For renaming, we're only interested in foo's declaration, so drop the other one
+void filterBaseUsingDecl(llvm::DenseSet<const NamedDecl *>& Decls) {
+  if (Decls.size() == 2) {
+    auto FirstDecl = Decls.begin();
+    auto SecondDecl = std::next(Decls.begin());
+
+    if (llvm::isa<BaseUsingDecl>(*FirstDecl)) 
+      Decls.erase(FirstDecl);
+    else if (llvm::isa<BaseUsingDecl>(*SecondDecl)) 
+      Decls.erase(SecondDecl);
+  }
+}
+
 // By default, we exclude symbols from system headers and protobuf symbols as
 // renaming these symbols would change system/generated files which are unlikely
 // to be good candidates for modification.
@@ -737,6 +758,7 @@
     return makeError(ReasonToReject::UnsupportedSymbol);
 
   auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
+  filterBaseUsingDecl(DeclsUnderCursor);
   if (DeclsUnderCursor.empty())
     return makeError(ReasonToReject::NoSymbolFound);
   if (DeclsUnderCursor.size() > 1)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135489.466181.patch
Type: text/x-patch
Size: 2444 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221007/f0a44301/attachment.bin>