[clangd] Use raw rename functions to implement the rename.
Summary:
The API provided by refactoring lib doesn't provide enough flexibility
to get clangd's rename to behave as we expect. Instead, we replace it
with the low-level rename functions, which give us more control.
Bonus:
- performance, previously we visit the TU to find all occurrences,
now we just visit top-level decls from main file;
- fix a bug where we wrongly filter out the main file replacement due to the
different relative/absolute file path;
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D65936
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@368429 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/clangd/refactor/Rename.cpp b/clangd/refactor/Rename.cpp
index b66eac4..845aae1 100644
--- a/clangd/refactor/Rename.cpp
+++ b/clangd/refactor/Rename.cpp
@@ -10,45 +10,15 @@
#include "AST.h"
#include "Logger.h"
#include "index/SymbolCollector.h"
-#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
+#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
+#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
namespace clang {
namespace clangd {
namespace {
-class RefactoringResultCollector final
- : public tooling::RefactoringResultConsumer {
-public:
- void handleError(llvm::Error Err) override {
- assert(!Result.hasValue());
- Result = std::move(Err);
- }
-
- // Using the handle(SymbolOccurrences) from parent class.
- using tooling::RefactoringResultConsumer::handle;
-
- void handle(tooling::AtomicChanges SourceReplacements) override {
- assert(!Result.hasValue());
- Result = std::move(SourceReplacements);
- }
-
- llvm::Optional<llvm::Expected<tooling::AtomicChanges>> Result;
-};
-
-// Expand a DiagnosticError to make it print-friendly (print the detailed
-// message, rather than "clang diagnostic").
-llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
- if (auto Diag = DiagnosticError::take(Err)) {
- llvm::cantFail(std::move(Err));
- SmallVector<char, 128> DiagMessage;
- Diag->second.EmitToString(DE, DiagMessage);
- return llvm::make_error<llvm::StringError>(DiagMessage,
- llvm::inconvertibleErrorCode());
- }
- return Err;
-}
-
llvm::Optional<std::string> filePath(const SymbolLocation &Loc,
llvm::StringRef HintFilePath) {
if (!Loc)
@@ -89,6 +59,7 @@
}
enum ReasonToReject {
+ NoSymbolFound,
NoIndexProvided,
NonIndexable,
UsedOutsideFile,
@@ -138,6 +109,8 @@
llvm::Error makeError(ReasonToReject Reason) {
auto Message = [](ReasonToReject Reason) {
switch (Reason) {
+ case NoSymbolFound:
+ return "there is no symbol at the given location";
case NoIndexProvided:
return "symbol may be used in other files (no index available)";
case UsedOutsideFile:
@@ -154,50 +127,62 @@
llvm::inconvertibleErrorCode());
}
+// Return all rename occurrences in the main file.
+tooling::SymbolOccurrences
+findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) {
+ const NamedDecl *CanonicalRenameDecl =
+ tooling::getCanonicalSymbolDeclaration(RenameDecl);
+ assert(CanonicalRenameDecl && "RenameDecl must be not null");
+ std::vector<std::string> RenameUSRs =
+ tooling::getUSRsForDeclaration(CanonicalRenameDecl, AST.getASTContext());
+ std::string OldName = CanonicalRenameDecl->getNameAsString();
+ tooling::SymbolOccurrences Result;
+ for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
+ tooling::SymbolOccurrences RenameInDecl =
+ tooling::getOccurrencesOfUSRs(RenameUSRs, OldName, TopLevelDecl);
+ Result.insert(Result.end(), std::make_move_iterator(RenameInDecl.begin()),
+ std::make_move_iterator(RenameInDecl.end()));
+ }
+ return Result;
+}
+
} // namespace
llvm::Expected<tooling::Replacements>
renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
llvm::StringRef NewName, const SymbolIndex *Index) {
- RefactoringResultCollector ResultCollector;
- ASTContext &ASTCtx = AST.getASTContext();
SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier(
AST, Pos, AST.getSourceManager().getMainFileID());
// FIXME: renaming macros is not supported yet, the macro-handling code should
// be moved to rename tooling library.
if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
return makeError(UnsupportedSymbol);
- tooling::RefactoringRuleContext Context(AST.getSourceManager());
- Context.setASTContext(ASTCtx);
- auto Rename = clang::tooling::RenameOccurrences::initiate(
- Context, SourceRange(SourceLocationBeg), NewName);
- if (!Rename)
- return expandDiagnostics(Rename.takeError(), ASTCtx.getDiagnostics());
- const auto *RenameDecl = Rename->getRenameDecl();
- assert(RenameDecl && "symbol must be found at this point");
+ const auto *RenameDecl =
+ tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg);
+ if (!RenameDecl)
+ return makeError(NoSymbolFound);
+
if (auto Reject =
renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index))
return makeError(*Reject);
- Rename->invoke(ResultCollector, Context);
-
- assert(ResultCollector.Result.hasValue());
- if (!ResultCollector.Result.getValue())
- return expandDiagnostics(ResultCollector.Result->takeError(),
- ASTCtx.getDiagnostics());
-
- tooling::Replacements FilteredChanges;
- // Right now we only support renaming the main file, so we
- // drop replacements not for the main file. In the future, we might
- // also support rename with wider scope.
// Rename sometimes returns duplicate edits (which is a bug). A side-effect of
// adding them to a single Replacements object is these are deduplicated.
- for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
- for (const auto &Rep : Change.getReplacements()) {
- if (Rep.getFilePath() == File)
- cantFail(FilteredChanges.add(Rep));
- }
+ tooling::Replacements FilteredChanges;
+ for (const tooling::SymbolOccurrence &Rename :
+ findOccurrencesWithinFile(AST, RenameDecl)) {
+ // Currently, we only support normal rename (one range) for C/C++.
+ // FIXME: support multiple-range rename for objective-c methods.
+ if (Rename.getNameRanges().size() > 1)
+ continue;
+ // We shouldn't have conflicting replacements. If there are conflicts, it
+ // means that we have bugs either in clangd or in Rename library, therefore
+ // we refuse to perform the rename.
+ if (auto Err = FilteredChanges.add(tooling::Replacement(
+ AST.getASTContext().getSourceManager(),
+ CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName)))
+ return std::move(Err);
}
return FilteredChanges;
}
diff --git a/clangd/test/rename.test b/clangd/test/rename.test
index 66c3f49..527b426 100644
--- a/clangd/test/rename.test
+++ b/clangd/test/rename.test
@@ -23,7 +23,7 @@
{"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32001,
-# CHECK-NEXT: "message": "there is no symbol at the given location"
+# CHECK-NEXT: "message": "Cannot rename symbol: there is no symbol at the given location"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 2,
# CHECK-NEXT: "jsonrpc": "2.0"
@@ -31,7 +31,7 @@
{"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32001,
-# CHECK-NEXT: "message": "there is no symbol at the given location"
+# CHECK-NEXT: "message": "Cannot rename symbol: there is no symbol at the given location"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 4,
# CHECK-NEXT: "jsonrpc": "2.0"
