[clang-tools-extra] caf5a4d - [clangd] Propagate versions into DraftStore, assigning where missing. NFC

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 3 07:20:24 PST 2020


Author: Sam McCall
Date: 2020-03-03T16:20:13+01:00
New Revision: caf5a4d57fe0ac0ca8c8d45fd31e5dbbc6bb6bec

URL: https://github.com/llvm/llvm-project/commit/caf5a4d57fe0ac0ca8c8d45fd31e5dbbc6bb6bec
DIFF: https://github.com/llvm/llvm-project/commit/caf5a4d57fe0ac0ca8c8d45fd31e5dbbc6bb6bec.diff

LOG: [clangd] Propagate versions into DraftStore, assigning where missing. NFC

This prepares for propagating versions through the server so diagnostics
etc can be versioned.

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdLSPServer.cpp
    clang-tools-extra/clangd/DraftStore.cpp
    clang-tools-extra/clangd/DraftStore.h
    clang-tools-extra/clangd/Protocol.cpp
    clang-tools-extra/clangd/Protocol.h
    clang-tools-extra/clangd/unittests/DraftStoreTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 9d93b8592fdc..44288eb7274d 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -117,7 +117,7 @@ llvm::Error validateEdits(const DraftStore &DraftMgr, const FileEdits &FE) {
       // If the file is open in user's editor, make sure the version we
       // saw and current version are compatible as this is the text that
       // will be replaced by editors.
-      if (!It.second.canApplyTo(*Draft)) {
+      if (!It.second.canApplyTo(Draft->Contents)) {
         ++InvalidFileCount;
         LastInvalidFile = It.first();
       }
@@ -630,7 +630,7 @@ void ClangdLSPServer::onDocumentDidOpen(
 
   const std::string &Contents = Params.textDocument.text;
 
-  DraftMgr.addDraft(File, Contents);
+  DraftMgr.addDraft(File, Params.textDocument.version, Contents);
   Server->addDocument(File, Contents, WantDiagnostics::Yes);
 }
 
@@ -642,19 +642,19 @@ void ClangdLSPServer::onDocumentDidChange(
                                                   : WantDiagnostics::No;
 
   PathRef File = Params.textDocument.uri.file();
-  llvm::Expected<std::string> Contents =
-      DraftMgr.updateDraft(File, Params.contentChanges);
-  if (!Contents) {
+  llvm::Expected<DraftStore::Draft> Draft = DraftMgr.updateDraft(
+      File, Params.textDocument.version, Params.contentChanges);
+  if (!Draft) {
     // If this fails, we are most likely going to be not in sync anymore with
     // the client.  It is better to remove the draft and let further operations
     // fail rather than giving wrong results.
     DraftMgr.removeDraft(File);
     Server->removeDocument(File);
-    elog("Failed to update {0}: {1}", File, Contents.takeError());
+    elog("Failed to update {0}: {1}", File, Draft.takeError());
     return;
   }
 
-  Server->addDocument(File, *Contents, WantDiags, Params.forceRebuild);
+  Server->addDocument(File, Draft->Contents, WantDiags, Params.forceRebuild);
 }
 
 void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
@@ -773,8 +773,7 @@ void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
 void ClangdLSPServer::onRename(const RenameParams &Params,
                                Callback<WorkspaceEdit> Reply) {
   Path File = std::string(Params.textDocument.uri.file());
-  llvm::Optional<std::string> Code = DraftMgr.getDraft(File);
-  if (!Code)
+  if (!DraftMgr.getDraft(File))
     return Reply(llvm::make_error<LSPError>(
         "onRename called for non-added file", ErrorCode::InvalidParams));
   Server->rename(
@@ -829,7 +828,7 @@ void ClangdLSPServer::onDocumentOnTypeFormatting(
         "onDocumentOnTypeFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  Reply(Server->formatOnType(*Code, File, Params.position, Params.ch));
+  Reply(Server->formatOnType(Code->Contents, File, Params.position, Params.ch));
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(
@@ -842,9 +841,10 @@ void ClangdLSPServer::onDocumentRangeFormatting(
         "onDocumentRangeFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  auto ReplacementsOrError = Server->formatRange(*Code, File, Params.range);
+  auto ReplacementsOrError =
+      Server->formatRange(Code->Contents, File, Params.range);
   if (ReplacementsOrError)
-    Reply(replacementsToEdits(*Code, ReplacementsOrError.get()));
+    Reply(replacementsToEdits(Code->Contents, ReplacementsOrError.get()));
   else
     Reply(ReplacementsOrError.takeError());
 }
@@ -859,9 +859,9 @@ void ClangdLSPServer::onDocumentFormatting(
         "onDocumentFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  auto ReplacementsOrError = Server->formatFile(*Code, File);
+  auto ReplacementsOrError = Server->formatFile(Code->Contents, File);
   if (ReplacementsOrError)
-    Reply(replacementsToEdits(*Code, ReplacementsOrError.get()));
+    Reply(replacementsToEdits(Code->Contents, ReplacementsOrError.get()));
   else
     Reply(ReplacementsOrError.takeError());
 }
@@ -1328,7 +1328,7 @@ bool ClangdLSPServer::shouldRunCompletion(
   // Running the lexer here would be more robust (e.g. we can detect comments
   // and avoid triggering completion there), but we choose to err on the side
   // of simplicity here.
-  auto Offset = positionToOffset(*Code, Params.position,
+  auto Offset = positionToOffset(Code->Contents, Params.position,
                                  /*AllowColumnsBeyondLineLength=*/false);
   if (!Offset) {
     vlog("could not convert position '{0}' to offset for file '{1}'",
@@ -1339,9 +1339,9 @@ bool ClangdLSPServer::shouldRunCompletion(
     return false;
 
   if (Trigger == ">")
-    return (*Code)[*Offset - 2] == '-'; // trigger only on '->'.
+    return Code->Contents[*Offset - 2] == '-'; // trigger only on '->'.
   if (Trigger == ":")
-    return (*Code)[*Offset - 2] == ':'; // trigger only on '::'.
+    return Code->Contents[*Offset - 2] == ':'; // trigger only on '::'.
   assert(false && "unhandled trigger character");
   return true;
 }
@@ -1475,7 +1475,7 @@ void ClangdLSPServer::reparseOpenedFiles(
   // Reparse only opened files that were modified.
   for (const Path &FilePath : DraftMgr.getActiveFiles())
     if (ModifiedFiles.find(FilePath) != ModifiedFiles.end())
-      Server->addDocument(FilePath, *DraftMgr.getDraft(FilePath),
+      Server->addDocument(FilePath, DraftMgr.getDraft(FilePath)->Contents,
                           WantDiagnostics::Auto);
 }
 

diff  --git a/clang-tools-extra/clangd/DraftStore.cpp b/clang-tools-extra/clangd/DraftStore.cpp
index d4a5d0c73b80..03867dcd286e 100644
--- a/clang-tools-extra/clangd/DraftStore.cpp
+++ b/clang-tools-extra/clangd/DraftStore.cpp
@@ -7,13 +7,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "DraftStore.h"
+#include "Logger.h"
 #include "SourceCode.h"
 #include "llvm/Support/Errc.h"
 
 namespace clang {
 namespace clangd {
 
-llvm::Optional<std::string> DraftStore::getDraft(PathRef File) const {
+llvm::Optional<DraftStore::Draft> DraftStore::getDraft(PathRef File) const {
   std::lock_guard<std::mutex> Lock(Mutex);
 
   auto It = Drafts.find(File);
@@ -33,14 +34,32 @@ std::vector<Path> DraftStore::getActiveFiles() const {
   return ResultVector;
 }
 
-void DraftStore::addDraft(PathRef File, llvm::StringRef Contents) {
+static void updateVersion(DraftStore::Draft &D,
+                          llvm::Optional<int64_t> Version) {
+  if (Version) {
+    // We treat versions as opaque, but the protocol says they increase.
+    if (*Version <= D.Version)
+      log("File version went from {0} to {1}", D.Version, Version);
+    D.Version = *Version;
+  } else {
+    // Note that if D was newly-created, this will bump D.Version from -1 to 0.
+    ++D.Version;
+  }
+}
+
+int64_t DraftStore::addDraft(PathRef File, llvm::Optional<int64_t> Version,
+                         llvm::StringRef Contents) {
   std::lock_guard<std::mutex> Lock(Mutex);
 
-  Drafts[File] = std::string(Contents);
+  Draft &D = Drafts[File];
+  updateVersion(D, Version);
+  D.Contents = Contents.str();
+  return D.Version;
 }
 
-llvm::Expected<std::string> DraftStore::updateDraft(
-    PathRef File, llvm::ArrayRef<TextDocumentContentChangeEvent> Changes) {
+llvm::Expected<DraftStore::Draft> DraftStore::updateDraft(
+    PathRef File, llvm::Optional<int64_t> Version,
+    llvm::ArrayRef<TextDocumentContentChangeEvent> Changes) {
   std::lock_guard<std::mutex> Lock(Mutex);
 
   auto EntryIt = Drafts.find(File);
@@ -49,8 +68,8 @@ llvm::Expected<std::string> DraftStore::updateDraft(
         "Trying to do incremental update on non-added document: " + File,
         llvm::errc::invalid_argument);
   }
-
-  std::string Contents = EntryIt->second;
+  Draft &D = EntryIt->second;
+  std::string Contents = EntryIt->second.Contents;
 
   for (const TextDocumentContentChangeEvent &Change : Changes) {
     if (!Change.range) {
@@ -104,8 +123,9 @@ llvm::Expected<std::string> DraftStore::updateDraft(
     Contents = std::move(NewContents);
   }
 
-  EntryIt->second = Contents;
-  return Contents;
+  updateVersion(D, Version);
+  D.Contents = std::move(Contents);
+  return D;
 }
 
 void DraftStore::removeDraft(PathRef File) {

diff  --git a/clang-tools-extra/clangd/DraftStore.h b/clang-tools-extra/clangd/DraftStore.h
index 1578ce9fad6b..babc679ed763 100644
--- a/clang-tools-extra/clangd/DraftStore.h
+++ b/clang-tools-extra/clangd/DraftStore.h
@@ -23,26 +23,37 @@ namespace clangd {
 /// A thread-safe container for files opened in a workspace, addressed by
 /// filenames. The contents are owned by the DraftStore. This class supports
 /// both whole and incremental updates of the documents.
+/// Each time a draft is updated, it is assigned a version number. This can be
+/// specified by the caller or incremented from the previous version.
 class DraftStore {
 public:
+  struct Draft {
+    std::string Contents;
+    int64_t Version = -1;
+  };
+
   /// \return Contents of the stored document.
   /// For untracked files, a llvm::None is returned.
-  llvm::Optional<std::string> getDraft(PathRef File) const;
+  llvm::Optional<Draft> getDraft(PathRef File) const;
 
   /// \return List of names of the drafts in this store.
   std::vector<Path> getActiveFiles() const;
 
   /// Replace contents of the draft for \p File with \p Contents.
-  void addDraft(PathRef File, StringRef Contents);
+  /// If no version is specified, one will be automatically assigned.
+  /// Returns the version.
+  int64_t addDraft(PathRef File, llvm::Optional<int64_t> Version,
+                   StringRef Contents);
 
   /// Update the contents of the draft for \p File based on \p Changes.
   /// If a position in \p Changes is invalid (e.g. out-of-range), the
   /// draft is not modified.
+  /// If no version is specified, one will be automatically assigned.
   ///
   /// \return The new version of the draft for \p File, or an error if the
   /// changes couldn't be applied.
-  llvm::Expected<std::string>
-  updateDraft(PathRef File,
+  llvm::Expected<Draft>
+  updateDraft(PathRef File, llvm::Optional<int64_t> Version,
               llvm::ArrayRef<TextDocumentContentChangeEvent> Changes);
 
   /// Remove the draft from the store.
@@ -50,7 +61,7 @@ class DraftStore {
 
 private:
   mutable std::mutex Mutex;
-  llvm::StringMap<std::string> Drafts;
+  llvm::StringMap<Draft> Drafts;
 };
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index 5a867c52c1ed..ba96f495c148 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -90,6 +90,20 @@ bool fromJSON(const llvm::json::Value &Params, TextDocumentIdentifier &R) {
   return O && O.map("uri", R.uri);
 }
 
+llvm::json::Value toJSON(const VersionedTextDocumentIdentifier &R) {
+  auto Result = toJSON(static_cast<const TextDocumentIdentifier &>(R));
+  if (R.version)
+    Result.getAsObject()->try_emplace("version", R.version);
+  return Result;
+}
+
+bool fromJSON(const llvm::json::Value &Params,
+              VersionedTextDocumentIdentifier &R) {
+  llvm::json::ObjectMapper O(Params);
+  return fromJSON(Params, static_cast<TextDocumentIdentifier &>(R)) && O &&
+         O.map("version", R.version);
+}
+
 bool fromJSON(const llvm::json::Value &Params, Position &R) {
   llvm::json::ObjectMapper O(Params);
   return O && O.map("line", R.line) && O.map("character", R.character);

diff  --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 596c7e9004e7..5d3914da699f 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -124,6 +124,20 @@ struct TextDocumentIdentifier {
 llvm::json::Value toJSON(const TextDocumentIdentifier &);
 bool fromJSON(const llvm::json::Value &, TextDocumentIdentifier &);
 
+struct VersionedTextDocumentIdentifier : public TextDocumentIdentifier {
+  // The version number of this document. If a versioned text document
+  // identifier is sent from the server to the client and the file is not open
+  // in the editor (the server has not received an open notification before) the
+  // server can send `null` to indicate that the version is known and the
+  // content on disk is the master (as speced with document content ownership).
+  //
+  // The version number of a document will increase after each change, including
+  // undo/redo. The number doesn't need to be consecutive.
+  llvm::Optional<std::int64_t> version;
+};
+llvm::json::Value toJSON(const VersionedTextDocumentIdentifier &);
+bool fromJSON(const llvm::json::Value &, VersionedTextDocumentIdentifier &);
+
 struct Position {
   /// Line position in a document (zero-based).
   int line = 0;
@@ -223,7 +237,7 @@ struct TextDocumentItem {
   std::string languageId;
 
   /// The version number of this document (it will strictly increase after each
-  int version = 0;
+  std::int64_t version = 0;
 
   /// The content of the opened text document.
   std::string text;
@@ -643,7 +657,7 @@ struct DidChangeTextDocumentParams {
   /// The document that did change. The version number points
   /// to the version after all provided content changes have
   /// been applied.
-  TextDocumentIdentifier textDocument;
+  VersionedTextDocumentIdentifier textDocument;
 
   /// The actual content changes.
   std::vector<TextDocumentContentChangeEvent> contentChanges;

diff  --git a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
index 1840892cd5e9..5ff9f254b55d 100644
--- a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "DraftStore.h"
 #include "SourceCode.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -36,7 +37,7 @@ void stepByStep(llvm::ArrayRef<IncrementalTestStep> Steps) {
   constexpr llvm::StringLiteral Path("/hello.cpp");
 
   // Set the initial content.
-  DS.addDraft(Path, InitialSrc.code());
+  EXPECT_EQ(0, DS.addDraft(Path, llvm::None, InitialSrc.code()));
 
   for (size_t i = 1; i < Steps.size(); i++) {
     Annotations SrcBefore(Steps[i - 1].Src);
@@ -48,10 +49,12 @@ void stepByStep(llvm::ArrayRef<IncrementalTestStep> Steps) {
         Contents.str(),
     };
 
-    llvm::Expected<std::string> Result = DS.updateDraft(Path, {Event});
+    llvm::Expected<DraftStore::Draft> Result =
+        DS.updateDraft(Path, llvm::None, {Event});
     ASSERT_TRUE(!!Result);
-    EXPECT_EQ(*Result, SrcAfter.code());
-    EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());
+    EXPECT_EQ(Result->Contents, SrcAfter.code());
+    EXPECT_EQ(DS.getDraft(Path)->Contents, SrcAfter.code());
+    EXPECT_EQ(Result->Version, static_cast<int64_t>(i));
   }
 }
 
@@ -75,13 +78,15 @@ void allAtOnce(llvm::ArrayRef<IncrementalTestStep> Steps) {
   }
 
   // Set the initial content.
-  DS.addDraft(Path, InitialSrc.code());
+  EXPECT_EQ(0, DS.addDraft(Path, llvm::None, InitialSrc.code()));
 
-  llvm::Expected<std::string> Result = DS.updateDraft(Path, Changes);
+  llvm::Expected<DraftStore::Draft> Result =
+      DS.updateDraft(Path, llvm::None, Changes);
 
   ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError());
-  EXPECT_EQ(*Result, FinalSrc.code());
-  EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());
+  EXPECT_EQ(Result->Contents, FinalSrc.code());
+  EXPECT_EQ(DS.getDraft(Path)->Contents, FinalSrc.code());
+  EXPECT_EQ(Result->Version, 1);
 }
 
 TEST(DraftStoreIncrementalUpdateTest, Simple) {
@@ -184,7 +189,7 @@ TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
   DraftStore DS;
   Path File = "foo.cpp";
 
-  DS.addDraft(File, "int main() {}\n");
+  DS.addDraft(File, llvm::None, "int main() {}\n");
 
   TextDocumentContentChangeEvent Change;
   Change.range.emplace();
@@ -194,7 +199,8 @@ TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
   Change.range->end.character = 2;
   Change.rangeLength = 10;
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change});
+  Expected<DraftStore::Draft> Result =
+      DS.updateDraft(File, llvm::None, {Change});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(
@@ -206,7 +212,7 @@ TEST(DraftStoreIncrementalUpdateTest, EndBeforeStart) {
   DraftStore DS;
   Path File = "foo.cpp";
 
-  DS.addDraft(File, "int main() {}\n");
+  DS.addDraft(File, llvm::None, "int main() {}\n");
 
   TextDocumentContentChangeEvent Change;
   Change.range.emplace();
@@ -215,7 +221,7 @@ TEST(DraftStoreIncrementalUpdateTest, EndBeforeStart) {
   Change.range->end.line = 0;
   Change.range->end.character = 3;
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change});
+  auto Result = DS.updateDraft(File, llvm::None, {Change});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(toString(Result.takeError()),
@@ -226,7 +232,7 @@ TEST(DraftStoreIncrementalUpdateTest, StartCharOutOfRange) {
   DraftStore DS;
   Path File = "foo.cpp";
 
-  DS.addDraft(File, "int main() {}\n");
+  DS.addDraft(File, llvm::None, "int main() {}\n");
 
   TextDocumentContentChangeEvent Change;
   Change.range.emplace();
@@ -236,7 +242,7 @@ TEST(DraftStoreIncrementalUpdateTest, StartCharOutOfRange) {
   Change.range->end.character = 100;
   Change.text = "foo";
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change});
+  auto Result = DS.updateDraft(File, llvm::None, {Change});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(toString(Result.takeError()),
@@ -247,7 +253,7 @@ TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {
   DraftStore DS;
   Path File = "foo.cpp";
 
-  DS.addDraft(File, "int main() {}\n");
+  DS.addDraft(File, llvm::None, "int main() {}\n");
 
   TextDocumentContentChangeEvent Change;
   Change.range.emplace();
@@ -257,7 +263,7 @@ TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {
   Change.range->end.character = 100;
   Change.text = "foo";
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change});
+  auto Result = DS.updateDraft(File, llvm::None, {Change});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(toString(Result.takeError()),
@@ -268,7 +274,7 @@ TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {
   DraftStore DS;
   Path File = "foo.cpp";
 
-  DS.addDraft(File, "int main() {}\n");
+  DS.addDraft(File, llvm::None, "int main() {}\n");
 
   TextDocumentContentChangeEvent Change;
   Change.range.emplace();
@@ -278,7 +284,7 @@ TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {
   Change.range->end.character = 0;
   Change.text = "foo";
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change});
+  auto Result = DS.updateDraft(File, llvm::None, {Change});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(toString(Result.takeError()), "Line value is out of range (100)");
@@ -288,7 +294,7 @@ TEST(DraftStoreIncrementalUpdateTest, EndLineOutOfRange) {
   DraftStore DS;
   Path File = "foo.cpp";
 
-  DS.addDraft(File, "int main() {}\n");
+  DS.addDraft(File, llvm::None, "int main() {}\n");
 
   TextDocumentContentChangeEvent Change;
   Change.range.emplace();
@@ -298,7 +304,7 @@ TEST(DraftStoreIncrementalUpdateTest, EndLineOutOfRange) {
   Change.range->end.character = 0;
   Change.text = "foo";
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change});
+  auto Result = DS.updateDraft(File, llvm::None, {Change});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(toString(Result.takeError()), "Line value is out of range (100)");
@@ -311,7 +317,7 @@ TEST(DraftStoreIncrementalUpdateTest, InvalidRangeInASequence) {
   Path File = "foo.cpp";
 
   StringRef OriginalContents = "int main() {}\n";
-  DS.addDraft(File, OriginalContents);
+  EXPECT_EQ(0, DS.addDraft(File, llvm::None, OriginalContents));
 
   // The valid change
   TextDocumentContentChangeEvent Change1;
@@ -331,15 +337,51 @@ TEST(DraftStoreIncrementalUpdateTest, InvalidRangeInASequence) {
   Change2.range->end.character = 100;
   Change2.text = "something";
 
-  Expected<std::string> Result = DS.updateDraft(File, {Change1, Change2});
+  auto Result = DS.updateDraft(File, llvm::None, {Change1, Change2});
 
   EXPECT_TRUE(!Result);
   EXPECT_EQ(toString(Result.takeError()),
             "utf-16 offset 100 is invalid for line 0");
 
-  Optional<std::string> Contents = DS.getDraft(File);
+  Optional<DraftStore::Draft> Contents = DS.getDraft(File);
   EXPECT_TRUE(Contents);
-  EXPECT_EQ(*Contents, OriginalContents);
+  EXPECT_EQ(Contents->Contents, OriginalContents);
+  EXPECT_EQ(Contents->Version, 0);
+}
+
+TEST(DraftStore, Version) {
+  DraftStore DS;
+  Path File = "foo.cpp";
+
+  EXPECT_EQ(25, DS.addDraft(File, 25, ""));
+  EXPECT_EQ(25, DS.getDraft(File)->Version);
+
+  EXPECT_EQ(26, DS.addDraft(File, llvm::None, ""));
+  EXPECT_EQ(26, DS.getDraft(File)->Version);
+
+  // We allow versions to go backwards.
+  EXPECT_EQ(7, DS.addDraft(File, 7, ""));
+  EXPECT_EQ(7, DS.getDraft(File)->Version);
+
+  // Valid (no-op) change modifies version.
+  auto Result = DS.updateDraft(File, 10, {});
+  EXPECT_TRUE(!!Result);
+  EXPECT_EQ(10, Result->Version);
+  EXPECT_EQ(10, DS.getDraft(File)->Version);
+
+  Result = DS.updateDraft(File, llvm::None, {});
+  EXPECT_TRUE(!!Result);
+  EXPECT_EQ(11, Result->Version);
+  EXPECT_EQ(11, DS.getDraft(File)->Version);
+
+  TextDocumentContentChangeEvent InvalidChange;
+  InvalidChange.range.emplace();
+  InvalidChange.rangeLength = 99;
+
+  Result = DS.updateDraft(File, 15, {InvalidChange});
+  EXPECT_FALSE(!!Result);
+  consumeError(Result.takeError());
+  EXPECT_EQ(11, DS.getDraft(File)->Version);
 }
 
 } // namespace


        


More information about the cfe-commits mailing list