[clangd] Abort rename when given the same name

When user wants to rename the symbol to the same name we shouldn't do any work.
Bail out early and return error to save compute.

Resolves: clangd/clangd#580

Reviewed By: hokein

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

Author: kirillbobyrev

clang-tools-extra/clangd/refactor/Rename.cpp

@@ -123,6 +123,7 @@ enum class ReasonToReject {
 
   // name validation.
   RenameToKeywords,
+  SameName,
 };
 
 llvm::Optional<ReasonToReject> renameable(const NamedDecl &RenameDecl,
@@ -213,6 +214,8 @@ llvm::Error makeError(ReasonToReject Reason) {
       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");
   };
@@ -556,6 +559,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
     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);

clang-tools-extra/clangd/unittests/RenameTests.cpp

@@ -774,6 +774,13 @@ TEST(RenameTest, Renameable) {
         }
       )cpp",
        nullptr, !HeaderFile, nullptr, "Conflict"},
+
+      {R"cpp(// Trying to rename into the same name, SameName == SameName.
+        void func() {
+          int S^ameName;
+        }
+      )cpp",
+       "new name is the same", !HeaderFile, nullptr, "SameName"},
   };
 
   for (const auto& Case : Cases) {
@@ -876,23 +883,23 @@ TEST(RenameTest, PrepareRename) {
 
   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.
   EXPECT_TRUE(Results->GlobalChanges.empty());
   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});
   EXPECT_FALSE(Results);
   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);