diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h --- a/clang-tools-extra/clangd/refactor/Rename.h +++ b/clang-tools-extra/clangd/refactor/Rename.h @@ -55,7 +55,8 @@ llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode, std::vector Occurrences, - llvm::StringRef NewName); + llvm::StringRef NewName, + llvm::StringRef OldName); /// Adjusts indexed occurrences to match the current state of the file. /// diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -374,7 +374,8 @@ llvm::inconvertibleErrorCode()); } auto RenameEdit = - buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName); + buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName, + RenameDecl.getNameAsString()); if (!RenameEdit) { return llvm::make_error( llvm::formatv("fail to build rename edit for file {0}: {1}", FilePath, @@ -522,7 +523,8 @@ llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode, std::vector Occurrences, - llvm::StringRef NewName) { + llvm::StringRef NewName, + llvm::StringRef OldName) { assert(std::is_sorted(Occurrences.begin(), Occurrences.end())); assert(std::unique(Occurrences.begin(), Occurrences.end()) == Occurrences.end() && @@ -557,7 +559,12 @@ auto EndOffset = Offset(R.end); if (!EndOffset) return EndOffset.takeError(); - OccurrencesOffsets.push_back({*StartOffset, *EndOffset}); + // FIXME: We should not generate invalid replacements in the first place, + // but it currently happens for implicit references and in some complicated + // cases where the might be incorrectly captured tokens. + if (*EndOffset > *StartOffset && + InitialCode.slice(*StartOffset, *EndOffset) == OldName) + OccurrencesOffsets.push_back({*StartOffset, *EndOffset}); } tooling::Replacements RenameEdit; @@ -600,10 +607,6 @@ assert(std::is_sorted(Indexed.begin(), Indexed.end())); assert(std::is_sorted(Lexed.begin(), Lexed.end())); - if (Indexed.size() > Lexed.size()) { - vlog("The number of lexed occurrences is less than indexed occurrences"); - return llvm::None; - } // Fast check for the special subset case. if (std::includes(Indexed.begin(), Indexed.end(), Lexed.begin(), Lexed.end())) return Indexed.vec(); diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -795,6 +795,7 @@ void func() { [[Foo]] foo; + foo.~[[Foo]]; } )cpp", }, @@ -868,6 +869,20 @@ } )cpp", }, + { + // Macros and implicit references. + R"cpp( + class [[Fo^o]] {}; + #define FooFoo Foo + )cpp", + R"cpp( + #include "foo.h" + void bar() { + [[Foo]] foo; + FooFoo z; + } + )cpp", + }, }; for (const auto& T : Cases) { @@ -920,7 +935,7 @@ auto LSPRange = Code.range(); llvm::StringRef FilePath = "/test/TestTU.cpp"; llvm::StringRef NewName = "abc"; - auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName); + auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName, "😂"); ASSERT_TRUE(bool(Edit)) << Edit.takeError(); ASSERT_EQ(1UL, Edit->Replacements.size()); EXPECT_EQ(FilePath, Edit->Replacements.begin()->getFilePath()); @@ -928,7 +943,7 @@ // Test invalid range. LSPRange.end = {10, 0}; // out of range - Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName); + Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName, "😂"); EXPECT_FALSE(Edit); EXPECT_THAT(llvm::toString(Edit.takeError()), testing::HasSubstr("fail to convert")); @@ -939,7 +954,7 @@ [[range]] [[range]] )cpp"); - Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName); + Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName, "range"); ASSERT_TRUE(bool(Edit)) << Edit.takeError(); EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second, expectedResult(T, NewName)); @@ -1013,11 +1028,6 @@ llvm::StringRef IndexedCode; llvm::StringRef LexedCode; } Tests[] = { - { - // no lexed ranges. - "[[]]", - "", - }, { // both line and column are changed, not a near miss. R"([[]])",