[clang-tools-extra] r328500 - [clangd] Support incremental document syncing
Reid Kleckner via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 27 10:47:20 PDT 2018
One of these new tests does not pass on Windows due to assumptions about
absolute path structure. The FileCheck output seems self-explanatory:
$ "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"
# command stderr:
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
# CHECK-NEXT: "uri": "file:///clangd-test/main.cpp"
^
<stdin>:68:7: note: scanning from here
"uri": "file:///C%3a/clangd-test/main.cpp"
I committed a speculative fix for this in r328645, hopefully it does the
job.
On Mon, Mar 26, 2018 at 7:32 PM Simon Marchi via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180327/a0f780ba/attachment-0001.html>
More information about the cfe-commits
mailing list