[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