Author: Kirill Bobyrev Date: 2020-11-11T11:13:47+01:00 New Revision: 91ce6fb5a65f75c1b29c898f2cb76e2c5d1ac681
URL: https://github.com/llvm/llvm-project/commit/91ce6fb5a65f75c1b29c898f2cb76e2c5d1ac681 DIFF: https://github.com/llvm/llvm-project/commit/91ce6fb5a65f75c1b29c898f2cb76e2c5d1ac681.diff LOG: [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: https://github.com/clangd/clangd/issues/580 Reviewed By: hokein Differential Revision: https://reviews.llvm.org/D91134 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 5adcf6d0e86e..33a9c845beaa 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/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); diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index b1c0d1d5e2d9..daf63d25ed7f 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/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,7 +883,7 @@ 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. @@ -884,7 +891,7 @@ TEST(RenameTest, PrepareRename) { 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 @@ TEST(RenameTest, PrepareRename) { 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
