[clang-tools-extra] b60428c - [clangd] Fix false positive in local rename collision detetction

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2021-02-09T11:51:18+01:00
New Revision: b60428c7ea685e338df3369807c9b650f994554b

URL: https://github.com/llvm/llvm-project/commit/b60428c7ea685e338df3369807c9b650f994554b
DIFF: https://github.com/llvm/llvm-project/commit/b60428c7ea685e338df3369807c9b650f994554b.diff

LOG: [clangd] Fix false positive in local rename collision detetction

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

Reviewed By: hokein

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 07422a06a923..6c117fc26273 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -421,6 +421,11 @@ const NamedDecl *lookupSiblingWithinEnclosingScope(ASTContext &Ctx,
     for (const auto *Parameter : EnclosingFunction->parameters())
       if (Parameter != &RenamedDecl && Parameter->getName() == NewName)
         return Parameter;
+    // FIXME: We don't modify all references to function parameters when
+    // renaming from forward declaration now, so using a name colliding with
+    // something in the definition's body is a valid transformation.
+    if (!EnclosingFunction->doesThisDeclarationHaveABody())
+      return nullptr;
     return CheckCompoundStmt(EnclosingFunction->getBody(), NewName);
   }
 

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 39104c675db2..f1f9e17a7b8b 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1083,12 +1083,23 @@ TEST(RenameTest, Renameable) {
        "conflict", !HeaderFile, nullptr, "Conflict"},
 
       {R"cpp(
+        void func(int Var);
+
         void func(int V^ar) {
           bool Conflict;
         }
       )cpp",
        "conflict", !HeaderFile, nullptr, "Conflict"},
 
+      {R"cpp(// No conflict: only forward declaration's argument is renamed.
+        void func(int [[V^ar]]);
+
+        void func(int Var) {
+          bool Conflict;
+        }
+      )cpp",
+       nullptr, !HeaderFile, nullptr, "Conflict"},
+
       {R"cpp(
         void func(int V^ar, int Conflict) {
         }