Author: Haojian Wu Date: 2021-06-11T13:51:50+02:00 New Revision: d30c202d276db86d741734954d1957e7dbbf123c
URL: https://github.com/llvm/llvm-project/commit/d30c202d276db86d741734954d1957e7dbbf123c DIFF: https://github.com/llvm/llvm-project/commit/d30c202d276db86d741734954d1957e7dbbf123c.diff LOG: [clangd] don't rename if the triggering loc is not actually being renamed. See context: https://github.com/clangd/clangd/issues/765 Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D101816 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 5431046836ca8..2193626ae099e 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -751,6 +751,26 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) { auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName); if (!MainFileRenameEdit) return MainFileRenameEdit.takeError(); + + // Check the rename-triggering location is actually being renamed. + // This is a robustness check to avoid surprising rename results -- if the + // the triggering location is not actually the name of the node we identified + // (e.g. for broken code), then rename is likely not what users expect, so we + // reject this kind of rename. + auto StartOffset = positionToOffset(MainFileCode, CurrentIdentifier.start); + auto EndOffset = positionToOffset(MainFileCode, CurrentIdentifier.end); + if (!StartOffset) + return StartOffset.takeError(); + if (!EndOffset) + return EndOffset.takeError(); + if (llvm::find_if( + *MainFileRenameEdit, + [&StartOffset, &EndOffset](const clang::tooling::Replacement &R) { + return R.getOffset() == *StartOffset && + R.getLength() == *EndOffset - *StartOffset; + }) == MainFileRenameEdit->end()) { + return makeError(ReasonToReject::NoSymbolFound); + } RenameResult Result; Result.Target = CurrentIdentifier; Edit MainFileEdits = Edit(MainFileCode, std::move(*MainFileRenameEdit)); diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index f917e30cd7fe0..f062f91c94378 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1071,6 +1071,13 @@ TEST(RenameTest, Renameable) { struct B : priv^ate A {}; )cpp", "Cannot rename symbol: there is no symbol at the given location", false}, + {R"cpp(// Ensure it doesn't associate base specifier with base name. + /*error-ok*/ + struct A { + A() : inva^lid(0) {} + }; + )cpp", + "no symbol", false}, }; for (const auto& Case : Cases) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
