<div dir="ltr">One of these new tests does not pass on Windows due to assumptions about absolute path structure. The FileCheck output seems self-explanatory:<div><br></div><div><div>$ "FileCheck" "-strict-whitespace" "C:\b\slave\clang-x86-windows-msvc2015\clang-x86-windows-msvc2015\llvm\tools\clang\tools\extra\test\clangd\textdocument-didchange-fail.test"</div><div># command stderr:</div><div>C:\b\slave\clang-x86-windows-msvc2015\clang-x86-windows-msvc2015\llvm\tools\clang\tools\extra\test\clangd\textdocument-didchange-fail.test:22:20: error: expected string not found in input</div><div># CHECK-NEXT: "uri": "file:///clangd-test/main.cpp"</div><div> ^</div><div><stdin>:68:7: note: scanning from here</div><div> "uri": "file:///C%3a/clangd-test/main.cpp"</div></div><div><br></div><div>I committed a speculative fix for this in r328645, hopefully it does the job.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Mar 26, 2018 at 7:32 PM Simon Marchi via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: simark<br>
Date: Mon Mar 26 07:41:40 2018<br>
New Revision: 328500<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=328500&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=328500&view=rev</a><br>
Log:<br>
[clangd] Support incremental document syncing<br>
<br>
Summary:<br>
This patch adds support for incremental document syncing, as described<br>
in the LSP spec. The protocol specifies ranges in terms of Position (a<br>
line and a character), and our drafts are stored as plain strings. So I<br>
see two things that may not be super efficient for very large files:<br>
<br>
- Converting a Position to an offset (the positionToOffset function)<br>
requires searching for end of lines until we reach the desired line.<br>
- When we update a range, we construct a new string, which implies<br>
copying the whole document.<br>
<br>
However, for the typical size of a C++ document and the frequency of<br>
update (at which a user types), it may not be an issue. This patch aims<br>
at getting the basic feature in, and we can always improve it later if<br>
we find it's too slow.<br>
<br>
Signed-off-by: Simon Marchi <<a href="mailto:simon.marchi@ericsson.com" target="_blank">simon.marchi@ericsson.com</a>><br>
<br>
Reviewers: malaperle, ilya-biryukov<br>
<br>
Reviewed By: ilya-biryukov<br>
<br>
Subscribers: MaskRay, klimek, mgorny, ilya-biryukov, jkorous-apple, ioeric, cfe-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D44272" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44272</a><br>
<br>
Added:<br>
clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test<br>
clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp<br>
Modified:<br>
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp<br>
clang-tools-extra/trunk/clangd/DraftStore.cpp<br>
clang-tools-extra/trunk/clangd/DraftStore.h<br>
clang-tools-extra/trunk/clangd/Protocol.cpp<br>
clang-tools-extra/trunk/clangd/Protocol.h<br>
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test<br>
clang-tools-extra/trunk/test/clangd/initialize-params.test<br>
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt<br>
<br>
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Mar 26 07:41:40 2018<br>
@@ -100,7 +100,7 @@ void ClangdLSPServer::onInitialize(Initi<br>
reply(json::obj{<br>
{{"capabilities",<br>
json::obj{<br>
- {"textDocumentSync", 1},<br>
+ {"textDocumentSync", (int)TextDocumentSyncKind::Incremental},<br>
{"documentFormattingProvider", true},<br>
{"documentRangeFormattingProvider", true},<br>
{"documentOnTypeFormattingProvider",<br>
@@ -147,25 +147,30 @@ void ClangdLSPServer::onDocumentDidOpen(<br>
PathRef File = Params.textDocument.uri.file();<br>
std::string &Contents = Params.textDocument.text;<br>
<br>
- DraftMgr.updateDraft(File, Contents);<br>
+ DraftMgr.addDraft(File, Contents);<br>
Server.addDocument(File, Contents, WantDiagnostics::Yes);<br>
}<br>
<br>
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {<br>
- if (Params.contentChanges.size() != 1)<br>
- return replyError(ErrorCode::InvalidParams,<br>
- "can only apply one change at a time");<br>
auto WantDiags = WantDiagnostics::Auto;<br>
if (Params.wantDiagnostics.hasValue())<br>
WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Yes<br>
: WantDiagnostics::No;<br>
<br>
PathRef File = Params.textDocument.uri.file();<br>
- std::string &Contents = Params.contentChanges[0].text;<br>
+ llvm::Expected<std::string> Contents =<br>
+ DraftMgr.updateDraft(File, Params.contentChanges);<br>
+ if (!Contents) {<br>
+ // If this fails, we are most likely going to be not in sync anymore with<br>
+ // the client. It is better to remove the draft and let further operations<br>
+ // fail rather than giving wrong results.<br>
+ DraftMgr.removeDraft(File);<br>
+ Server.removeDocument(File);<br>
+ log(llvm::toString(Contents.takeError()));<br>
+ return;<br>
+ }<br>
<br>
- // We only support full syncing right now.<br>
- DraftMgr.updateDraft(File, Contents);<br>
- Server.addDocument(File, Contents, WantDiags);<br>
+ Server.addDocument(File, *Contents, WantDiags);<br>
}<br>
<br>
void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {<br>
<br>
Modified: clang-tools-extra/trunk/clangd/DraftStore.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.cpp?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.cpp?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/DraftStore.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/DraftStore.cpp Mon Mar 26 07:41:40 2018<br>
@@ -8,6 +8,8 @@<br>
//===----------------------------------------------------------------------===//<br>
<br>
#include "DraftStore.h"<br>
+#include "SourceCode.h"<br>
+#include "llvm/Support/Errc.h"<br>
<br>
using namespace clang;<br>
using namespace clang::clangd;<br>
@@ -32,12 +34,72 @@ std::vector<Path> DraftStore::getActiveF<br>
return ResultVector;<br>
}<br>
<br>
-void DraftStore::updateDraft(PathRef File, StringRef Contents) {<br>
+void DraftStore::addDraft(PathRef File, StringRef Contents) {<br>
std::lock_guard<std::mutex> Lock(Mutex);<br>
<br>
Drafts[File] = Contents;<br>
}<br>
<br>
+llvm::Expected<std::string> DraftStore::updateDraft(<br>
+ PathRef File, llvm::ArrayRef<TextDocumentContentChangeEvent> Changes) {<br>
+ std::lock_guard<std::mutex> Lock(Mutex);<br>
+<br>
+ auto EntryIt = Drafts.find(File);<br>
+ if (EntryIt == Drafts.end()) {<br>
+ return llvm::make_error<llvm::StringError>(<br>
+ "Trying to do incremental update on non-added document: " + File,<br>
+ llvm::errc::invalid_argument);<br>
+ }<br>
+<br>
+ std::string Contents = EntryIt->second;<br>
+<br>
+ for (const TextDocumentContentChangeEvent &Change : Changes) {<br>
+ if (!Change.range) {<br>
+ Contents = Change.text;<br>
+ continue;<br>
+ }<br>
+<br>
+ const Position &Start = Change.range->start;<br>
+ llvm::Expected<size_t> StartIndex =<br>
+ positionToOffset(Contents, Start, false);<br>
+ if (!StartIndex)<br>
+ return StartIndex.takeError();<br>
+<br>
+ const Position &End = Change.range->end;<br>
+ llvm::Expected<size_t> EndIndex = positionToOffset(Contents, End, false);<br>
+ if (!EndIndex)<br>
+ return EndIndex.takeError();<br>
+<br>
+ if (*EndIndex < *StartIndex)<br>
+ return llvm::make_error<llvm::StringError>(<br>
+ llvm::formatv(<br>
+ "Range's end position ({0}) is before start position ({1})", End,<br>
+ Start),<br>
+ llvm::errc::invalid_argument);<br>
+<br>
+ if (Change.rangeLength &&<br>
+ (ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)<br>
+ return llvm::make_error<llvm::StringError>(<br>
+ llvm::formatv("Change's rangeLength ({0}) doesn't match the "<br>
+ "computed range length ({1}).",<br>
+ *Change.rangeLength, *EndIndex - *StartIndex),<br>
+ llvm::errc::invalid_argument);<br>
+<br>
+ std::string NewContents;<br>
+ NewContents.reserve(*StartIndex + Change.text.length() +<br>
+ (Contents.length() - *EndIndex));<br>
+<br>
+ NewContents = Contents.substr(0, *StartIndex);<br>
+ NewContents += Change.text;<br>
+ NewContents += Contents.substr(*EndIndex);<br>
+<br>
+ Contents = std::move(NewContents);<br>
+ }<br>
+<br>
+ EntryIt->second = Contents;<br>
+ return Contents;<br>
+}<br>
+<br>
void DraftStore::removeDraft(PathRef File) {<br>
std::lock_guard<std::mutex> Lock(Mutex);<br>
<br>
<br>
Modified: clang-tools-extra/trunk/clangd/DraftStore.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.h?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.h?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/DraftStore.h (original)<br>
+++ clang-tools-extra/trunk/clangd/DraftStore.h Mon Mar 26 07:41:40 2018<br>
@@ -11,6 +11,7 @@<br>
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_DRAFTSTORE_H<br>
<br>
#include "Path.h"<br>
+#include "Protocol.h"<br>
#include "clang/Basic/LLVM.h"<br>
#include "llvm/ADT/StringMap.h"<br>
#include <mutex><br>
@@ -21,7 +22,8 @@ namespace clang {<br>
namespace clangd {<br>
<br>
/// A thread-safe container for files opened in a workspace, addressed by<br>
-/// filenames. The contents are owned by the DraftStore.<br>
+/// filenames. The contents are owned by the DraftStore. This class supports<br>
+/// both whole and incremental updates of the documents.<br>
class DraftStore {<br>
public:<br>
/// \return Contents of the stored document.<br>
@@ -32,7 +34,17 @@ public:<br>
std::vector<Path> getActiveFiles() const;<br>
<br>
/// Replace contents of the draft for \p File with \p Contents.<br>
- void updateDraft(PathRef File, StringRef Contents);<br>
+ void addDraft(PathRef File, StringRef Contents);<br>
+<br>
+ /// Update the contents of the draft for \p File based on \p Changes.<br>
+ /// If a position in \p Changes is invalid (e.g. out-of-range), the<br>
+ /// draft is not modified.<br>
+ ///<br>
+ /// \return The new version of the draft for \p File, or an error if the<br>
+ /// changes couldn't be applied.<br>
+ llvm::Expected<std::string><br>
+ updateDraft(PathRef File,<br>
+ llvm::ArrayRef<TextDocumentContentChangeEvent> Changes);<br>
<br>
/// Remove the draft from the store.<br>
void removeDraft(PathRef File);<br>
<br>
Modified: clang-tools-extra/trunk/clangd/Protocol.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Mon Mar 26 07:41:40 2018<br>
@@ -248,7 +248,8 @@ bool fromJSON(const json::Expr &Params,<br>
<br>
bool fromJSON(const json::Expr &Params, TextDocumentContentChangeEvent &R) {<br>
json::ObjectMapper O(Params);<br>
- return O && O.map("text", R.text);<br>
+ return O && O.map("range", R.range) && O.map("rangeLength", R.rangeLength) &&<br>
+ O.map("text", R.text);<br>
}<br>
<br>
bool fromJSON(const json::Expr &Params, FormattingOptions &R) {<br>
<br>
Modified: clang-tools-extra/trunk/clangd/Protocol.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/Protocol.h (original)<br>
+++ clang-tools-extra/trunk/clangd/Protocol.h Mon Mar 26 07:41:40 2018<br>
@@ -197,6 +197,20 @@ inline bool fromJSON(const json::Expr &,<br>
using ShutdownParams = NoParams;<br>
using ExitParams = NoParams;<br>
<br>
+/// Defines how the host (editor) should sync document changes to the language<br>
+/// server.<br>
+enum class TextDocumentSyncKind {<br>
+ /// Documents should not be synced at all.<br>
+ None = 0,<br>
+<br>
+ /// Documents are synced by always sending the full content of the document.<br>
+ Full = 1,<br>
+<br>
+ /// Documents are synced by sending the full content on open. After that<br>
+ /// only incremental updates to the document are send.<br>
+ Incremental = 2,<br>
+};<br>
+<br>
struct CompletionItemClientCapabilities {<br>
/// Client supports snippets as insert text.<br>
bool snippetSupport = false;<br>
@@ -287,7 +301,13 @@ struct DidCloseTextDocumentParams {<br>
bool fromJSON(const json::Expr &, DidCloseTextDocumentParams &);<br>
<br>
struct TextDocumentContentChangeEvent {<br>
- /// The new text of the document.<br>
+ /// The range of the document that changed.<br>
+ llvm::Optional<Range> range;<br>
+<br>
+ /// The length of the range that got replaced.<br>
+ llvm::Optional<int> rangeLength;<br>
+<br>
+ /// The new text of the range/document.<br>
std::string text;<br>
};<br>
bool fromJSON(const json::Expr &, TextDocumentContentChangeEvent &);<br>
@@ -745,4 +765,14 @@ json::Expr toJSON(const DocumentHighligh<br>
} // namespace clangd<br>
} // namespace clang<br>
<br>
+namespace llvm {<br>
+template <> struct format_provider<clang::clangd::Position> {<br>
+ static void format(const clang::clangd::Position &Pos, raw_ostream &OS,<br>
+ StringRef Style) {<br>
+ assert(Style.empty() && "style modifiers for this type are not supported");<br>
+ OS << Pos;<br>
+ }<br>
+};<br>
+} // namespace llvm<br>
+<br>
#endif<br>
<br>
Modified: clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test (original)<br>
+++ clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test Mon Mar 26 07:41:40 2018<br>
@@ -36,7 +36,7 @@<br>
# CHECK-NEXT: ","<br>
# CHECK-NEXT: ]<br>
# CHECK-NEXT: },<br>
-# CHECK-NEXT: "textDocumentSync": 1<br>
+# CHECK-NEXT: "textDocumentSync": 2<br>
# CHECK-NEXT: }<br>
# CHECK-NEXT: }<br>
---<br>
<br>
Modified: clang-tools-extra/trunk/test/clangd/initialize-params.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params.test?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params.test?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clangd/initialize-params.test (original)<br>
+++ clang-tools-extra/trunk/test/clangd/initialize-params.test Mon Mar 26 07:41:40 2018<br>
@@ -36,7 +36,7 @@<br>
# CHECK-NEXT: ","<br>
# CHECK-NEXT: ]<br>
# CHECK-NEXT: },<br>
-# CHECK-NEXT: "textDocumentSync": 1<br>
+# CHECK-NEXT: "textDocumentSync": 2<br>
# CHECK-NEXT: }<br>
# CHECK-NEXT: }<br>
---<br>
<br>
Added: clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test?rev=328500&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test?rev=328500&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test (added)<br>
+++ clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test Mon Mar 26 07:41:40 2018<br>
@@ -0,0 +1,37 @@<br>
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s<br>
+# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s<br>
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}<br>
+---<br>
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int main() {}\n"}}}<br>
+---<br>
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":6}}}<br>
+# CHECK: "id": 1,<br>
+# CHECK-NEXT: "jsonrpc": "2.0",<br>
+# CHECK-NEXT: "result": [<br>
+# CHECK-NEXT: {<br>
+# CHECK-NEXT: "range": {<br>
+# CHECK-NEXT: "end": {<br>
+# CHECK-NEXT: "character": 8,<br>
+# CHECK-NEXT: "line": 0<br>
+# CHECK-NEXT: },<br>
+# CHECK-NEXT: "start": {<br>
+# CHECK-NEXT: "character": 4,<br>
+# CHECK-NEXT: "line": 0<br>
+# CHECK-NEXT: }<br>
+# CHECK-NEXT: },<br>
+# CHECK-NEXT: "uri": "file:///clangd-test/main.cpp"<br>
+# CHECK-NEXT: }<br>
+# CHECK-NEXT: ]<br>
+---<br>
+{"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"}]}}<br>
+---<br>
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":6}}}<br>
+# CHECK: "error": {<br>
+# CHECK-NEXT: "code": -32602,<br>
+# CHECK-NEXT: "message": "trying to get AST for non-added document"<br>
+# CHECK-NEXT: },<br>
+# CHECK-NEXT: "id": 1,<br>
+# CHECK-NEXT: "jsonrpc": "2.0"<br>
+# CHECK-NEXT:}<br>
+---<br>
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}<br>
<br>
Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=328500&r1=328499&r2=328500&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=328500&r1=328499&r2=328500&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)<br>
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Mon Mar 26 07:41:40 2018<br>
@@ -15,6 +15,7 @@ add_extra_unittest(ClangdTests<br>
CodeCompleteTests.cpp<br>
CodeCompletionStringsTests.cpp<br>
ContextTests.cpp<br>
+ DraftStoreTests.cpp<br>
FileIndexTests.cpp<br>
FuzzyMatchTests.cpp<br>
HeadersTests.cpp<br>
<br>
Added: clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp?rev=328500&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp?rev=328500&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp (added)<br>
+++ clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp Mon Mar 26 07:41:40 2018<br>
@@ -0,0 +1,350 @@<br>
+//===-- DraftStoreTests.cpp -------------------------------------*- C++ -*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "Annotations.h"<br>
+#include "DraftStore.h"<br>
+#include "SourceCode.h"<br>
+#include "gmock/gmock.h"<br>
+#include "gtest/gtest.h"<br>
+<br>
+namespace clang {<br>
+namespace clangd {<br>
+namespace {<br>
+<br>
+struct IncrementalTestStep {<br>
+ StringRef Src;<br>
+ StringRef Contents;<br>
+};<br>
+<br>
+int rangeLength(StringRef Code, const Range &Rng) {<br>
+ llvm::Expected<size_t> Start = positionToOffset(Code, Rng.start);<br>
+ llvm::Expected<size_t> End = positionToOffset(Code, Rng.end);<br>
+ assert(Start);<br>
+ assert(End);<br>
+ return *End - *Start;<br>
+}<br>
+<br>
+/// Send the changes one by one to updateDraft, verify the intermediate results.<br>
+void stepByStep(llvm::ArrayRef<IncrementalTestStep> Steps) {<br>
+ DraftStore DS;<br>
+ Annotations InitialSrc(Steps.front().Src);<br>
+ constexpr llvm::StringLiteral Path("/hello.cpp");<br>
+<br>
+ // Set the initial content.<br>
+ DS.addDraft(Path, InitialSrc.code());<br>
+<br>
+ for (size_t i = 1; i < Steps.size(); i++) {<br>
+ Annotations SrcBefore(Steps[i - 1].Src);<br>
+ Annotations SrcAfter(Steps[i].Src);<br>
+ StringRef Contents = Steps[i - 1].Contents;<br>
+ TextDocumentContentChangeEvent Event{<br>
+ SrcBefore.range(),<br>
+ rangeLength(SrcBefore.code(), SrcBefore.range()),<br>
+ Contents.str(),<br>
+ };<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(Path, {Event});<br>
+ ASSERT_TRUE(!!Result);<br>
+ EXPECT_EQ(*Result, SrcAfter.code());<br>
+ EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());<br>
+ }<br>
+}<br>
+<br>
+/// Send all the changes at once to updateDraft, check only the final result.<br>
+void allAtOnce(llvm::ArrayRef<IncrementalTestStep> Steps) {<br>
+ DraftStore DS;<br>
+ Annotations InitialSrc(Steps.front().Src);<br>
+ Annotations FinalSrc(Steps.back().Src);<br>
+ constexpr llvm::StringLiteral Path("/hello.cpp");<br>
+ std::vector<TextDocumentContentChangeEvent> Changes;<br>
+<br>
+ for (size_t i = 0; i < Steps.size() - 1; i++) {<br>
+ Annotations Src(Steps[i].Src);<br>
+ StringRef Contents = Steps[i].Contents;<br>
+<br>
+ Changes.push_back({<br>
+ Src.range(),<br>
+ rangeLength(Src.code(), Src.range()),<br>
+ Contents.str(),<br>
+ });<br>
+ }<br>
+<br>
+ // Set the initial content.<br>
+ DS.addDraft(Path, InitialSrc.code());<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(Path, Changes);<br>
+<br>
+ ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError());<br>
+ EXPECT_EQ(*Result, FinalSrc.code());<br>
+ EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, Simple) {<br>
+ // clang-format off<br>
+ IncrementalTestStep Steps[] =<br>
+ {<br>
+ // Replace a range<br>
+ {<br>
+R"cpp(static int<br>
+hello[[World]]()<br>
+{})cpp",<br>
+ "Universe"<br>
+ },<br>
+ // Delete a range<br>
+ {<br>
+R"cpp(static int<br>
+hello[[Universe]]()<br>
+{})cpp",<br>
+ ""<br>
+ },<br>
+ // Add a range<br>
+ {<br>
+R"cpp(static int<br>
+hello[[]]()<br>
+{})cpp",<br>
+ "Monde"<br>
+ },<br>
+ {<br>
+R"cpp(static int<br>
+helloMonde()<br>
+{})cpp",<br>
+ ""<br>
+ }<br>
+ };<br>
+ // clang-format on<br>
+<br>
+ stepByStep(Steps);<br>
+ allAtOnce(Steps);<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, MultiLine) {<br>
+ // clang-format off<br>
+ IncrementalTestStep Steps[] =<br>
+ {<br>
+ // Replace a range<br>
+ {<br>
+R"cpp(static [[int<br>
+helloWorld]]()<br>
+{})cpp",<br>
+R"cpp(char<br>
+welcome)cpp"<br>
+ },<br>
+ // Delete a range<br>
+ {<br>
+R"cpp(static char[[<br>
+welcome]]()<br>
+{})cpp",<br>
+ ""<br>
+ },<br>
+ // Add a range<br>
+ {<br>
+R"cpp(static char[[]]()<br>
+{})cpp",<br>
+ R"cpp(<br>
+cookies)cpp"<br>
+ },<br>
+ // Replace the whole file<br>
+ {<br>
+R"cpp([[static char<br>
+cookies()<br>
+{}]])cpp",<br>
+ R"cpp(#include <stdio.h><br>
+)cpp"<br>
+ },<br>
+ // Delete the whole file<br>
+ {<br>
+ R"cpp([[#include <stdio.h><br>
+]])cpp",<br>
+ "",<br>
+ },<br>
+ // Add something to an empty file<br>
+ {<br>
+ "[[]]",<br>
+ R"cpp(int main() {<br>
+)cpp",<br>
+ },<br>
+ {<br>
+ R"cpp(int main() {<br>
+)cpp",<br>
+ ""<br>
+ }<br>
+ };<br>
+ // clang-format on<br>
+<br>
+ stepByStep(Steps);<br>
+ allAtOnce(Steps);<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ DS.addDraft(File, "int main() {}\n");<br>
+<br>
+ TextDocumentContentChangeEvent Change;<br>
+ Change.range.emplace();<br>
+ Change.range->start.line = 0;<br>
+ Change.range->start.character = 0;<br>
+ Change.range->end.line = 0;<br>
+ Change.range->end.character = 2;<br>
+ Change.rangeLength = 10;<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(<br>
+ llvm::toString(Result.takeError()),<br>
+ "Change's rangeLength (10) doesn't match the computed range length (2).");<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, EndBeforeStart) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ DS.addDraft(File, "int main() {}\n");<br>
+<br>
+ TextDocumentContentChangeEvent Change;<br>
+ Change.range.emplace();<br>
+ Change.range->start.line = 0;<br>
+ Change.range->start.character = 5;<br>
+ Change.range->end.line = 0;<br>
+ Change.range->end.character = 3;<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(llvm::toString(Result.takeError()),<br>
+ "Range's end position (0:3) is before start position (0:5)");<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, StartCharOutOfRange) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ DS.addDraft(File, "int main() {}\n");<br>
+<br>
+ TextDocumentContentChangeEvent Change;<br>
+ Change.range.emplace();<br>
+ Change.range->start.line = 0;<br>
+ Change.range->start.character = 100;<br>
+ Change.range->end.line = 0;<br>
+ Change.range->end.character = 100;<br>
+ Change.text = "foo";<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(llvm::toString(Result.takeError()),<br>
+ "Character value is out of range (100)");<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ DS.addDraft(File, "int main() {}\n");<br>
+<br>
+ TextDocumentContentChangeEvent Change;<br>
+ Change.range.emplace();<br>
+ Change.range->start.line = 0;<br>
+ Change.range->start.character = 0;<br>
+ Change.range->end.line = 0;<br>
+ Change.range->end.character = 100;<br>
+ Change.text = "foo";<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(llvm::toString(Result.takeError()),<br>
+ "Character value is out of range (100)");<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ DS.addDraft(File, "int main() {}\n");<br>
+<br>
+ TextDocumentContentChangeEvent Change;<br>
+ Change.range.emplace();<br>
+ Change.range->start.line = 100;<br>
+ Change.range->start.character = 0;<br>
+ Change.range->end.line = 100;<br>
+ Change.range->end.character = 0;<br>
+ Change.text = "foo";<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(llvm::toString(Result.takeError()),<br>
+ "Line value is out of range (100)");<br>
+}<br>
+<br>
+TEST(DraftStoreIncrementalUpdateTest, EndLineOutOfRange) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ DS.addDraft(File, "int main() {}\n");<br>
+<br>
+ TextDocumentContentChangeEvent Change;<br>
+ Change.range.emplace();<br>
+ Change.range->start.line = 0;<br>
+ Change.range->start.character = 0;<br>
+ Change.range->end.line = 100;<br>
+ Change.range->end.character = 0;<br>
+ Change.text = "foo";<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(llvm::toString(Result.takeError()),<br>
+ "Line value is out of range (100)");<br>
+}<br>
+<br>
+/// Check that if a valid change is followed by an invalid change, the original<br>
+/// version of the document (prior to all changes) is kept.<br>
+TEST(DraftStoreIncrementalUpdateTest, InvalidRangeInASequence) {<br>
+ DraftStore DS;<br>
+ Path File = "foo.cpp";<br>
+<br>
+ StringRef OriginalContents = "int main() {}\n";<br>
+ DS.addDraft(File, OriginalContents);<br>
+<br>
+ // The valid change<br>
+ TextDocumentContentChangeEvent Change1;<br>
+ Change1.range.emplace();<br>
+ Change1.range->start.line = 0;<br>
+ Change1.range->start.character = 0;<br>
+ Change1.range->end.line = 0;<br>
+ Change1.range->end.character = 0;<br>
+ Change1.text = "Hello ";<br>
+<br>
+ // The invalid change<br>
+ TextDocumentContentChangeEvent Change2;<br>
+ Change2.range.emplace();<br>
+ Change2.range->start.line = 0;<br>
+ Change2.range->start.character = 5;<br>
+ Change2.range->end.line = 0;<br>
+ Change2.range->end.character = 100;<br>
+ Change2.text = "something";<br>
+<br>
+ llvm::Expected<std::string> Result = DS.updateDraft(File, {Change1, Change2});<br>
+<br>
+ EXPECT_TRUE(!Result);<br>
+ EXPECT_EQ(llvm::toString(Result.takeError()),<br>
+ "Character value is out of range (100)");<br>
+<br>
+ llvm::Optional<std::string> Contents = DS.getDraft(File);<br>
+ EXPECT_TRUE(Contents);<br>
+ EXPECT_EQ(*Contents, OriginalContents);<br>
+}<br>
+<br>
+} // namespace<br>
+} // namespace clangd<br>
+} // namespace clang<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>