[clang-tools-extra] bca3e24 - [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 2 13:59:01 PST 2021
Author: Sam McCall
Date: 2021-03-02T22:58:50+01:00
New Revision: bca3e24139cc301d7deae56a020057cf657035b6
URL: https://github.com/llvm/llvm-project/commit/bca3e24139cc301d7deae56a020057cf657035b6
DIFF: https://github.com/llvm/llvm-project/commit/bca3e24139cc301d7deae56a020057cf657035b6.diff
LOG: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.
ClangdServer already gets notified of every change, so it makes sense for it to
be the source of truth.
This is a step towards having ClangdServer expose a FS that includes dirty
buffers: D94554
Related changes:
- version is now optional for ClangdServer, to preserve our existing fuzziness
in this area (missing version ==> autoincrement)
- ClangdServer::format{File,Range} are now more regular ClangdServer functions
that don't need the code passed in. While here, combine into one function.
- incremental content update logic is moved from DraftStore to
ClangdLSPServer, with most of the implementation in SourceCode.cpp.
DraftStore is now fairly trivial, and will probably ultimately be
*replaced* by the dirty FS stuff.
Differential Revision: https://reviews.llvm.org/D97738
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/DraftStore.cpp
clang-tools-extra/clangd/DraftStore.h
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/test/crash-non-added-files.test
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
clang-tools-extra/clangd/unittests/SyncAPI.cpp
clang-tools-extra/clangd/unittests/SyncAPI.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index f2ec8189b92c..0efc7124477a 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -59,8 +59,8 @@ constexpr trace::Metric LSPLatency("lsp_latency", trace::Metric::Distribution,
// LSP defines file versions as numbers that increase.
// ClangdServer treats them as opaque and therefore uses strings instead.
-std::string encodeVersion(int64_t LSPVersion) {
- return llvm::to_string(LSPVersion);
+std::string encodeVersion(llvm::Optional<int64_t> LSPVersion) {
+ return LSPVersion ? llvm::to_string(*LSPVersion) : "";
}
llvm::Optional<int64_t> decodeVersion(llvm::StringRef Encoded) {
int64_t Result;
@@ -124,15 +124,15 @@ CompletionItemKindBitset defaultCompletionItemKinds() {
// Makes sure edits in \p FE 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 FileEdits &FE) {
+llvm::Error validateEdits(const ClangdServer &Server, const FileEdits &FE) {
size_t InvalidFileCount = 0;
llvm::StringRef LastInvalidFile;
for (const auto &It : FE) {
- if (auto Draft = DraftMgr.getDraft(It.first())) {
+ if (auto Draft = Server.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
// will be replaced by editors.
- if (!It.second.canApplyTo(Draft->Contents)) {
+ if (!It.second.canApplyTo(*Draft)) {
++InvalidFileCount;
LastInvalidFile = It.first();
}
@@ -648,8 +648,8 @@ void ClangdLSPServer::onDocumentDidOpen(
const std::string &Contents = Params.textDocument.text;
- auto Version = DraftMgr.addDraft(File, Params.textDocument.version, Contents);
- Server->addDocument(File, Contents, encodeVersion(Version),
+ Server->addDocument(File, Contents,
+ encodeVersion(Params.textDocument.version),
WantDiagnostics::Yes);
}
@@ -661,25 +661,28 @@ void ClangdLSPServer::onDocumentDidChange(
: WantDiagnostics::No;
PathRef File = Params.textDocument.uri.file();
- llvm::Expected<DraftStore::Draft> Draft = DraftMgr.updateDraft(
- File, Params.textDocument.version, Params.contentChanges);
- if (!Draft) {
- // If this fails, we are most likely going to be not in sync anymore with
- // the client. It is better to remove the draft and let further operations
- // fail rather than giving wrong results.
- DraftMgr.removeDraft(File);
- Server->removeDocument(File);
- elog("Failed to update {0}: {1}", File, Draft.takeError());
+ auto Code = Server->getDraft(File);
+ if (!Code) {
+ log("Trying to incrementally change non-added document: {0}", File);
return;
}
-
- Server->addDocument(File, Draft->Contents, encodeVersion(Draft->Version),
+ for (const auto &Change : Params.contentChanges) {
+ if (auto Err = applyChange(*Code, Change)) {
+ // If this fails, we are most likely going to be not in sync anymore with
+ // the client. It is better to remove the draft and let further
+ // operations fail rather than giving wrong results.
+ Server->removeDocument(File);
+ elog("Failed to update {0}: {1}", File, std::move(Err));
+ return;
+ }
+ }
+ Server->addDocument(File, *Code, encodeVersion(Params.textDocument.version),
WantDiags, Params.forceRebuild);
}
void ClangdLSPServer::onDocumentDidSave(
const DidSaveTextDocumentParams &Params) {
- reparseOpenFilesIfNeeded([](llvm::StringRef) { return true; });
+ Server->reparseOpenFilesIfNeeded([](llvm::StringRef) { return true; });
}
void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
@@ -720,13 +723,8 @@ void ClangdLSPServer::onCommandApplyEdit(const WorkspaceEdit &WE,
void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args,
Callback<llvm::json::Value> Reply) {
- auto Code = DraftMgr.getDraft(Args.file.file());
- if (!Code)
- return Reply(error("trying to apply a code action for a non-added file"));
-
- auto Action = [this, Reply = std::move(Reply), File = Args.file,
- Code = std::move(*Code)](
- llvm::Expected<Tweak::Effect> R) mutable {
+ auto Action = [this, Reply = std::move(Reply),
+ File = Args.file](llvm::Expected<Tweak::Effect> R) mutable {
if (!R)
return Reply(R.takeError());
@@ -742,7 +740,7 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args,
if (R->ApplyEdits.empty())
return Reply("Tweak applied.");
- if (auto Err = validateEdits(DraftMgr, R->ApplyEdits))
+ if (auto Err = validateEdits(*Server, R->ApplyEdits))
return Reply(std::move(Err));
WorkspaceEdit WE;
@@ -808,7 +806,7 @@ void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
void ClangdLSPServer::onRename(const RenameParams &Params,
Callback<WorkspaceEdit> Reply) {
Path File = std::string(Params.textDocument.uri.file());
- if (!DraftMgr.getDraft(File))
+ if (!Server->getDraft(File))
return Reply(llvm::make_error<LSPError>(
"onRename called for non-added file", ErrorCode::InvalidParams));
Server->rename(
@@ -817,7 +815,7 @@ void ClangdLSPServer::onRename(const RenameParams &Params,
this](llvm::Expected<RenameResult> R) mutable {
if (!R)
return Reply(R.takeError());
- if (auto Err = validateEdits(DraftMgr, R->GlobalChanges))
+ if (auto Err = validateEdits(*Server, R->GlobalChanges))
return Reply(std::move(Err));
WorkspaceEdit Result;
Result.changes.emplace();
@@ -832,7 +830,6 @@ void ClangdLSPServer::onRename(const RenameParams &Params,
void ClangdLSPServer::onDocumentDidClose(
const DidCloseTextDocumentParams &Params) {
PathRef File = Params.textDocument.uri.file();
- DraftMgr.removeDraft(File);
Server->removeDocument(File);
{
@@ -857,52 +854,35 @@ void ClangdLSPServer::onDocumentOnTypeFormatting(
const DocumentOnTypeFormattingParams &Params,
Callback<std::vector<TextEdit>> Reply) {
auto File = Params.textDocument.uri.file();
- auto Code = DraftMgr.getDraft(File);
- if (!Code)
- return Reply(llvm::make_error<LSPError>(
- "onDocumentOnTypeFormatting called for non-added file",
- ErrorCode::InvalidParams));
-
- Server->formatOnType(File, Code->Contents, Params.position, Params.ch,
- std::move(Reply));
+ Server->formatOnType(File, Params.position, Params.ch, std::move(Reply));
}
void ClangdLSPServer::onDocumentRangeFormatting(
const DocumentRangeFormattingParams &Params,
Callback<std::vector<TextEdit>> Reply) {
auto File = Params.textDocument.uri.file();
- auto Code = DraftMgr.getDraft(File);
- if (!Code)
- return Reply(llvm::make_error<LSPError>(
- "onDocumentRangeFormatting called for non-added file",
- ErrorCode::InvalidParams));
-
- Server->formatRange(
- File, Code->Contents, Params.range,
- [Code = Code->Contents, Reply = std::move(Reply)](
- llvm::Expected<tooling::Replacements> Result) mutable {
- if (Result)
- Reply(replacementsToEdits(Code, Result.get()));
- else
- Reply(Result.takeError());
- });
+ auto Code = Server->getDraft(File);
+ Server->formatFile(File, Params.range,
+ [Code = std::move(Code), Reply = std::move(Reply)](
+ llvm::Expected<tooling::Replacements> Result) mutable {
+ if (Result)
+ Reply(replacementsToEdits(*Code, Result.get()));
+ else
+ Reply(Result.takeError());
+ });
}
void ClangdLSPServer::onDocumentFormatting(
const DocumentFormattingParams &Params,
Callback<std::vector<TextEdit>> Reply) {
auto File = Params.textDocument.uri.file();
- auto Code = DraftMgr.getDraft(File);
- if (!Code)
- return Reply(llvm::make_error<LSPError>(
- "onDocumentFormatting called for non-added file",
- ErrorCode::InvalidParams));
-
- Server->formatFile(File, Code->Contents,
- [Code = Code->Contents, Reply = std::move(Reply)](
+ auto Code = Server->getDraft(File);
+ Server->formatFile(File,
+ /*Rng=*/llvm::None,
+ [Code = std::move(Code), Reply = std::move(Reply)](
llvm::Expected<tooling::Replacements> Result) mutable {
if (Result)
- Reply(replacementsToEdits(Code, Result.get()));
+ Reply(replacementsToEdits(*Code, Result.get()));
else
Reply(Result.takeError());
});
@@ -978,11 +958,6 @@ static llvm::Optional<Command> asCommand(const CodeAction &Action) {
void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
Callback<llvm::json::Value> Reply) {
URIForFile File = Params.textDocument.uri;
- auto Code = DraftMgr.getDraft(File.file());
- if (!Code)
- return Reply(llvm::make_error<LSPError>(
- "onCodeAction called for non-added file", ErrorCode::InvalidParams));
-
// Checks whether a particular CodeActionKind is included in the response.
auto KindAllowed = [Only(Params.context.only)](llvm::StringRef Kind) {
if (Only.empty())
@@ -1005,8 +980,8 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
// Now enumerate the semantic code actions.
auto ConsumeActions =
- [Reply = std::move(Reply), File, Code = std::move(*Code),
- Selection = Params.range, FixIts = std::move(FixIts), this](
+ [Reply = std::move(Reply), File, Selection = Params.range,
+ FixIts = std::move(FixIts), this](
llvm::Expected<std::vector<ClangdServer::TweakRef>> Tweaks) mutable {
if (!Tweaks)
return Reply(Tweaks.takeError());
@@ -1246,7 +1221,7 @@ void ClangdLSPServer::applyConfiguration(
}
}
- reparseOpenFilesIfNeeded(
+ Server->reparseOpenFilesIfNeeded(
[&](llvm::StringRef File) { return ModifiedFiles.count(File) != 0; });
}
@@ -1557,17 +1532,17 @@ bool ClangdLSPServer::shouldRunCompletion(
const CompletionParams &Params) const {
if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter)
return true;
- auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
+ auto Code = Server->getDraft(Params.textDocument.uri.file());
if (!Code)
return true; // completion code will log the error for untracked doc.
- auto Offset = positionToOffset(Code->Contents, Params.position,
+ auto Offset = positionToOffset(*Code, Params.position,
/*AllowColumnsBeyondLineLength=*/false);
if (!Offset) {
vlog("could not convert position '{0}' to offset for file '{1}'",
Params.position, Params.textDocument.uri.file());
return true;
}
- return allowImplicitCompletion(Code->Contents, *Offset);
+ return allowImplicitCompletion(*Code, *Offset);
}
void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
@@ -1681,16 +1656,5 @@ void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
NotifyFileStatus(Status.render(File));
}
-void ClangdLSPServer::reparseOpenFilesIfNeeded(
- llvm::function_ref<bool(llvm::StringRef File)> Filter) {
- // Reparse only opened files that were modified.
- for (const Path &FilePath : DraftMgr.getActiveFiles())
- if (Filter(FilePath))
- if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
- Server->addDocument(FilePath, std::move(Draft->Contents),
- encodeVersion(Draft->Version),
- WantDiagnostics::Auto);
-}
-
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index eeabebbc23cb..4a6b4c41dc2f 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -194,12 +194,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
/// produce '->' and '::', respectively.
bool shouldRunCompletion(const CompletionParams &Params) const;
- /// Requests a reparse of currently opened files using their latest source.
- /// This will typically only rebuild if something other than the source has
- /// changed (e.g. the CDB yields
diff erent flags, or files included in the
- /// preamble have been modified).
- void reparseOpenFilesIfNeeded(
- llvm::function_ref<bool(llvm::StringRef File)> Filter);
void applyConfiguration(const ConfigurationSettings &Settings);
/// Runs profiling and exports memory usage metrics if tracing is enabled and
@@ -282,8 +276,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
BackgroundQueue::Stats PendingBackgroundIndexProgress;
/// LSP extension: skip WorkDoneProgressCreate, just send progress streams.
bool BackgroundIndexSkipCreate = false;
- // Store of the current versions of the open documents.
- DraftStore DraftMgr;
Options Opts;
// The CDB is created by the "initialize" LSP method.
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 245a2d081f93..cf6cce658416 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -194,13 +194,14 @@ ClangdServer::~ClangdServer() {
void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
llvm::StringRef Version,
WantDiagnostics WantDiags, bool ForceRebuild) {
+ std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents);
ParseOptions Opts;
// Compile command is set asynchronously during update, as it can be slow.
ParseInputs Inputs;
Inputs.TFS = &TFS;
Inputs.Contents = std::string(Contents);
- Inputs.Version = Version.str();
+ Inputs.Version = std::move(ActualVersion);
Inputs.ForceRebuild = ForceRebuild;
Inputs.Opts = std::move(Opts);
Inputs.Index = Index;
@@ -211,6 +212,23 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
BackgroundIdx->boostRelated(File);
}
+void ClangdServer::reparseOpenFilesIfNeeded(
+ llvm::function_ref<bool(llvm::StringRef File)> Filter) {
+ // Reparse only opened files that were modified.
+ for (const Path &FilePath : DraftMgr.getActiveFiles())
+ if (Filter(FilePath))
+ if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
+ addDocument(FilePath, std::move(Draft->Contents), Draft->Version,
+ WantDiagnostics::Auto);
+}
+
+llvm::Optional<std::string> ClangdServer::getDraft(PathRef File) const {
+ auto Draft = DraftMgr.getDraft(File);
+ if (!Draft)
+ return llvm::None;
+ return std::move(Draft->Contents);
+}
+
std::function<Context(PathRef)>
ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
Callbacks *Publish) {
@@ -288,7 +306,10 @@ ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
};
}
-void ClangdServer::removeDocument(PathRef File) { WorkScheduler->remove(File); }
+void ClangdServer::removeDocument(PathRef File) {
+ DraftMgr.removeDraft(File);
+ WorkScheduler->remove(File);
+}
void ClangdServer::codeComplete(PathRef File, Position Pos,
const clangd::CodeCompleteOptions &Opts,
@@ -372,31 +393,55 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
std::move(Action));
}
-void ClangdServer::formatRange(PathRef File, llvm::StringRef Code, Range Rng,
- Callback<tooling::Replacements> CB) {
- llvm::Expected<size_t> Begin = positionToOffset(Code, Rng.start);
- if (!Begin)
- return CB(Begin.takeError());
- llvm::Expected<size_t> End = positionToOffset(Code, Rng.end);
- if (!End)
- return CB(End.takeError());
- formatCode(File, Code, {tooling::Range(*Begin, *End - *Begin)},
- std::move(CB));
-}
-
-void ClangdServer::formatFile(PathRef File, llvm::StringRef Code,
+void ClangdServer::formatFile(PathRef File, llvm::Optional<Range> Rng,
Callback<tooling::Replacements> CB) {
- // Format everything.
- formatCode(File, Code, {tooling::Range(0, Code.size())}, std::move(CB));
+ auto Code = getDraft(File);
+ if (!Code)
+ return CB(llvm::make_error<LSPError>("trying to format non-added document",
+ ErrorCode::InvalidParams));
+ tooling::Range RequestedRange;
+ if (Rng) {
+ llvm::Expected<size_t> Begin = positionToOffset(*Code, Rng->start);
+ if (!Begin)
+ return CB(Begin.takeError());
+ llvm::Expected<size_t> End = positionToOffset(*Code, Rng->end);
+ if (!End)
+ return CB(End.takeError());
+ RequestedRange = tooling::Range(*Begin, *End - *Begin);
+ } else {
+ RequestedRange = tooling::Range(0, Code->size());
+ }
+
+ // Call clang-format.
+ auto Action = [File = File.str(), Code = std::move(*Code),
+ Ranges = std::vector<tooling::Range>{RequestedRange},
+ CB = std::move(CB), this]() mutable {
+ format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS);
+ tooling::Replacements IncludeReplaces =
+ format::sortIncludes(Style, Code, Ranges, File);
+ auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
+ if (!Changed)
+ return CB(Changed.takeError());
+
+ CB(IncludeReplaces.merge(format::reformat(
+ Style, *Changed,
+ tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
+ File)));
+ };
+ WorkScheduler->runQuick("Format", File, std::move(Action));
}
-void ClangdServer::formatOnType(PathRef File, llvm::StringRef Code,
- Position Pos, StringRef TriggerText,
+void ClangdServer::formatOnType(PathRef File, Position Pos,
+ StringRef TriggerText,
Callback<std::vector<TextEdit>> CB) {
- llvm::Expected<size_t> CursorPos = positionToOffset(Code, Pos);
+ auto Code = getDraft(File);
+ if (!Code)
+ return CB(llvm::make_error<LSPError>("trying to format non-added document",
+ ErrorCode::InvalidParams));
+ llvm::Expected<size_t> CursorPos = positionToOffset(*Code, Pos);
if (!CursorPos)
return CB(CursorPos.takeError());
- auto Action = [File = File.str(), Code = Code.str(),
+ auto Action = [File = File.str(), Code = std::move(*Code),
TriggerText = TriggerText.str(), CursorPos = *CursorPos,
CB = std::move(CB), this]() mutable {
auto Style = format::getStyle(format::DefaultFormatStyle, File,
@@ -616,27 +661,6 @@ void ClangdServer::switchSourceHeader(
WorkScheduler->runWithAST("SwitchHeaderSource", Path, std::move(Action));
}
-void ClangdServer::formatCode(PathRef File, llvm::StringRef Code,
- llvm::ArrayRef<tooling::Range> Ranges,
- Callback<tooling::Replacements> CB) {
- // Call clang-format.
- auto Action = [File = File.str(), Code = Code.str(), Ranges = Ranges.vec(),
- CB = std::move(CB), this]() mutable {
- format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS);
- tooling::Replacements IncludeReplaces =
- format::sortIncludes(Style, Code, Ranges, File);
- auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
- if (!Changed)
- return CB(Changed.takeError());
-
- CB(IncludeReplaces.merge(format::reformat(
- Style, *Changed,
- tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
- File)));
- };
- WorkScheduler->runQuick("Format", File, std::move(Action));
-}
-
void ClangdServer::findDocumentHighlights(
PathRef File, Position Pos, Callback<std::vector<DocumentHighlight>> CB) {
auto Action =
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 198395c4a8f3..970956d68499 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -12,6 +12,7 @@
#include "../clang-tidy/ClangTidyOptions.h"
#include "CodeComplete.h"
#include "ConfigProvider.h"
+#include "DraftStore.h"
#include "GlobalCompilationDatabase.h"
#include "Hover.h"
#include "Module.h"
@@ -176,7 +177,7 @@ class ClangdServer {
/// separate thread. When the parsing is complete, DiagConsumer passed in
/// constructor will receive onDiagnosticsReady callback.
/// Version identifies this snapshot and is propagated to ASTs, preambles,
- /// diagnostics etc built from it.
+ /// diagnostics etc built from it. If empty, a version number is generated.
void addDocument(PathRef File, StringRef Contents,
llvm::StringRef Version = "null",
WantDiagnostics WD = WantDiagnostics::Auto,
@@ -188,6 +189,13 @@ class ClangdServer {
/// An empty set of diagnostics will be delivered, with Version = "".
void removeDocument(PathRef File);
+ /// Requests a reparse of currently opened files using their latest source.
+ /// This will typically only rebuild if something other than the source has
+ /// changed (e.g. the CDB yields
diff erent flags, or files included in the
+ /// preamble have been modified).
+ void reparseOpenFilesIfNeeded(
+ llvm::function_ref<bool(llvm::StringRef File)> Filter);
+
/// Run code completion for \p File at \p Pos.
///
/// This method should only be called for currently tracked files.
@@ -253,18 +261,15 @@ class ClangdServer {
void findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback<ReferencesResult> CB);
- /// Run formatting for \p Rng inside \p File with content \p Code.
- void formatRange(PathRef File, StringRef Code, Range Rng,
- Callback<tooling::Replacements> CB);
-
- /// Run formatting for the whole \p File with content \p Code.
- void formatFile(PathRef File, StringRef Code,
+ /// Run formatting for the \p File with content \p Code.
+ /// If \p Rng is non-null, formats only that region.
+ void formatFile(PathRef File, llvm::Optional<Range> Rng,
Callback<tooling::Replacements> CB);
/// Run formatting after \p TriggerText was typed at \p Pos in \p File with
/// content \p Code.
- void formatOnType(PathRef File, StringRef Code, Position Pos,
- StringRef TriggerText, Callback<std::vector<TextEdit>> CB);
+ void formatOnType(PathRef File, Position Pos, StringRef TriggerText,
+ Callback<std::vector<TextEdit>> CB);
/// Test the validity of a rename operation.
///
@@ -334,6 +339,8 @@ class ClangdServer {
/// FIXME: those metrics might be useful too, we should add them.
llvm::StringMap<TUScheduler::FileStats> fileStats() const;
+ llvm::Optional<std::string> getDraft(PathRef File) const;
+
// Blocks the main thread until the server is idle. Only for use in tests.
// Returns false if the timeout expires.
// FIXME: various subcomponents each get the full timeout, so it's more of
@@ -345,10 +352,6 @@ class ClangdServer {
void profile(MemoryTree &MT) const;
private:
- void formatCode(PathRef File, llvm::StringRef Code,
- ArrayRef<tooling::Range> Ranges,
- Callback<tooling::Replacements> CB);
-
ModuleSet *Modules;
const GlobalCompilationDatabase &CDB;
const ThreadsafeFS &TFS;
@@ -377,6 +380,11 @@ class ClangdServer {
llvm::Optional<std::string> WorkspaceRoot;
llvm::Optional<TUScheduler> WorkScheduler;
+
+ // Store of the current versions of the open documents.
+ // Only written from the main thread (despite being threadsafe).
+ // FIXME: TUScheduler also keeps these, unify?
+ DraftStore DraftMgr;
};
} // namespace clangd
diff --git a/clang-tools-extra/clangd/DraftStore.cpp b/clang-tools-extra/clangd/DraftStore.cpp
index 1299efbfba9f..97ac9eeaf89f 100644
--- a/clang-tools-extra/clangd/DraftStore.cpp
+++ b/clang-tools-extra/clangd/DraftStore.cpp
@@ -9,6 +9,7 @@
#include "DraftStore.h"
#include "SourceCode.h"
#include "support/Logger.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Errc.h"
namespace clang {
@@ -34,21 +35,44 @@ std::vector<Path> DraftStore::getActiveFiles() const {
return ResultVector;
}
+static void increment(std::string &S) {
+ // Ensure there is a numeric suffix.
+ if (S.empty() || !llvm::isDigit(S.back())) {
+ S.push_back('0');
+ return;
+ }
+ // Increment the numeric suffix.
+ auto I = S.rbegin(), E = S.rend();
+ for (;;) {
+ if (I == E || !llvm::isDigit(*I)) {
+ // Reached start of numeric section, it was all 9s.
+ S.insert(I.base(), '1');
+ break;
+ }
+ if (*I != '9') {
+ // Found a digit we can increment, we're done.
+ ++*I;
+ break;
+ }
+ *I = '0'; // and keep incrementing to the left.
+ }
+}
+
static void updateVersion(DraftStore::Draft &D,
- llvm::Optional<int64_t> Version) {
- if (Version) {
+ llvm::StringRef SpecifiedVersion) {
+ if (!SpecifiedVersion.empty()) {
// We treat versions as opaque, but the protocol says they increase.
- if (*Version <= D.Version)
- log("File version went from {0} to {1}", D.Version, Version);
- D.Version = *Version;
+ if (SpecifiedVersion.compare_numeric(D.Version) <= 0)
+ log("File version went from {0} to {1}", D.Version, SpecifiedVersion);
+ D.Version = SpecifiedVersion.str();
} else {
- // Note that if D was newly-created, this will bump D.Version from -1 to 0.
- ++D.Version;
+ // Note that if D was newly-created, this will bump D.Version from "" to 1.
+ increment(D.Version);
}
}
-int64_t DraftStore::addDraft(PathRef File, llvm::Optional<int64_t> Version,
- llvm::StringRef Contents) {
+std::string DraftStore::addDraft(PathRef File, llvm::StringRef Version,
+ llvm::StringRef Contents) {
std::lock_guard<std::mutex> Lock(Mutex);
Draft &D = Drafts[File];
@@ -57,74 +81,6 @@ int64_t DraftStore::addDraft(PathRef File, llvm::Optional<int64_t> Version,
return D.Version;
}
-llvm::Expected<DraftStore::Draft> DraftStore::updateDraft(
- PathRef File, llvm::Optional<int64_t> Version,
- llvm::ArrayRef<TextDocumentContentChangeEvent> Changes) {
- std::lock_guard<std::mutex> Lock(Mutex);
-
- auto EntryIt = Drafts.find(File);
- if (EntryIt == Drafts.end()) {
- return error(llvm::errc::invalid_argument,
- "Trying to do incremental update on non-added document: {0}",
- File);
- }
- Draft &D = EntryIt->second;
- std::string Contents = EntryIt->second.Contents;
-
- for (const TextDocumentContentChangeEvent &Change : Changes) {
- if (!Change.range) {
- Contents = Change.text;
- continue;
- }
-
- const Position &Start = Change.range->start;
- llvm::Expected<size_t> StartIndex =
- positionToOffset(Contents, Start, false);
- if (!StartIndex)
- return StartIndex.takeError();
-
- const Position &End = Change.range->end;
- llvm::Expected<size_t> EndIndex = positionToOffset(Contents, End, false);
- if (!EndIndex)
- return EndIndex.takeError();
-
- if (*EndIndex < *StartIndex)
- return error(llvm::errc::invalid_argument,
- "Range's end position ({0}) is before start position ({1})",
- End, Start);
-
- // Since the range length between two LSP positions is dependent on the
- // contents of the buffer we compute the range length between the start and
- // end position ourselves and compare it to the range length of the LSP
- // message to verify the buffers of the client and server are in sync.
-
- // EndIndex and StartIndex are in bytes, but Change.rangeLength is in UTF-16
- // code units.
- ssize_t ComputedRangeLength =
- lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex));
-
- if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength)
- return error(llvm::errc::invalid_argument,
- "Change's rangeLength ({0}) doesn't match the "
- "computed range length ({1}).",
- *Change.rangeLength, ComputedRangeLength);
-
- std::string NewContents;
- NewContents.reserve(*StartIndex + Change.text.length() +
- (Contents.length() - *EndIndex));
-
- NewContents = Contents.substr(0, *StartIndex);
- NewContents += Change.text;
- NewContents += Contents.substr(*EndIndex);
-
- Contents = std::move(NewContents);
- }
-
- updateVersion(D, Version);
- D.Contents = std::move(Contents);
- return D;
-}
-
void DraftStore::removeDraft(PathRef File) {
std::lock_guard<std::mutex> Lock(Mutex);
diff --git a/clang-tools-extra/clangd/DraftStore.h b/clang-tools-extra/clangd/DraftStore.h
index 3c2d0c6a4b0f..e997272bd6b4 100644
--- a/clang-tools-extra/clangd/DraftStore.h
+++ b/clang-tools-extra/clangd/DraftStore.h
@@ -21,15 +21,14 @@ namespace clang {
namespace clangd {
/// A thread-safe container for files opened in a workspace, addressed by
-/// filenames. The contents are owned by the DraftStore. This class supports
-/// both whole and incremental updates of the documents.
-/// Each time a draft is updated, it is assigned a version number. This can be
+/// filenames. The contents are owned by the DraftStore.
+/// Each time a draft is updated, it is assigned a version. This can be
/// specified by the caller or incremented from the previous version.
class DraftStore {
public:
struct Draft {
std::string Contents;
- int64_t Version = -1;
+ std::string Version;
};
/// \return Contents of the stored document.
@@ -40,21 +39,10 @@ class DraftStore {
std::vector<Path> getActiveFiles() const;
/// Replace contents of the draft for \p File with \p Contents.
- /// If no version is specified, one will be automatically assigned.
+ /// If version is empty, one will be automatically assigned.
/// Returns the version.
- int64_t addDraft(PathRef File, llvm::Optional<int64_t> Version,
- StringRef Contents);
-
- /// Update the contents of the draft for \p File based on \p Changes.
- /// If a position in \p Changes is invalid (e.g. out-of-range), the
- /// draft is not modified.
- /// If no version is specified, one will be automatically assigned.
- ///
- /// \return The new version of the draft for \p File, or an error if the
- /// changes couldn't be applied.
- llvm::Expected<Draft>
- updateDraft(PathRef File, llvm::Optional<int64_t> Version,
- llvm::ArrayRef<TextDocumentContentChangeEvent> Changes);
+ std::string addDraft(PathRef File, llvm::StringRef Version,
+ StringRef Contents);
/// Remove the draft from the store.
void removeDraft(PathRef File);
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index c0ccf2152750..14cde561474e 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -1053,6 +1053,56 @@ llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style) {
return llvm::Error::success();
}
+llvm::Error applyChange(std::string &Contents,
+ const TextDocumentContentChangeEvent &Change) {
+ if (!Change.range) {
+ Contents = Change.text;
+ return llvm::Error::success();
+ }
+
+ const Position &Start = Change.range->start;
+ llvm::Expected<size_t> StartIndex = positionToOffset(Contents, Start, false);
+ if (!StartIndex)
+ return StartIndex.takeError();
+
+ const Position &End = Change.range->end;
+ llvm::Expected<size_t> EndIndex = positionToOffset(Contents, End, false);
+ if (!EndIndex)
+ return EndIndex.takeError();
+
+ if (*EndIndex < *StartIndex)
+ return error(llvm::errc::invalid_argument,
+ "Range's end position ({0}) is before start position ({1})",
+ End, Start);
+
+ // Since the range length between two LSP positions is dependent on the
+ // contents of the buffer we compute the range length between the start and
+ // end position ourselves and compare it to the range length of the LSP
+ // message to verify the buffers of the client and server are in sync.
+
+ // EndIndex and StartIndex are in bytes, but Change.rangeLength is in UTF-16
+ // code units.
+ ssize_t ComputedRangeLength =
+ lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex));
+
+ if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength)
+ return error(llvm::errc::invalid_argument,
+ "Change's rangeLength ({0}) doesn't match the "
+ "computed range length ({1}).",
+ *Change.rangeLength, ComputedRangeLength);
+
+ std::string NewContents;
+ NewContents.reserve(*StartIndex + Change.text.length() +
+ (Contents.length() - *EndIndex));
+
+ NewContents = Contents.substr(0, *StartIndex);
+ NewContents += Change.text;
+ NewContents += Contents.substr(*EndIndex);
+
+ std::swap(Contents, NewContents);
+ return llvm::Error::success();
+}
+
EligibleRegion getEligiblePoints(llvm::StringRef Code,
llvm::StringRef FullyQualifiedName,
const LangOptions &LangOpts) {
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index be78e2f86436..7628fe138d93 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -203,6 +203,10 @@ using FileEdits = llvm::StringMap<Edit>;
/// Replacements to formatted ones if succeeds.
llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style);
+/// Apply an incremental update to a text document.
+llvm::Error applyChange(std::string &Contents,
+ const TextDocumentContentChangeEvent &Change);
+
/// Collects identifiers with counts in the source code.
llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
const format::FormatStyle &Style);
diff --git a/clang-tools-extra/clangd/test/crash-non-added-files.test b/clang-tools-extra/clangd/test/crash-non-added-files.test
index d86f7d26d870..55736bb715b0 100644
--- a/clang-tools-extra/clangd/test/crash-non-added-files.test
+++ b/clang-tools-extra/clangd/test/crash-non-added-files.test
@@ -4,28 +4,28 @@
{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32602
-# CHECK-NEXT: "message": "onCodeAction called for non-added file"
+# CHECK-NEXT: "message": "trying to get AST for non-added document"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 2,
---
{"jsonrpc":"2.0","id":3,"method":"textDocument/rangeFormatting","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":1,"character":4},"end":{"line":1,"character":12}},"options":{"tabSize":4,"insertSpaces":true}}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32602
-# CHECK-NEXT: "message": "onDocumentRangeFormatting called for non-added file"
+# CHECK-NEXT: "message": "trying to format non-added document"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 3,
---
{"jsonrpc":"2.0","id":4,"method":"textDocument/formatting","params":{"textDocument":{"uri":"test:///foo.c"},"options":{"tabSize":4,"insertSpaces":true}}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32602
-# CHECK-NEXT: "message": "onDocumentFormatting called for non-added file"
+# CHECK-NEXT: "message": "trying to format non-added document"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 4,
---
{"jsonrpc":"2.0","id":5,"method":"textDocument/onTypeFormatting","params":{"textDocument":{"uri":"test:///foo.c"},"position":{"line":3,"character":1},"ch":"}","options":{"tabSize":4,"insertSpaces":true}}}
# CHECK: "error": {
# CHECK-NEXT: "code": -32602
-# CHECK-NEXT: "message": "onDocumentOnTypeFormatting called for non-added file"
+# CHECK-NEXT: "message": "trying to format non-added document"
# CHECK-NEXT: },
# CHECK-NEXT: "id": 5,
---
diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp
index f10a994fca5c..15320e8bd8e8 100644
--- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -945,7 +945,7 @@ void f() {}
FS.Files[Path] = Code;
runAddDocument(Server, Path, Code);
- auto Replaces = runFormatFile(Server, Path, Code);
+ auto Replaces = runFormatFile(Server, Path, /*Rng=*/llvm::None);
EXPECT_TRUE(static_cast<bool>(Replaces));
auto Changed = tooling::applyAllReplacements(Code, *Replaces);
EXPECT_TRUE(static_cast<bool>(Changed));
diff --git a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
index 5ff9f254b55d..a6ed5d4436a0 100644
--- a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
@@ -9,6 +9,7 @@
#include "Annotations.h"
#include "DraftStore.h"
#include "SourceCode.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Testing/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -17,371 +18,26 @@ namespace clang {
namespace clangd {
namespace {
-struct IncrementalTestStep {
- llvm::StringRef Src;
- llvm::StringRef Contents;
-};
-
-int rangeLength(llvm::StringRef Code, const Range &Rng) {
- llvm::Expected<size_t> Start = positionToOffset(Code, Rng.start);
- llvm::Expected<size_t> End = positionToOffset(Code, Rng.end);
- assert(Start);
- assert(End);
- return *End - *Start;
-}
-
-/// Send the changes one by one to updateDraft, verify the intermediate results.
-void stepByStep(llvm::ArrayRef<IncrementalTestStep> Steps) {
- DraftStore DS;
- Annotations InitialSrc(Steps.front().Src);
- constexpr llvm::StringLiteral Path("/hello.cpp");
-
- // Set the initial content.
- EXPECT_EQ(0, DS.addDraft(Path, llvm::None, InitialSrc.code()));
-
- for (size_t i = 1; i < Steps.size(); i++) {
- Annotations SrcBefore(Steps[i - 1].Src);
- Annotations SrcAfter(Steps[i].Src);
- llvm::StringRef Contents = Steps[i - 1].Contents;
- TextDocumentContentChangeEvent Event{
- SrcBefore.range(),
- rangeLength(SrcBefore.code(), SrcBefore.range()),
- Contents.str(),
- };
-
- llvm::Expected<DraftStore::Draft> Result =
- DS.updateDraft(Path, llvm::None, {Event});
- ASSERT_TRUE(!!Result);
- EXPECT_EQ(Result->Contents, SrcAfter.code());
- EXPECT_EQ(DS.getDraft(Path)->Contents, SrcAfter.code());
- EXPECT_EQ(Result->Version, static_cast<int64_t>(i));
- }
-}
-
-/// Send all the changes at once to updateDraft, check only the final result.
-void allAtOnce(llvm::ArrayRef<IncrementalTestStep> Steps) {
- DraftStore DS;
- Annotations InitialSrc(Steps.front().Src);
- Annotations FinalSrc(Steps.back().Src);
- constexpr llvm::StringLiteral Path("/hello.cpp");
- std::vector<TextDocumentContentChangeEvent> Changes;
-
- for (size_t i = 0; i < Steps.size() - 1; i++) {
- Annotations Src(Steps[i].Src);
- llvm::StringRef Contents = Steps[i].Contents;
-
- Changes.push_back({
- Src.range(),
- rangeLength(Src.code(), Src.range()),
- Contents.str(),
- });
- }
-
- // Set the initial content.
- EXPECT_EQ(0, DS.addDraft(Path, llvm::None, InitialSrc.code()));
-
- llvm::Expected<DraftStore::Draft> Result =
- DS.updateDraft(Path, llvm::None, Changes);
-
- ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError());
- EXPECT_EQ(Result->Contents, FinalSrc.code());
- EXPECT_EQ(DS.getDraft(Path)->Contents, FinalSrc.code());
- EXPECT_EQ(Result->Version, 1);
-}
-
-TEST(DraftStoreIncrementalUpdateTest, Simple) {
- // clang-format off
- IncrementalTestStep Steps[] =
- {
- // Replace a range
- {
-R"cpp(static int
-hello[[World]]()
-{})cpp",
- "Universe"
- },
- // Delete a range
- {
-R"cpp(static int
-hello[[Universe]]()
-{})cpp",
- ""
- },
- // Add a range
- {
-R"cpp(static int
-hello[[]]()
-{})cpp",
- "Monde"
- },
- {
-R"cpp(static int
-helloMonde()
-{})cpp",
- ""
- }
- };
- // clang-format on
-
- stepByStep(Steps);
- allAtOnce(Steps);
-}
-
-TEST(DraftStoreIncrementalUpdateTest, MultiLine) {
- // clang-format off
- IncrementalTestStep Steps[] =
- {
- // Replace a range
- {
-R"cpp(static [[int
-helloWorld]]()
-{})cpp",
-R"cpp(char
-welcome)cpp"
- },
- // Delete a range
- {
-R"cpp(static char[[
-welcome]]()
-{})cpp",
- ""
- },
- // Add a range
- {
-R"cpp(static char[[]]()
-{})cpp",
- R"cpp(
-cookies)cpp"
- },
- // Replace the whole file
- {
-R"cpp([[static char
-cookies()
-{}]])cpp",
- R"cpp(#include <stdio.h>
-)cpp"
- },
- // Delete the whole file
- {
- R"cpp([[#include <stdio.h>
-]])cpp",
- "",
- },
- // Add something to an empty file
- {
- "[[]]",
- R"cpp(int main() {
-)cpp",
- },
- {
- R"cpp(int main() {
-)cpp",
- ""
- }
- };
- // clang-format on
-
- stepByStep(Steps);
- allAtOnce(Steps);
-}
-
-TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- DS.addDraft(File, llvm::None, "int main() {}\n");
-
- TextDocumentContentChangeEvent Change;
- Change.range.emplace();
- Change.range->start.line = 0;
- Change.range->start.character = 0;
- Change.range->end.line = 0;
- Change.range->end.character = 2;
- Change.rangeLength = 10;
-
- Expected<DraftStore::Draft> Result =
- DS.updateDraft(File, llvm::None, {Change});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(
- toString(Result.takeError()),
- "Change's rangeLength (10) doesn't match the computed range length (2).");
-}
-
-TEST(DraftStoreIncrementalUpdateTest, EndBeforeStart) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- DS.addDraft(File, llvm::None, "int main() {}\n");
-
- TextDocumentContentChangeEvent Change;
- Change.range.emplace();
- Change.range->start.line = 0;
- Change.range->start.character = 5;
- Change.range->end.line = 0;
- Change.range->end.character = 3;
-
- auto Result = DS.updateDraft(File, llvm::None, {Change});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(toString(Result.takeError()),
- "Range's end position (0:3) is before start position (0:5)");
-}
-
-TEST(DraftStoreIncrementalUpdateTest, StartCharOutOfRange) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- DS.addDraft(File, llvm::None, "int main() {}\n");
-
- TextDocumentContentChangeEvent Change;
- Change.range.emplace();
- Change.range->start.line = 0;
- Change.range->start.character = 100;
- Change.range->end.line = 0;
- Change.range->end.character = 100;
- Change.text = "foo";
-
- auto Result = DS.updateDraft(File, llvm::None, {Change});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(toString(Result.takeError()),
- "utf-16 offset 100 is invalid for line 0");
-}
-
-TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- DS.addDraft(File, llvm::None, "int main() {}\n");
-
- TextDocumentContentChangeEvent Change;
- Change.range.emplace();
- Change.range->start.line = 0;
- Change.range->start.character = 0;
- Change.range->end.line = 0;
- Change.range->end.character = 100;
- Change.text = "foo";
-
- auto Result = DS.updateDraft(File, llvm::None, {Change});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(toString(Result.takeError()),
- "utf-16 offset 100 is invalid for line 0");
-}
-
-TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- DS.addDraft(File, llvm::None, "int main() {}\n");
-
- TextDocumentContentChangeEvent Change;
- Change.range.emplace();
- Change.range->start.line = 100;
- Change.range->start.character = 0;
- Change.range->end.line = 100;
- Change.range->end.character = 0;
- Change.text = "foo";
-
- auto Result = DS.updateDraft(File, llvm::None, {Change});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(toString(Result.takeError()), "Line value is out of range (100)");
-}
-
-TEST(DraftStoreIncrementalUpdateTest, EndLineOutOfRange) {
+TEST(DraftStore, Versions) {
DraftStore DS;
Path File = "foo.cpp";
- DS.addDraft(File, llvm::None, "int main() {}\n");
+ EXPECT_EQ("25", DS.addDraft(File, "25", ""));
+ EXPECT_EQ("25", DS.getDraft(File)->Version);
+ EXPECT_EQ("", DS.getDraft(File)->Contents);
- TextDocumentContentChangeEvent Change;
- Change.range.emplace();
- Change.range->start.line = 0;
- Change.range->start.character = 0;
- Change.range->end.line = 100;
- Change.range->end.character = 0;
- Change.text = "foo";
+ EXPECT_EQ("26", DS.addDraft(File, "", "x"));
+ EXPECT_EQ("26", DS.getDraft(File)->Version);
+ EXPECT_EQ("x", DS.getDraft(File)->Contents);
- auto Result = DS.updateDraft(File, llvm::None, {Change});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(toString(Result.takeError()), "Line value is out of range (100)");
-}
-
-/// Check that if a valid change is followed by an invalid change, the original
-/// version of the document (prior to all changes) is kept.
-TEST(DraftStoreIncrementalUpdateTest, InvalidRangeInASequence) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- StringRef OriginalContents = "int main() {}\n";
- EXPECT_EQ(0, DS.addDraft(File, llvm::None, OriginalContents));
-
- // The valid change
- TextDocumentContentChangeEvent Change1;
- Change1.range.emplace();
- Change1.range->start.line = 0;
- Change1.range->start.character = 0;
- Change1.range->end.line = 0;
- Change1.range->end.character = 0;
- Change1.text = "Hello ";
-
- // The invalid change
- TextDocumentContentChangeEvent Change2;
- Change2.range.emplace();
- Change2.range->start.line = 0;
- Change2.range->start.character = 5;
- Change2.range->end.line = 0;
- Change2.range->end.character = 100;
- Change2.text = "something";
-
- auto Result = DS.updateDraft(File, llvm::None, {Change1, Change2});
-
- EXPECT_TRUE(!Result);
- EXPECT_EQ(toString(Result.takeError()),
- "utf-16 offset 100 is invalid for line 0");
-
- Optional<DraftStore::Draft> Contents = DS.getDraft(File);
- EXPECT_TRUE(Contents);
- EXPECT_EQ(Contents->Contents, OriginalContents);
- EXPECT_EQ(Contents->Version, 0);
-}
-
-TEST(DraftStore, Version) {
- DraftStore DS;
- Path File = "foo.cpp";
-
- EXPECT_EQ(25, DS.addDraft(File, 25, ""));
- EXPECT_EQ(25, DS.getDraft(File)->Version);
-
- EXPECT_EQ(26, DS.addDraft(File, llvm::None, ""));
- EXPECT_EQ(26, DS.getDraft(File)->Version);
+ EXPECT_EQ("27", DS.addDraft(File, "", "x")) << "no-op change";
+ EXPECT_EQ("27", DS.getDraft(File)->Version);
+ EXPECT_EQ("x", DS.getDraft(File)->Contents);
// We allow versions to go backwards.
- EXPECT_EQ(7, DS.addDraft(File, 7, ""));
- EXPECT_EQ(7, DS.getDraft(File)->Version);
-
- // Valid (no-op) change modifies version.
- auto Result = DS.updateDraft(File, 10, {});
- EXPECT_TRUE(!!Result);
- EXPECT_EQ(10, Result->Version);
- EXPECT_EQ(10, DS.getDraft(File)->Version);
-
- Result = DS.updateDraft(File, llvm::None, {});
- EXPECT_TRUE(!!Result);
- EXPECT_EQ(11, Result->Version);
- EXPECT_EQ(11, DS.getDraft(File)->Version);
-
- TextDocumentContentChangeEvent InvalidChange;
- InvalidChange.range.emplace();
- InvalidChange.rangeLength = 99;
-
- Result = DS.updateDraft(File, 15, {InvalidChange});
- EXPECT_FALSE(!!Result);
- consumeError(Result.takeError());
- EXPECT_EQ(11, DS.getDraft(File)->Version);
+ EXPECT_EQ("7", DS.addDraft(File, "7", "y"));
+ EXPECT_EQ("7", DS.getDraft(File)->Version);
+ EXPECT_EQ("y", DS.getDraft(File)->Contents);
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index c05515f2c094..bfe0e430b775 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -27,6 +27,7 @@ namespace clangd {
namespace {
using llvm::Failed;
+using llvm::FailedWithMessage;
using llvm::HasValue;
MATCHER_P2(Pos, Line, Col, "") {
@@ -802,6 +803,226 @@ TEST(SourceCodeTests, isKeywords) {
EXPECT_FALSE(isKeyword("override", LangOpts));
}
+struct IncrementalTestStep {
+ llvm::StringRef Src;
+ llvm::StringRef Contents;
+};
+
+int rangeLength(llvm::StringRef Code, const Range &Rng) {
+ llvm::Expected<size_t> Start = positionToOffset(Code, Rng.start);
+ llvm::Expected<size_t> End = positionToOffset(Code, Rng.end);
+ assert(Start);
+ assert(End);
+ return *End - *Start;
+}
+
+/// Send the changes one by one to updateDraft, verify the intermediate results.
+void stepByStep(llvm::ArrayRef<IncrementalTestStep> Steps) {
+ std::string Code = Annotations(Steps.front().Src).code().str();
+
+ for (size_t I = 1; I < Steps.size(); I++) {
+ Annotations SrcBefore(Steps[I - 1].Src);
+ Annotations SrcAfter(Steps[I].Src);
+ llvm::StringRef Contents = Steps[I - 1].Contents;
+ TextDocumentContentChangeEvent Event{
+ SrcBefore.range(),
+ rangeLength(SrcBefore.code(), SrcBefore.range()),
+ Contents.str(),
+ };
+
+ EXPECT_THAT_ERROR(applyChange(Code, Event), llvm::Succeeded());
+ EXPECT_EQ(Code, SrcAfter.code());
+ }
+}
+
+TEST(ApplyEditsTest, Simple) {
+ // clang-format off
+ IncrementalTestStep Steps[] =
+ {
+ // Replace a range
+ {
+R"cpp(static int
+hello[[World]]()
+{})cpp",
+ "Universe"
+ },
+ // Delete a range
+ {
+R"cpp(static int
+hello[[Universe]]()
+{})cpp",
+ ""
+ },
+ // Add a range
+ {
+R"cpp(static int
+hello[[]]()
+{})cpp",
+ "Monde"
+ },
+ {
+R"cpp(static int
+helloMonde()
+{})cpp",
+ ""
+ }
+ };
+ // clang-format on
+
+ stepByStep(Steps);
+}
+
+TEST(ApplyEditsTest, MultiLine) {
+ // clang-format off
+ IncrementalTestStep Steps[] =
+ {
+ // Replace a range
+ {
+R"cpp(static [[int
+helloWorld]]()
+{})cpp",
+R"cpp(char
+welcome)cpp"
+ },
+ // Delete a range
+ {
+R"cpp(static char[[
+welcome]]()
+{})cpp",
+ ""
+ },
+ // Add a range
+ {
+R"cpp(static char[[]]()
+{})cpp",
+ R"cpp(
+cookies)cpp"
+ },
+ // Replace the whole file
+ {
+R"cpp([[static char
+cookies()
+{}]])cpp",
+ R"cpp(#include <stdio.h>
+)cpp"
+ },
+ // Delete the whole file
+ {
+ R"cpp([[#include <stdio.h>
+]])cpp",
+ "",
+ },
+ // Add something to an empty file
+ {
+ "[[]]",
+ R"cpp(int main() {
+)cpp",
+ },
+ {
+ R"cpp(int main() {
+)cpp",
+ ""
+ }
+ };
+ // clang-format on
+
+ stepByStep(Steps);
+}
+
+TEST(ApplyEditsTest, WrongRangeLength) {
+ std::string Code = "int main() {}\n";
+
+ TextDocumentContentChangeEvent Change;
+ Change.range.emplace();
+ Change.range->start.line = 0;
+ Change.range->start.character = 0;
+ Change.range->end.line = 0;
+ Change.range->end.character = 2;
+ Change.rangeLength = 10;
+
+ EXPECT_THAT_ERROR(applyChange(Code, Change),
+ FailedWithMessage("Change's rangeLength (10) doesn't match "
+ "the computed range length (2)."));
+}
+
+TEST(ApplyEditsTest, EndBeforeStart) {
+ std::string Code = "int main() {}\n";
+
+ TextDocumentContentChangeEvent Change;
+ Change.range.emplace();
+ Change.range->start.line = 0;
+ Change.range->start.character = 5;
+ Change.range->end.line = 0;
+ Change.range->end.character = 3;
+
+ EXPECT_THAT_ERROR(
+ applyChange(Code, Change),
+ FailedWithMessage(
+ "Range's end position (0:3) is before start position (0:5)"));
+}
+
+TEST(ApplyEditsTest, StartCharOutOfRange) {
+ std::string Code = "int main() {}\n";
+
+ TextDocumentContentChangeEvent Change;
+ Change.range.emplace();
+ Change.range->start.line = 0;
+ Change.range->start.character = 100;
+ Change.range->end.line = 0;
+ Change.range->end.character = 100;
+ Change.text = "foo";
+
+ EXPECT_THAT_ERROR(
+ applyChange(Code, Change),
+ FailedWithMessage("utf-16 offset 100 is invalid for line 0"));
+}
+
+TEST(ApplyEditsTest, EndCharOutOfRange) {
+ std::string Code = "int main() {}\n";
+
+ TextDocumentContentChangeEvent Change;
+ Change.range.emplace();
+ Change.range->start.line = 0;
+ Change.range->start.character = 0;
+ Change.range->end.line = 0;
+ Change.range->end.character = 100;
+ Change.text = "foo";
+
+ EXPECT_THAT_ERROR(
+ applyChange(Code, Change),
+ FailedWithMessage("utf-16 offset 100 is invalid for line 0"));
+}
+
+TEST(ApplyEditsTest, StartLineOutOfRange) {
+ std::string Code = "int main() {}\n";
+
+ TextDocumentContentChangeEvent Change;
+ Change.range.emplace();
+ Change.range->start.line = 100;
+ Change.range->start.character = 0;
+ Change.range->end.line = 100;
+ Change.range->end.character = 0;
+ Change.text = "foo";
+
+ EXPECT_THAT_ERROR(applyChange(Code, Change),
+ FailedWithMessage("Line value is out of range (100)"));
+}
+
+TEST(ApplyEditsTest, EndLineOutOfRange) {
+ std::string Code = "int main() {}\n";
+
+ TextDocumentContentChangeEvent Change;
+ Change.range.emplace();
+ Change.range->start.line = 0;
+ Change.range->start.character = 0;
+ Change.range->end.line = 100;
+ Change.range->end.character = 0;
+ Change.text = "foo";
+
+ EXPECT_THAT_ERROR(applyChange(Code, Change),
+ FailedWithMessage("Line value is out of range (100)"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp
index 27b6cf33e055..8c18148a1d30 100644
--- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ b/clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -115,9 +115,9 @@ runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
}
llvm::Expected<tooling::Replacements>
-runFormatFile(ClangdServer &Server, PathRef File, StringRef Code) {
+runFormatFile(ClangdServer &Server, PathRef File, llvm::Optional<Range> Rng) {
llvm::Optional<llvm::Expected<tooling::Replacements>> Result;
- Server.formatFile(File, Code, capture(Result));
+ Server.formatFile(File, Rng, capture(Result));
return std::move(*Result);
}
diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h
index fd0f5dba604d..71fdf33ce720 100644
--- a/clang-tools-extra/clangd/unittests/SyncAPI.h
+++ b/clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -50,7 +50,7 @@ runPrepareRename(ClangdServer &Server, PathRef File, Position Pos,
const clangd::RenameOptions &RenameOpts);
llvm::Expected<tooling::Replacements>
-runFormatFile(ClangdServer &Server, PathRef File, StringRef Code);
+runFormatFile(ClangdServer &Server, PathRef File, llvm::Optional<Range>);
SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
More information about the cfe-commits
mailing list