[clang-tools-extra] r328500 - [clangd] Support incremental document syncing
Simon Marchi via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 26 07:41:40 PDT 2018
Author: simark
Date: Mon Mar 26 07:41:40 2018
New Revision: 328500
URL: http://llvm.org/viewvc/llvm-project?rev=328500&view=rev
Log:
[clangd] Support incremental document syncing
Summary:
This patch adds support for incremental document syncing, as described
in the LSP spec. The protocol specifies ranges in terms of Position (a
line and a character), and our drafts are stored as plain strings. So I
see two things that may not be super efficient for very large files:
- Converting a Position to an offset (the positionToOffset function)
requires searching for end of lines until we reach the desired line.
- When we update a range, we construct a new string, which implies
copying the whole document.
However, for the typical size of a C++ document and the frequency of
update (at which a user types), it may not be an issue. This patch aims
at getting the basic feature in, and we can always improve it later if
we find it's too slow.
Signed-off-by: Simon Marchi <simon.marchi at ericsson.com>
Reviewers: malaperle, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: MaskRay, klimek, mgorny, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44272
Added:
clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test
clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/DraftStore.cpp
clang-tools-extra/trunk/clangd/DraftStore.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
clang-tools-extra/trunk/test/clangd/initialize-params.test
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Mar 26 07:41:40 2018
@@ -100,7 +100,7 @@ void ClangdLSPServer::onInitialize(Initi
reply(json::obj{
{{"capabilities",
json::obj{
- {"textDocumentSync", 1},
+ {"textDocumentSync", (int)TextDocumentSyncKind::Incremental},
{"documentFormattingProvider", true},
{"documentRangeFormattingProvider", true},
{"documentOnTypeFormattingProvider",
@@ -147,25 +147,30 @@ void ClangdLSPServer::onDocumentDidOpen(
PathRef File = Params.textDocument.uri.file();
std::string &Contents = Params.textDocument.text;
- DraftMgr.updateDraft(File, Contents);
+ DraftMgr.addDraft(File, Contents);
Server.addDocument(File, Contents, WantDiagnostics::Yes);
}
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
- if (Params.contentChanges.size() != 1)
- return replyError(ErrorCode::InvalidParams,
- "can only apply one change at a time");
auto WantDiags = WantDiagnostics::Auto;
if (Params.wantDiagnostics.hasValue())
WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Yes
: WantDiagnostics::No;
PathRef File = Params.textDocument.uri.file();
- std::string &Contents = Params.contentChanges[0].text;
+ llvm::Expected<std::string> Contents =
+ DraftMgr.updateDraft(File, Params.contentChanges);
+ if (!Contents) {
+ // 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);
+ log(llvm::toString(Contents.takeError()));
+ return;
+ }
- // We only support full syncing right now.
- DraftMgr.updateDraft(File, Contents);
- Server.addDocument(File, Contents, WantDiags);
+ Server.addDocument(File, *Contents, WantDiags);
}
void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
Modified: clang-tools-extra/trunk/clangd/DraftStore.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.cpp?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/DraftStore.cpp (original)
+++ clang-tools-extra/trunk/clangd/DraftStore.cpp Mon Mar 26 07:41:40 2018
@@ -8,6 +8,8 @@
//===----------------------------------------------------------------------===//
#include "DraftStore.h"
+#include "SourceCode.h"
+#include "llvm/Support/Errc.h"
using namespace clang;
using namespace clang::clangd;
@@ -32,12 +34,72 @@ std::vector<Path> DraftStore::getActiveF
return ResultVector;
}
-void DraftStore::updateDraft(PathRef File, StringRef Contents) {
+void DraftStore::addDraft(PathRef File, StringRef Contents) {
std::lock_guard<std::mutex> Lock(Mutex);
Drafts[File] = Contents;
}
+llvm::Expected<std::string> DraftStore::updateDraft(
+ PathRef File, llvm::ArrayRef<TextDocumentContentChangeEvent> Changes) {
+ std::lock_guard<std::mutex> Lock(Mutex);
+
+ auto EntryIt = Drafts.find(File);
+ if (EntryIt == Drafts.end()) {
+ return llvm::make_error<llvm::StringError>(
+ "Trying to do incremental update on non-added document: " + File,
+ llvm::errc::invalid_argument);
+ }
+
+ std::string Contents = EntryIt->second;
+
+ 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 llvm::make_error<llvm::StringError>(
+ llvm::formatv(
+ "Range's end position ({0}) is before start position ({1})", End,
+ Start),
+ llvm::errc::invalid_argument);
+
+ if (Change.rangeLength &&
+ (ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("Change's rangeLength ({0}) doesn't match the "
+ "computed range length ({1}).",
+ *Change.rangeLength, *EndIndex - *StartIndex),
+ llvm::errc::invalid_argument);
+
+ 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);
+ }
+
+ EntryIt->second = Contents;
+ return Contents;
+}
+
void DraftStore::removeDraft(PathRef File) {
std::lock_guard<std::mutex> Lock(Mutex);
Modified: clang-tools-extra/trunk/clangd/DraftStore.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.h?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/DraftStore.h (original)
+++ clang-tools-extra/trunk/clangd/DraftStore.h Mon Mar 26 07:41:40 2018
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_DRAFTSTORE_H
#include "Path.h"
+#include "Protocol.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/StringMap.h"
#include <mutex>
@@ -21,7 +22,8 @@ namespace clang {
namespace clangd {
/// A thread-safe container for files opened in a workspace, addressed by
-/// filenames. The contents are owned by the DraftStore.
+/// filenames. The contents are owned by the DraftStore. This class supports
+/// both whole and incremental updates of the documents.
class DraftStore {
public:
/// \return Contents of the stored document.
@@ -32,7 +34,17 @@ public:
std::vector<Path> getActiveFiles() const;
/// Replace contents of the draft for \p File with \p Contents.
- void updateDraft(PathRef File, StringRef Contents);
+ void addDraft(PathRef File, 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.
+ ///
+ /// \return The new version of the draft for \p File, or an error if the
+ /// changes couldn't be applied.
+ llvm::Expected<std::string>
+ updateDraft(PathRef File,
+ llvm::ArrayRef<TextDocumentContentChangeEvent> Changes);
/// Remove the draft from the store.
void removeDraft(PathRef File);
Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Mon Mar 26 07:41:40 2018
@@ -248,7 +248,8 @@ bool fromJSON(const json::Expr &Params,
bool fromJSON(const json::Expr &Params, TextDocumentContentChangeEvent &R) {
json::ObjectMapper O(Params);
- return O && O.map("text", R.text);
+ return O && O.map("range", R.range) && O.map("rangeLength", R.rangeLength) &&
+ O.map("text", R.text);
}
bool fromJSON(const json::Expr &Params, FormattingOptions &R) {
Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Mon Mar 26 07:41:40 2018
@@ -197,6 +197,20 @@ inline bool fromJSON(const json::Expr &,
using ShutdownParams = NoParams;
using ExitParams = NoParams;
+/// Defines how the host (editor) should sync document changes to the language
+/// server.
+enum class TextDocumentSyncKind {
+ /// Documents should not be synced at all.
+ None = 0,
+
+ /// Documents are synced by always sending the full content of the document.
+ Full = 1,
+
+ /// Documents are synced by sending the full content on open. After that
+ /// only incremental updates to the document are send.
+ Incremental = 2,
+};
+
struct CompletionItemClientCapabilities {
/// Client supports snippets as insert text.
bool snippetSupport = false;
@@ -287,7 +301,13 @@ struct DidCloseTextDocumentParams {
bool fromJSON(const json::Expr &, DidCloseTextDocumentParams &);
struct TextDocumentContentChangeEvent {
- /// The new text of the document.
+ /// The range of the document that changed.
+ llvm::Optional<Range> range;
+
+ /// The length of the range that got replaced.
+ llvm::Optional<int> rangeLength;
+
+ /// The new text of the range/document.
std::string text;
};
bool fromJSON(const json::Expr &, TextDocumentContentChangeEvent &);
@@ -745,4 +765,14 @@ json::Expr toJSON(const DocumentHighligh
} // namespace clangd
} // namespace clang
+namespace llvm {
+template <> struct format_provider<clang::clangd::Position> {
+ static void format(const clang::clangd::Position &Pos, raw_ostream &OS,
+ StringRef Style) {
+ assert(Style.empty() && "style modifiers for this type are not supported");
+ OS << Pos;
+ }
+};
+} // namespace llvm
+
#endif
Modified: clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test (original)
+++ clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test Mon Mar 26 07:41:40 2018
@@ -36,7 +36,7 @@
# CHECK-NEXT: ","
# CHECK-NEXT: ]
# CHECK-NEXT: },
-# CHECK-NEXT: "textDocumentSync": 1
+# CHECK-NEXT: "textDocumentSync": 2
# CHECK-NEXT: }
# CHECK-NEXT: }
---
Modified: clang-tools-extra/trunk/test/clangd/initialize-params.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params.test?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/initialize-params.test (original)
+++ clang-tools-extra/trunk/test/clangd/initialize-params.test Mon Mar 26 07:41:40 2018
@@ -36,7 +36,7 @@
# CHECK-NEXT: ","
# CHECK-NEXT: ]
# CHECK-NEXT: },
-# CHECK-NEXT: "textDocumentSync": 1
+# CHECK-NEXT: "textDocumentSync": 2
# CHECK-NEXT: }
# CHECK-NEXT: }
---
Added: clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test?rev=328500&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test (added)
+++ clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test Mon Mar 26 07:41:40 2018
@@ -0,0 +1,37 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int main() {}\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":6}}}
+# CHECK: "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 8,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file:///clangd-test/main.cpp"
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///main.cpp"},"contentChanges":[{"range":{"start":{"line":100,"character":0},"end":{"line":100,"character":0}},"text": "foo"}]}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":6}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32602,
+# CHECK-NEXT: "message": "trying to get AST for non-added document"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0"
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=328500&r1=328499&r2=328500&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Mon Mar 26 07:41:40 2018
@@ -15,6 +15,7 @@ add_extra_unittest(ClangdTests
CodeCompleteTests.cpp
CodeCompletionStringsTests.cpp
ContextTests.cpp
+ DraftStoreTests.cpp
FileIndexTests.cpp
FuzzyMatchTests.cpp
HeadersTests.cpp
Added: clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp?rev=328500&view=auto
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp (added)
+++ clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp Mon Mar 26 07:41:40 2018
@@ -0,0 +1,350 @@
+//===-- DraftStoreTests.cpp -------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Annotations.h"
+#include "DraftStore.h"
+#include "SourceCode.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+struct IncrementalTestStep {
+ StringRef Src;
+ StringRef Contents;
+};
+
+int rangeLength(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.
+ DS.addDraft(Path, InitialSrc.code());
+
+ for (size_t i = 1; i < Steps.size(); i++) {
+ Annotations SrcBefore(Steps[i - 1].Src);
+ Annotations SrcAfter(Steps[i].Src);
+ StringRef Contents = Steps[i - 1].Contents;
+ TextDocumentContentChangeEvent Event{
+ SrcBefore.range(),
+ rangeLength(SrcBefore.code(), SrcBefore.range()),
+ Contents.str(),
+ };
+
+ llvm::Expected<std::string> Result = DS.updateDraft(Path, {Event});
+ ASSERT_TRUE(!!Result);
+ EXPECT_EQ(*Result, SrcAfter.code());
+ EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());
+ }
+}
+
+/// 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);
+ StringRef Contents = Steps[i].Contents;
+
+ Changes.push_back({
+ Src.range(),
+ rangeLength(Src.code(), Src.range()),
+ Contents.str(),
+ });
+ }
+
+ // Set the initial content.
+ DS.addDraft(Path, InitialSrc.code());
+
+ llvm::Expected<std::string> Result = DS.updateDraft(Path, Changes);
+
+ ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError());
+ EXPECT_EQ(*Result, FinalSrc.code());
+ EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());
+}
+
+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, "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;
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(
+ llvm::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, "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;
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(llvm::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, "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";
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(llvm::toString(Result.takeError()),
+ "Character value is out of range (100)");
+}
+
+TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {
+ DraftStore DS;
+ Path File = "foo.cpp";
+
+ DS.addDraft(File, "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";
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(llvm::toString(Result.takeError()),
+ "Character value is out of range (100)");
+}
+
+TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {
+ DraftStore DS;
+ Path File = "foo.cpp";
+
+ DS.addDraft(File, "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";
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(llvm::toString(Result.takeError()),
+ "Line value is out of range (100)");
+}
+
+TEST(DraftStoreIncrementalUpdateTest, EndLineOutOfRange) {
+ DraftStore DS;
+ Path File = "foo.cpp";
+
+ DS.addDraft(File, "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";
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(llvm::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";
+ DS.addDraft(File, 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";
+
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change1, Change2});
+
+ EXPECT_TRUE(!Result);
+ EXPECT_EQ(llvm::toString(Result.takeError()),
+ "Character value is out of range (100)");
+
+ llvm::Optional<std::string> Contents = DS.getDraft(File);
+ EXPECT_TRUE(Contents);
+ EXPECT_EQ(*Contents, OriginalContents);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
More information about the cfe-commits
mailing list