hokein created this revision.
hokein added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
This is the initial version. The cross-file rename is purely based on
the index (plus a text-match verification).
It is hidden under a command-line flag, and only available for a small set
of symbols.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D69263
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Rename.h
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SyncAPI.cpp
clang-tools-extra/clangd/unittests/SyncAPI.h
clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -41,9 +41,10 @@
const NamedDecl *getCanonicalSymbolDeclaration(const NamedDecl *FoundDecl) {
// If FoundDecl is a constructor or destructor, we want to instead take
// the Decl of the corresponding class.
- if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FoundDecl))
+ if (const auto *CtorDecl = dyn_cast_or_null<CXXConstructorDecl>(FoundDecl))
FoundDecl = CtorDecl->getParent();
- else if (const auto *DtorDecl = dyn_cast<CXXDestructorDecl>(FoundDecl))
+ else if (const auto *DtorDecl =
+ dyn_cast_or_null<CXXDestructorDecl>(FoundDecl))
FoundDecl = DtorDecl->getParent();
// FIXME: (Alex L): Canonicalize implicit template instantions, just like
// the indexer does it.
Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===================================================================
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
llvm::Expected<std::vector<DocumentHighlight>>
runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
-llvm::Expected<std::vector<TextEdit>>
-runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
+ Position Pos, StringRef NewName);
std::string runDumpAST(ClangdServer &Server, PathRef File);
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,10 +96,9 @@
return std::move(*Result);
}
-llvm::Expected<std::vector<TextEdit>> runRename(ClangdServer &Server,
- PathRef File, Position Pos,
- llvm::StringRef NewName) {
- llvm::Optional<llvm::Expected<std::vector<TextEdit>>> Result;
+llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
+ Position Pos, llvm::StringRef NewName) {
+ llvm::Optional<llvm::Expected<FileEdits>> Result;
Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
return std::move(*Result);
}
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -22,6 +22,19 @@
return replacementToEdit(Code, arg).range == Range;
}
+RenameInputs localRenameInputs(const Annotations &Code, llvm::StringRef NewName,
+ llvm::StringRef Path,
+ const SymbolIndex *Index = nullptr) {
+ RenameInputs Inputs;
+ Inputs.Pos = Code.point();
+ Inputs.MainFileCode = Code.code();
+ Inputs.MainFilePath = Path;
+ Inputs.NewName = NewName;
+ Inputs.Index = Index;
+ Inputs.AllowCrossFile = false;
+ return Inputs;
+}
+
TEST(RenameTest, SingleFile) {
struct Test {
const char* Before;
@@ -80,10 +93,13 @@
auto TU = TestTU::withCode(Code.code());
auto AST = TU.build();
auto RenameResult =
- renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde");
+ rename(AST, localRenameInputs(Code, "abcde", testPath(TU.Filename)));
+
ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
- auto ApplyResult =
- tooling::applyAllReplacements(Code.code(), *RenameResult);
+ ASSERT_TRUE(RenameResult->size() == 1);
+ EXPECT_EQ(Code.code(), RenameResult->begin()->second.InitialCode);
+ auto ApplyResult = tooling::applyAllReplacements(
+ Code.code(), RenameResult->begin()->second.Replacements);
ASSERT_TRUE(bool(ApplyResult)) << ApplyResult.takeError();
EXPECT_EQ(T.After, *ApplyResult) << T.Before;
@@ -177,24 +193,27 @@
}
auto AST = TU.build();
- auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(),
- "dummyNewName", Case.Index);
+ auto Results =
+ rename(AST, localRenameInputs(T, "dummyNewName", testPath(TU.Filename),
+ Case.Index));
bool WantRename = true;
if (T.ranges().empty())
WantRename = false;
if (!WantRename) {
assert(Case.ErrorMessage && "Error message must be set!");
- EXPECT_FALSE(Results) << "expected renameWithinFile returned an error: "
- << T.code();
+ EXPECT_FALSE(Results)
+ << "expected rename returned an error: " << T.code();
auto ActualMessage = llvm::toString(Results.takeError());
EXPECT_THAT(ActualMessage, testing::HasSubstr(Case.ErrorMessage));
} else {
- EXPECT_TRUE(bool(Results)) << "renameWithinFile returned an error: "
+ EXPECT_TRUE(bool(Results)) << "rename returned an error: "
<< llvm::toString(Results.takeError());
+ ASSERT_TRUE(Results->size() == 1);
std::vector<testing::Matcher<tooling::Replacement>> Expected;
for (const auto &R : T.ranges())
Expected.push_back(RenameRange(TU.Code, R));
- EXPECT_THAT(*Results, UnorderedElementsAreArray(Expected));
+ EXPECT_THAT(Results->begin()->second.Replacements,
+ UnorderedElementsAreArray(Expected));
}
}
}
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -263,6 +263,14 @@
CommaSeparated,
};
+opt<bool> CrossFileRename{
+ "cross-file-rename",
+ cat(Features),
+ desc("Enable cross-file rename fature. By default, "
+ "clangd only allows local rename."),
+ init(false),
+};
+
opt<unsigned> WorkerThreadsCount{
"j",
cat(Misc),
@@ -594,6 +602,7 @@
}
Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
+ Opts.CrossFileRename = CrossFileRename;
clangd::CodeCompleteOptions CCOpts;
CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/refactor/Tweak.h
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -75,7 +75,7 @@
llvm::Optional<std::string> ShowMessage;
/// A mapping from file path(the one used for accessing the underlying VFS)
/// to edits.
- llvm::StringMap<Edit> ApplyEdits;
+ FileEdits ApplyEdits;
static Effect showMessage(StringRef S) {
Effect E;
Index: clang-tools-extra/clangd/refactor/Rename.h
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_H
#include "Protocol.h"
+#include "SourceCode.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/Support/Error.h"
@@ -18,13 +19,23 @@
class ParsedAST;
class SymbolIndex;
+struct RenameInputs {
+ Position Pos; // the position triggering the rename
+ llvm::StringRef NewName;
+
+ llvm::StringRef MainFilePath;
+ llvm::StringRef MainFileCode;
+
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
+ const SymbolIndex *Index = nullptr;
+
+ bool AllowCrossFile = false; // true if enable cross-file rename
+};
+
/// Renames all occurrences of the symbol at \p Pos to \p NewName.
-/// Occurrences outside the current file are not modified.
-/// Returns an error if rename a symbol that's used in another file (per the
-/// index).
-llvm::Expected<tooling::Replacements>
-renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
- llvm::StringRef NewName, const SymbolIndex *Index = nullptr);
+/// If AllowCrossFile is false, returns an error if rename a symbol that's used
+/// in another file (per the index).
+llvm::Expected<FileEdits> rename(ParsedAST &AST, const RenameInputs &RInputs);
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -64,7 +64,7 @@
NoSymbolFound,
NoIndexProvided,
NonIndexable,
- UsedOutsideFile,
+ UsedOutsideFile, // for within-file rename only.
UnsupportedSymbol,
};
@@ -116,6 +116,50 @@
return ReasonToReject::UsedOutsideFile;
}
+llvm::Optional<ReasonToReject>
+renameableOutsideFile(const NamedDecl &RenameDecl, const SymbolIndex *Index) {
+ if (RenameDecl.getParentFunctionOrMethod())
+ return None; // function-local symbols
+
+ // Symbols in system headers are not renamable, as changing system headers is
+ // mostly undesired.
+ if (RenameDecl.getASTContext().getSourceManager().isInSystemHeader(
+ RenameDecl.getLocation()))
+ return ReasonToReject::UnsupportedSymbol;
+
+ // Disable renaming all non-identifier symbols, e.g. coversion operators
+ // user-defined literals.
+ auto II = RenameDecl.getDeclName().getAsIdentifierInfo();
+ if (!II || II->getName().empty())
+ return ReasonToReject::UnsupportedSymbol;
+
+ if (!Index)
+ return ReasonToReject::NoIndexProvided;
+ bool IsIndexable = SymbolCollector::shouldCollectSymbol(
+ RenameDecl, RenameDecl.getASTContext(), SymbolCollector::Options(),
+ /*IsMainFileSymbol=*/true);
+ if (!IsIndexable)
+ return ReasonToReject::NonIndexable;
+
+ // FIXME: disable templates rename. When renaming templates, all related
+ // specializations need to be considered, which is not supported in our index.
+ if (RenameDecl.getDescribedTemplate())
+ return ReasonToReject::UnsupportedSymbol;
+
+ // A whitelist of renamable symbols.
+ if (llvm::isa<CXXRecordDecl>(RenameDecl))
+ return None;
+ else if (const auto *S = llvm::dyn_cast<CXXMethodDecl>(&RenameDecl)) {
+ // FIXME: renaming virtual methods requires to rename all overridens in
+ // subclasses, which our index doesn't support yet.
+ if (!S->isVirtual())
+ return None;
+ } else if (isa<FunctionDecl>(&RenameDecl)) {
+ return None;
+ }
+ return UnsupportedSymbol;
+}
+
llvm::Error makeError(ReasonToReject Reason) {
auto Message = [](ReasonToReject Reason) {
switch (Reason) {
@@ -156,28 +200,9 @@
return Result;
}
-} // namespace
-
llvm::Expected<tooling::Replacements>
-renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
- llvm::StringRef NewName, const SymbolIndex *Index) {
- const SourceManager &SM = AST.getSourceManager();
- SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
- // 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);
-
- const auto *RenameDecl =
- tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg);
- if (!RenameDecl)
- return makeError(NoSymbolFound);
-
- if (auto Reject =
- renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index))
- return makeError(*Reject);
-
+renameWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl,
+ llvm::StringRef NewName) {
// Rename sometimes returns duplicate edits (which is a bug). A side-effect of
// adding them to a single Replacements object is these are deduplicated.
tooling::Replacements FilteredChanges;
@@ -198,5 +223,184 @@
return FilteredChanges;
}
+// Return all rename occurrences (per the index) outside of the main file.
+llvm::StringMap<std::vector<Range>>
+findOccurrencesOutsideFile(const NamedDecl *RenameDecl,
+ llvm::StringRef MainFile, const SymbolIndex *Index) {
+ if (!Index)
+ return {};
+ RefsRequest RQuest;
+ // FIXME: revisit the limit, and our index should have a way to inform
+ // us whether the ref results is completed.
+ RQuest.Limit = 100;
+ if (auto ID = getSymbolID(RenameDecl))
+ RQuest.IDs.insert(*ID);
+
+ // Helper.
+ auto ToRange = [](const SymbolLocation &L) {
+ Range R;
+ R.start.line = L.Start.line();
+ R.start.character = L.Start.column();
+ R.end.line = L.End.line();
+ R.end.character = L.End.column();
+ return R;
+ };
+ llvm::StringMap<std::vector<Range>>
+ AffectedFiles; // absolute file path => rename ocurrences in that file.
+ Index->refs(RQuest, [&](const Ref &R) {
+ if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
+ if (*RefFilePath != MainFile)
+ AffectedFiles[*RefFilePath].push_back(ToRange(R.Location));
+ }
+ });
+ return AffectedFiles;
+}
+
+// Run cross-file rename Per the index.
+llvm::Expected<FileEdits>
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+ llvm::StringRef NewName, const SymbolIndex *Index,
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+ auto AffectedFiles =
+ findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index);
+ // FIXME: make the limit customizable.
+ static constexpr size_t MaxLimitFiles = 50;
+ if (AffectedFiles.size() >= MaxLimitFiles)
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv(
+ "The number of affected files exceeds the max limit {0}: {1}",
+ MaxLimitFiles, AffectedFiles.size()),
+ llvm::inconvertibleErrorCode());
+
+ // Helpers.
+ auto ReadFileFromVfs = [&](llvm::StringRef FilePath)
+ -> llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>> {
+ auto File = VFS.get()->openFileForRead(FilePath);
+ if (!File)
+ return llvm::errorCodeToError(File.getError());
+ if (!*File)
+ return nullptr;
+ auto Stat = (*File)->status();
+ if (!Stat)
+ return llvm::errorCodeToError(Stat.getError());
+
+ auto FileContent = File->get()->getBuffer(Stat->getName());
+ if (!FileContent)
+ return llvm::errorCodeToError(FileContent.getError());
+ return std::move(*FileContent);
+ };
+ auto ToRangeOffset =
+ [](const clangd::Range &R,
+ llvm::StringRef Code) -> llvm::Expected<std::pair<size_t, size_t>> {
+ auto StartOffset = positionToOffset(Code, R.start);
+ if (!StartOffset) {
+ return StartOffset.takeError();
+ }
+ auto EndOffset = positionToOffset(Code, R.end);
+ if (!EndOffset) {
+ return EndOffset.takeError();
+ }
+ return std::make_pair<>(*StartOffset, *EndOffset);
+ };
+
+ // The cross-file rename is purely based on the index, as we don't want to
+ // build all ASTs for affected files, which may cause a performance hit.
+ // This is a correctness/performance tradeoff, we assume that the index is
+ // always update-to-date (or users should expect that the cross-file rename
+ // only works well when the bacground indexing is done).
+ //
+ // To mitigate the diverged issue of the index (e.g. xref may return implicit
+ // references which are non-eligible for rename), we do a post text-match
+ // verification, filtering out all non-matching occurrences.
+ FileEdits Results;
+ std::string OldName = RenameDecl->getNameAsString();
+ for (const auto &FileAndOccurrences : AffectedFiles) {
+ llvm::StringRef FilePath = FileAndOccurrences.first();
+
+ auto Content = ReadFileFromVfs(FilePath);
+ if (!Content) {
+ elog("Fail to read file {0}: {1}", FilePath, Content.takeError());
+ continue;
+ }
+ if (!*Content)
+ continue;
+ llvm::StringRef AffectedFileCode = (*Content)->getBuffer();
+
+ tooling::Replacements RenameEdit;
+
+ // Now we do a text-match verification.
+ for (const Range &Occurrence : FileAndOccurrences.getValue()) {
+ // !positionToOffset is O(N), it is okay at the moment since we only
+ // process at most 100 references.
+ auto RangeOffset = ToRangeOffset(Occurrence, AffectedFileCode);
+ if (!RangeOffset) {
+ elog("Error on transfer range to offset: {0} ",
+ RangeOffset.takeError());
+ continue;
+ }
+ auto ByteLength = RangeOffset->second - RangeOffset->first;
+ // FIXME: be more tolerant with the dirty buffer.
+ if (OldName != AffectedFileCode.substr(RangeOffset->first, ByteLength))
+ continue;
+
+ if (auto Err = RenameEdit.add(tooling::Replacement(
+ AffectedFileCode, RangeOffset->first, ByteLength, NewName)))
+ elog("Failed to add replacements: {0}", std::move(Err));
+ }
+ if (!RenameEdit.empty())
+ Results.try_emplace(FilePath, AffectedFileCode, std::move(RenameEdit));
+ }
+ return Results;
+}
+
+} // namespace
+
+llvm::Expected<FileEdits> rename(ParsedAST &AST, const RenameInputs &RInputs) {
+ const SourceManager &SM = AST.getSourceManager();
+ SourceLocation SourceLocationBeg =
+ SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+ RInputs.Pos, SM, AST.getASTContext().getLangOpts()));
+ // 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);
+
+ const NamedDecl *RenameDecl = tooling::getCanonicalSymbolDeclaration(
+ tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg));
+ if (!RenameDecl)
+ return makeError(NoSymbolFound);
+
+ if (!RInputs.AllowCrossFile) {
+ // Handle within-file rename.
+ auto Reject = renamableWithinFile(*RenameDecl->getCanonicalDecl(),
+ RInputs.MainFilePath, RInputs.Index);
+ if (Reject)
+ return makeError(*Reject);
+ auto MainFileRenameEdit =
+ renameWithinFile(AST, RenameDecl, RInputs.NewName);
+ if (!MainFileRenameEdit)
+ return MainFileRenameEdit.takeError();
+ FileEdits Results;
+ Results.try_emplace(RInputs.MainFilePath, RInputs.MainFileCode,
+ std::move(*MainFileRenameEdit));
+ return Results;
+ }
+
+ // Handle cross-file rename.
+ if (auto Reject = renameableOutsideFile(*RenameDecl, RInputs.Index))
+ return makeError(*Reject);
+ auto Results = renameOutsideFile(RenameDecl, RInputs.MainFilePath,
+ RInputs.NewName, RInputs.Index, RInputs.VFS);
+ if (!Results)
+ return Results.takeError();
+ auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName);
+ if (!MainFileRenameEdit)
+ // FIXME: should we report the error to client?
+ return MainFileRenameEdit.takeError();
+ Results->try_emplace(RInputs.MainFilePath, RInputs.MainFileCode,
+ std::move(*MainFileRenameEdit));
+ return Results;
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -263,10 +263,6 @@
Decl::FriendObjectKind::FOK_None) &&
!(Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
return true;
- // Skip non-semantic references, we should start processing these when we
- // decide to implement renaming with index support.
- if ((Roles & static_cast<unsigned>(index::SymbolRole::NameReference)))
- return true;
// A declaration created for a friend declaration should not be used as the
// canonical declaration in the index. Use OrigD instead, unless we've already
// picked a replacement for D
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -223,6 +223,7 @@
/// Checks whether the Replacements are applicable to given Code.
bool canApplyTo(llvm::StringRef Code) const;
};
+using FileEdits = llvm::StringMap<Edit>;
/// Formats the edits and code around it according to Style. Changes
/// Replacements to formatted ones if succeeds.
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -23,6 +23,7 @@
#include "index/Background.h"
#include "index/FileIndex.h"
#include "index/Index.h"
+#include "refactor/Rename.h"
#include "refactor/Tweak.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Core/Replacement.h"
@@ -132,6 +133,9 @@
/// Enable semantic highlighting features.
bool SemanticHighlighting = false;
+ /// Enable cross-file rename feature.
+ bool CrossFileRename = false;
+
/// Returns true if the tweak should be enabled.
std::function<bool(const Tweak &)> TweakFilter = [](const Tweak &T) {
return !T.hidden(); // only enable non-hidden tweaks.
@@ -251,7 +255,7 @@
/// embedders could use this method to get all occurrences of the symbol (e.g.
/// highlighting them in prepare stage).
void rename(PathRef File, Position Pos, llvm::StringRef NewName,
- bool WantFormat, Callback<std::vector<TextEdit>> CB);
+ bool WantFormat, Callback<FileEdits> CB);
struct TweakRef {
std::string ID; /// ID to pass for applyTweak.
@@ -326,6 +330,9 @@
// can be caused by missing includes (e.g. member access in incomplete type).
bool SuggestMissingIncludes = false;
+ // If this is true, enable cross-file rename.
+ bool CrossFileRename = false;
+
std::function<bool(const Tweak &)> TweakFilter;
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -119,7 +119,8 @@
: nullptr),
GetClangTidyOptions(Opts.GetClangTidyOptions),
SuggestMissingIncludes(Opts.SuggestMissingIncludes),
- TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
+ CrossFileRename(Opts.CrossFileRename), TweakFilter(Opts.TweakFilter),
+ WorkspaceRoot(Opts.WorkspaceRoot),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
// is parsed.
@@ -308,16 +309,28 @@
if (!InpAST)
return CB(InpAST.takeError());
auto &AST = InpAST->AST;
- // Performing the rename isn't substantially more expensive than doing an
- // AST-based check, so we just rename and throw away the results. We may
- // have to revisit this when we support cross-file rename.
- auto Changes = renameWithinFile(AST, File, Pos, "dummy", Index);
- if (!Changes) {
- // LSP says to return null on failure, but that will result in a generic
- // failure message. If we send an LSP error response, clients can surface
- // the message to users (VSCode does).
- return CB(Changes.takeError());
+ // FIXME: for cross-file rename, we presume prepareRename always succeeds,
+ // revisit this strategy.
+ if (!CrossFileRename) {
+ // Performing the local rename isn't substantially more expensive than
+ // doing an AST-based check, so we just rename and throw away the results.
+ RenameInputs RInputs;
+ RInputs.MainFileCode = InpAST->Inputs.Contents;
+ RInputs.MainFilePath = File;
+ RInputs.NewName = "dummy";
+ RInputs.Pos = Pos;
+ RInputs.AllowCrossFile = false;
+ RInputs.Index = Index;
+ RInputs.VFS = FSProvider.getFileSystem();
+ auto Changes = clangd::rename(AST, RInputs); // within-file rename.
+ if (!Changes) {
+ // LSP says to return null on failure, but that will result in a generic
+ // failure message. If we send an LSP error response, clients can
+ // surface the message to users (VSCode does).
+ return CB(Changes.takeError());
+ }
}
+
SourceLocation Loc = getBeginningOfIdentifier(
Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
if (auto Range = getTokenRange(AST.getSourceManager(),
@@ -330,32 +343,34 @@
}
void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
- bool WantFormat, Callback<std::vector<TextEdit>> CB) {
+ bool WantFormat, Callback<FileEdits> CB) {
auto Action = [File = File.str(), NewName = NewName.str(), Pos, WantFormat,
CB = std::move(CB),
this](llvm::Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Changes = renameWithinFile(InpAST->AST, File, Pos, NewName, Index);
- if (!Changes)
- return CB(Changes.takeError());
+ RenameInputs RInputs;
+ RInputs.MainFileCode = InpAST->Inputs.Contents;
+ RInputs.MainFilePath = File;
+ RInputs.NewName = NewName;
+ RInputs.Pos = Pos;
+ RInputs.AllowCrossFile = CrossFileRename;
+ RInputs.Index = Index;
+ RInputs.VFS = FSProvider.getFileSystem();
+ auto Edits = clangd::rename(InpAST->AST, RInputs);
+ if (!Edits)
+ return CB(Edits.takeError());
if (WantFormat) {
auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
InpAST->Inputs.FS.get());
- if (auto Formatted =
- cleanupAndFormat(InpAST->Inputs.Contents, *Changes, Style))
- *Changes = std::move(*Formatted);
- else
- elog("Failed to format replacements: {0}", Formatted.takeError());
+ for (auto &E : *Edits) {
+ if (auto Err = reformatEdit(E.getValue(), Style))
+ elog("Failed to format replacements: {0}", std::move(Err));
+ }
}
-
- std::vector<TextEdit> Edits;
- for (const auto &Rep : *Changes)
- Edits.push_back(replacementToEdit(InpAST->Inputs.Contents, Rep));
- return CB(std::move(Edits));
+ CB(std::move(*Edits));
};
-
WorkScheduler.runWithAST("Rename", File, std::move(Action));
}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -106,10 +106,10 @@
// Makes sure edits in \p E are applicable to latest file contents reported by
// editor. If not generates an error message containing information about files
// that needs to be saved.
-llvm::Error validateEdits(const DraftStore &DraftMgr, const Tweak::Effect &E) {
+llvm::Error validateEdits(const DraftStore &DraftMgr, const FileEdits &FE) {
size_t InvalidFileCount = 0;
llvm::StringRef LastInvalidFile;
- for (const auto &It : E.ApplyEdits) {
+ for (const auto &It : FE) {
if (auto Draft = DraftMgr.getDraft(It.first())) {
// If the file is open in user's editor, make sure the version we
// saw and current version are compatible as this is the text that
@@ -704,7 +704,7 @@
if (R->ApplyEdits.empty())
return Reply("Tweak applied.");
- if (auto Err = validateEdits(DraftMgr, *R))
+ if (auto Err = validateEdits(DraftMgr, R->ApplyEdits))
return Reply(std::move(Err));
WorkspaceEdit WE;
@@ -758,17 +758,23 @@
if (!Code)
return Reply(llvm::make_error<LSPError>(
"onRename called for non-added file", ErrorCode::InvalidParams));
-
- Server->rename(File, Params.position, Params.newName, /*WantFormat=*/true,
- [File, Code, Params, Reply = std::move(Reply)](
- llvm::Expected<std::vector<TextEdit>> Edits) mutable {
- if (!Edits)
- return Reply(Edits.takeError());
-
- WorkspaceEdit WE;
- WE.changes = {{Params.textDocument.uri.uri(), *Edits}};
- Reply(WE);
- });
+ Server->rename(
+ File, Params.position, Params.newName,
+ /*WantFormat=*/true,
+ [File, Code, Params, Reply = std::move(Reply),
+ this](llvm::Expected<FileEdits> Edits) mutable {
+ if (!Edits)
+ return Reply(Edits.takeError());
+ if (auto Err = validateEdits(DraftMgr, *Edits))
+ return Reply(std::move(Err));
+ WorkspaceEdit Result;
+ Result.changes.emplace();
+ for (const auto &Rep : *Edits) {
+ (*Result.changes)[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+ }
+ Reply(Result);
+ });
}
void ClangdLSPServer::onDocumentDidClose(
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
