[PATCH] D91134: [clangd] Abort rename when given the same name

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 304423.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

- Change error message to description rather than suggestion.
- Move test to more appropriate location.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91134/new/

https://reviews.llvm.org/D91134

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
@@ -774,6 +774,13 @@
         }
       )cpp",
        nullptr, !HeaderFile, nullptr, "Conflict"},
+
+      {R"cpp(//
+        void func() {
+          int V^ar;
+        }
+      )cpp",
+       "new name is the same", !HeaderFile, nullptr, "Var"},
   };
 
   for (const auto& Case : Cases) {
@@ -876,7 +883,7 @@
 
   auto Results = runPrepareRename(Server, FooCCPath, FooCC.point(),
                                   /*NewName=*/llvm::None, {/*CrossFile=*/true});
-  // verify that for multi-file rename, we only return main-file occurrences.
+  // Verify that for multi-file rename, we only return main-file occurrences.
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   // We don't know the result is complete in prepareRename (passing a nullptr
   // index internally), so GlobalChanges should be empty.
@@ -884,7 +891,7 @@
   EXPECT_THAT(FooCC.ranges(),
               testing::UnorderedElementsAreArray(Results->LocalChanges));
 
-  // verify name validation.
+  // Name validation.
   Results =
       runPrepareRename(Server, FooCCPath, FooCC.point(),
                        /*NewName=*/std::string("int"), {/*CrossFile=*/true});
@@ -892,7 +899,7 @@
   EXPECT_THAT(llvm::toString(Results.takeError()),
               testing::HasSubstr("keyword"));
 
-  // single-file rename on global symbols, we should report an error.
+  // Single-file rename on global symbols, we should report an error.
   Results = runPrepareRename(Server, FooCCPath, FooCC.point(),
                              /*NewName=*/llvm::None, {/*CrossFile=*/false});
   EXPECT_FALSE(Results);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -123,6 +123,7 @@
 
   // name validation.
   RenameToKeywords,
+  SameName,
 };
 
 llvm::Optional<ReasonToReject> renameable(const NamedDecl &RenameDecl,
@@ -213,6 +214,8 @@
       return "there are multiple symbols at the given location";
     case ReasonToReject::RenameToKeywords:
       return "the chosen name is a keyword";
+    case ReasonToReject::SameName:
+      return "new name is the same as the old name";
     }
     llvm_unreachable("unhandled reason kind");
   };
@@ -558,6 +561,8 @@
     return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl =
       llvm::cast<NamedDecl>(*(*DeclsUnderCursor.begin())->getCanonicalDecl());
+  if (RenameDecl.getName() == RInputs.NewName)
+    return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)
     return makeError(*Invalid);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91134.304423.patch
Type: text/x-patch
Size: 2903 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201111/98288c1f/attachment-0001.bin>