diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -194,12 +194,6 @@ /// produce '->' and '::', respectively. bool shouldRunCompletion(const CompletionParams &Params) const; - /// Requests a reparse of currently opened files using their latest source. - /// This will typically only rebuild if something other than the source has - /// changed (e.g. the CDB yields different flags, or files included in the - /// preamble have been modified). - void reparseOpenFilesIfNeeded( - llvm::function_ref Filter); void applyConfiguration(const ConfigurationSettings &Settings); /// Runs profiling and exports memory usage metrics if tracing is enabled and @@ -282,8 +276,6 @@ BackgroundQueue::Stats PendingBackgroundIndexProgress; /// LSP extension: skip WorkDoneProgressCreate, just send progress streams. bool BackgroundIndexSkipCreate = false; - // Store of the current versions of the open documents. - DraftStore DraftMgr; Options Opts; // The CDB is created by the "initialize" LSP method. diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -59,8 +59,8 @@ // LSP defines file versions as numbers that increase. // ClangdServer treats them as opaque and therefore uses strings instead. -std::string encodeVersion(int64_t LSPVersion) { - return llvm::to_string(LSPVersion); +std::string encodeVersion(llvm::Optional LSPVersion) { + return LSPVersion ? llvm::to_string(*LSPVersion) : ""; } llvm::Optional decodeVersion(llvm::StringRef Encoded) { int64_t Result; @@ -124,15 +124,15 @@ // Makes sure edits in \p FE are applicable to latest file contents reported by // editor. If not generates an error message containing information about files // that needs to be saved. -llvm::Error validateEdits(const DraftStore &DraftMgr, const FileEdits &FE) { +llvm::Error validateEdits(const ClangdServer &Server, const FileEdits &FE) { size_t InvalidFileCount = 0; llvm::StringRef LastInvalidFile; for (const auto &It : FE) { - if (auto Draft = DraftMgr.getDraft(It.first())) { + if (auto Draft = Server.getDraft(It.first())) { // 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->Contents)) { + if (!It.second.canApplyTo(*Draft)) { ++InvalidFileCount; LastInvalidFile = It.first(); } @@ -648,8 +648,8 @@ const std::string &Contents = Params.textDocument.text; - auto Version = DraftMgr.addDraft(File, Params.textDocument.version, Contents); - Server->addDocument(File, Contents, encodeVersion(Version), + Server->addDocument(File, Contents, + encodeVersion(Params.textDocument.version), WantDiagnostics::Yes); } @@ -661,25 +661,28 @@ : WantDiagnostics::No; PathRef File = Params.textDocument.uri.file(); - llvm::Expected 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, Draft.takeError()); + auto Code = Server->getDraft(File); + if (!Code) { + log("Trying to incrementally change non-added document: {0}", File); return; } - - Server->addDocument(File, Draft->Contents, encodeVersion(Draft->Version), + for (const auto &Change : Params.contentChanges) { + if (auto Err = applyChange(*Code, Change)) { + // 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. + Server->removeDocument(File); + elog("Failed to update {0}: {1}", File, std::move(Err)); + return; + } + } + Server->addDocument(File, *Code, encodeVersion(Params.textDocument.version), WantDiags, Params.forceRebuild); } void ClangdLSPServer::onDocumentDidSave( const DidSaveTextDocumentParams &Params) { - reparseOpenFilesIfNeeded([](llvm::StringRef) { return true; }); + Server->reparseOpenFilesIfNeeded([](llvm::StringRef) { return true; }); } void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { @@ -720,13 +723,8 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args, Callback Reply) { - auto Code = DraftMgr.getDraft(Args.file.file()); - if (!Code) - return Reply(error("trying to apply a code action for a non-added file")); - - auto Action = [this, Reply = std::move(Reply), File = Args.file, - Code = std::move(*Code)]( - llvm::Expected R) mutable { + auto Action = [this, Reply = std::move(Reply), + File = Args.file](llvm::Expected R) mutable { if (!R) return Reply(R.takeError()); @@ -742,7 +740,7 @@ if (R->ApplyEdits.empty()) return Reply("Tweak applied."); - if (auto Err = validateEdits(DraftMgr, R->ApplyEdits)) + if (auto Err = validateEdits(*Server, R->ApplyEdits)) return Reply(std::move(Err)); WorkspaceEdit WE; @@ -808,7 +806,7 @@ void ClangdLSPServer::onRename(const RenameParams &Params, Callback Reply) { Path File = std::string(Params.textDocument.uri.file()); - if (!DraftMgr.getDraft(File)) + if (!Server->getDraft(File)) return Reply(llvm::make_error( "onRename called for non-added file", ErrorCode::InvalidParams)); Server->rename( @@ -817,7 +815,7 @@ this](llvm::Expected R) mutable { if (!R) return Reply(R.takeError()); - if (auto Err = validateEdits(DraftMgr, R->GlobalChanges)) + if (auto Err = validateEdits(*Server, R->GlobalChanges)) return Reply(std::move(Err)); WorkspaceEdit Result; Result.changes.emplace(); @@ -832,7 +830,6 @@ void ClangdLSPServer::onDocumentDidClose( const DidCloseTextDocumentParams &Params) { PathRef File = Params.textDocument.uri.file(); - DraftMgr.removeDraft(File); Server->removeDocument(File); { @@ -857,52 +854,35 @@ const DocumentOnTypeFormattingParams &Params, Callback> Reply) { auto File = Params.textDocument.uri.file(); - auto Code = DraftMgr.getDraft(File); - if (!Code) - return Reply(llvm::make_error( - "onDocumentOnTypeFormatting called for non-added file", - ErrorCode::InvalidParams)); - - Server->formatOnType(File, Code->Contents, Params.position, Params.ch, - std::move(Reply)); + Server->formatOnType(File, Params.position, Params.ch, std::move(Reply)); } void ClangdLSPServer::onDocumentRangeFormatting( const DocumentRangeFormattingParams &Params, Callback> Reply) { auto File = Params.textDocument.uri.file(); - auto Code = DraftMgr.getDraft(File); - if (!Code) - return Reply(llvm::make_error( - "onDocumentRangeFormatting called for non-added file", - ErrorCode::InvalidParams)); - - Server->formatRange( - File, Code->Contents, Params.range, - [Code = Code->Contents, Reply = std::move(Reply)]( - llvm::Expected Result) mutable { - if (Result) - Reply(replacementsToEdits(Code, Result.get())); - else - Reply(Result.takeError()); - }); + auto Code = Server->getDraft(File); + Server->formatFile(File, Params.range, + [Code = std::move(Code), Reply = std::move(Reply)]( + llvm::Expected Result) mutable { + if (Result) + Reply(replacementsToEdits(*Code, Result.get())); + else + Reply(Result.takeError()); + }); } void ClangdLSPServer::onDocumentFormatting( const DocumentFormattingParams &Params, Callback> Reply) { auto File = Params.textDocument.uri.file(); - auto Code = DraftMgr.getDraft(File); - if (!Code) - return Reply(llvm::make_error( - "onDocumentFormatting called for non-added file", - ErrorCode::InvalidParams)); - - Server->formatFile(File, Code->Contents, - [Code = Code->Contents, Reply = std::move(Reply)]( + auto Code = Server->getDraft(File); + Server->formatFile(File, + /*Rng=*/llvm::None, + [Code = std::move(Code), Reply = std::move(Reply)]( llvm::Expected Result) mutable { if (Result) - Reply(replacementsToEdits(Code, Result.get())); + Reply(replacementsToEdits(*Code, Result.get())); else Reply(Result.takeError()); }); @@ -978,11 +958,6 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, Callback Reply) { URIForFile File = Params.textDocument.uri; - auto Code = DraftMgr.getDraft(File.file()); - if (!Code) - return Reply(llvm::make_error( - "onCodeAction called for non-added file", ErrorCode::InvalidParams)); - // Checks whether a particular CodeActionKind is included in the response. auto KindAllowed = [Only(Params.context.only)](llvm::StringRef Kind) { if (Only.empty()) @@ -1005,8 +980,8 @@ // Now enumerate the semantic code actions. auto ConsumeActions = - [Reply = std::move(Reply), File, Code = std::move(*Code), - Selection = Params.range, FixIts = std::move(FixIts), this]( + [Reply = std::move(Reply), File, Selection = Params.range, + FixIts = std::move(FixIts), this]( llvm::Expected> Tweaks) mutable { if (!Tweaks) return Reply(Tweaks.takeError()); @@ -1246,7 +1221,7 @@ } } - reparseOpenFilesIfNeeded( + Server->reparseOpenFilesIfNeeded( [&](llvm::StringRef File) { return ModifiedFiles.count(File) != 0; }); } @@ -1557,17 +1532,17 @@ const CompletionParams &Params) const { if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter) return true; - auto Code = DraftMgr.getDraft(Params.textDocument.uri.file()); + auto Code = Server->getDraft(Params.textDocument.uri.file()); if (!Code) return true; // completion code will log the error for untracked doc. - auto Offset = positionToOffset(Code->Contents, Params.position, + auto Offset = positionToOffset(*Code, Params.position, /*AllowColumnsBeyondLineLength=*/false); if (!Offset) { vlog("could not convert position '{0}' to offset for file '{1}'", Params.position, Params.textDocument.uri.file()); return true; } - return allowImplicitCompletion(Code->Contents, *Offset); + return allowImplicitCompletion(*Code, *Offset); } void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version, @@ -1681,16 +1656,5 @@ NotifyFileStatus(Status.render(File)); } -void ClangdLSPServer::reparseOpenFilesIfNeeded( - llvm::function_ref Filter) { - // Reparse only opened files that were modified. - for (const Path &FilePath : DraftMgr.getActiveFiles()) - if (Filter(FilePath)) - if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race? - Server->addDocument(FilePath, std::move(Draft->Contents), - encodeVersion(Draft->Version), - WantDiagnostics::Auto); -} - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -12,6 +12,7 @@ #include "../clang-tidy/ClangTidyOptions.h" #include "CodeComplete.h" #include "ConfigProvider.h" +#include "DraftStore.h" #include "GlobalCompilationDatabase.h" #include "Hover.h" #include "Module.h" @@ -176,7 +177,7 @@ /// separate thread. When the parsing is complete, DiagConsumer passed in /// constructor will receive onDiagnosticsReady callback. /// Version identifies this snapshot and is propagated to ASTs, preambles, - /// diagnostics etc built from it. + /// diagnostics etc built from it. If empty, a version number is generated. void addDocument(PathRef File, StringRef Contents, llvm::StringRef Version = "null", WantDiagnostics WD = WantDiagnostics::Auto, @@ -188,6 +189,13 @@ /// An empty set of diagnostics will be delivered, with Version = "". void removeDocument(PathRef File); + /// Requests a reparse of currently opened files using their latest source. + /// This will typically only rebuild if something other than the source has + /// changed (e.g. the CDB yields different flags, or files included in the + /// preamble have been modified). + void reparseOpenFilesIfNeeded( + llvm::function_ref Filter); + /// Run code completion for \p File at \p Pos. /// /// This method should only be called for currently tracked files. @@ -253,18 +261,15 @@ void findReferences(PathRef File, Position Pos, uint32_t Limit, Callback CB); - /// Run formatting for \p Rng inside \p File with content \p Code. - void formatRange(PathRef File, StringRef Code, Range Rng, - Callback CB); - - /// Run formatting for the whole \p File with content \p Code. - void formatFile(PathRef File, StringRef Code, + /// Run formatting for the \p File with content \p Code. + /// If \p Rng is non-null, formats only that region. + void formatFile(PathRef File, llvm::Optional Rng, Callback CB); /// Run formatting after \p TriggerText was typed at \p Pos in \p File with /// content \p Code. - void formatOnType(PathRef File, StringRef Code, Position Pos, - StringRef TriggerText, Callback> CB); + void formatOnType(PathRef File, Position Pos, StringRef TriggerText, + Callback> CB); /// Test the validity of a rename operation. /// @@ -334,6 +339,8 @@ /// FIXME: those metrics might be useful too, we should add them. llvm::StringMap fileStats() const; + llvm::Optional getDraft(PathRef File) const; + // Blocks the main thread until the server is idle. Only for use in tests. // Returns false if the timeout expires. // FIXME: various subcomponents each get the full timeout, so it's more of @@ -345,10 +352,6 @@ void profile(MemoryTree &MT) const; private: - void formatCode(PathRef File, llvm::StringRef Code, - ArrayRef Ranges, - Callback CB); - ModuleSet *Modules; const GlobalCompilationDatabase &CDB; const ThreadsafeFS &TFS; @@ -377,6 +380,11 @@ llvm::Optional WorkspaceRoot; llvm::Optional WorkScheduler; + + // Store of the current versions of the open documents. + // Only written from the main thread (despite being threadsafe). + // FIXME: TUScheduler also keeps these, unify? + DraftStore DraftMgr; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -194,13 +194,14 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, llvm::StringRef Version, WantDiagnostics WantDiags, bool ForceRebuild) { + std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents); ParseOptions Opts; // Compile command is set asynchronously during update, as it can be slow. ParseInputs Inputs; Inputs.TFS = &TFS; Inputs.Contents = std::string(Contents); - Inputs.Version = Version.str(); + Inputs.Version = std::move(ActualVersion); Inputs.ForceRebuild = ForceRebuild; Inputs.Opts = std::move(Opts); Inputs.Index = Index; @@ -211,6 +212,23 @@ BackgroundIdx->boostRelated(File); } +void ClangdServer::reparseOpenFilesIfNeeded( + llvm::function_ref Filter) { + // Reparse only opened files that were modified. + for (const Path &FilePath : DraftMgr.getActiveFiles()) + if (Filter(FilePath)) + if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race? + addDocument(FilePath, std::move(Draft->Contents), Draft->Version, + WantDiagnostics::Auto); +} + +llvm::Optional ClangdServer::getDraft(PathRef File) const { + auto Draft = DraftMgr.getDraft(File); + if (!Draft) + return llvm::None; + return std::move(Draft->Contents); +} + std::function ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, Callbacks *Publish) { @@ -288,7 +306,10 @@ }; } -void ClangdServer::removeDocument(PathRef File) { WorkScheduler->remove(File); } +void ClangdServer::removeDocument(PathRef File) { + DraftMgr.removeDraft(File); + WorkScheduler->remove(File); +} void ClangdServer::codeComplete(PathRef File, Position Pos, const clangd::CodeCompleteOptions &Opts, @@ -372,31 +393,55 @@ std::move(Action)); } -void ClangdServer::formatRange(PathRef File, llvm::StringRef Code, Range Rng, - Callback CB) { - llvm::Expected Begin = positionToOffset(Code, Rng.start); - if (!Begin) - return CB(Begin.takeError()); - llvm::Expected End = positionToOffset(Code, Rng.end); - if (!End) - return CB(End.takeError()); - formatCode(File, Code, {tooling::Range(*Begin, *End - *Begin)}, - std::move(CB)); -} - -void ClangdServer::formatFile(PathRef File, llvm::StringRef Code, +void ClangdServer::formatFile(PathRef File, llvm::Optional Rng, Callback CB) { - // Format everything. - formatCode(File, Code, {tooling::Range(0, Code.size())}, std::move(CB)); + auto Code = getDraft(File); + if (!Code) + return CB(llvm::make_error("trying to format non-added document", + ErrorCode::InvalidParams)); + tooling::Range RequestedRange; + if (Rng) { + llvm::Expected Begin = positionToOffset(*Code, Rng->start); + if (!Begin) + return CB(Begin.takeError()); + llvm::Expected End = positionToOffset(*Code, Rng->end); + if (!End) + return CB(End.takeError()); + RequestedRange = tooling::Range(*Begin, *End - *Begin); + } else { + RequestedRange = tooling::Range(0, Code->size()); + } + + // Call clang-format. + auto Action = [File = File.str(), Code = std::move(*Code), + Ranges = std::vector{RequestedRange}, + CB = std::move(CB), this]() mutable { + format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS); + tooling::Replacements IncludeReplaces = + format::sortIncludes(Style, Code, Ranges, File); + auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces); + if (!Changed) + return CB(Changed.takeError()); + + CB(IncludeReplaces.merge(format::reformat( + Style, *Changed, + tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges), + File))); + }; + WorkScheduler->runQuick("Format", File, std::move(Action)); } -void ClangdServer::formatOnType(PathRef File, llvm::StringRef Code, - Position Pos, StringRef TriggerText, +void ClangdServer::formatOnType(PathRef File, Position Pos, + StringRef TriggerText, Callback> CB) { - llvm::Expected CursorPos = positionToOffset(Code, Pos); + auto Code = getDraft(File); + if (!Code) + return CB(llvm::make_error("trying to format non-added document", + ErrorCode::InvalidParams)); + llvm::Expected CursorPos = positionToOffset(*Code, Pos); if (!CursorPos) return CB(CursorPos.takeError()); - auto Action = [File = File.str(), Code = Code.str(), + auto Action = [File = File.str(), Code = std::move(*Code), TriggerText = TriggerText.str(), CursorPos = *CursorPos, CB = std::move(CB), this]() mutable { auto Style = format::getStyle(format::DefaultFormatStyle, File, @@ -616,27 +661,6 @@ WorkScheduler->runWithAST("SwitchHeaderSource", Path, std::move(Action)); } -void ClangdServer::formatCode(PathRef File, llvm::StringRef Code, - llvm::ArrayRef Ranges, - Callback CB) { - // Call clang-format. - auto Action = [File = File.str(), Code = Code.str(), Ranges = Ranges.vec(), - CB = std::move(CB), this]() mutable { - format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS); - tooling::Replacements IncludeReplaces = - format::sortIncludes(Style, Code, Ranges, File); - auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces); - if (!Changed) - return CB(Changed.takeError()); - - CB(IncludeReplaces.merge(format::reformat( - Style, *Changed, - tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges), - File))); - }; - WorkScheduler->runQuick("Format", File, std::move(Action)); -} - void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { auto Action = diff --git a/clang-tools-extra/clangd/DraftStore.h b/clang-tools-extra/clangd/DraftStore.h --- a/clang-tools-extra/clangd/DraftStore.h +++ b/clang-tools-extra/clangd/DraftStore.h @@ -21,15 +21,14 @@ 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 +/// filenames. The contents are owned by the DraftStore. +/// Each time a draft is updated, it is assigned a version. 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; + std::string Version; }; /// \return Contents of the stored document. @@ -40,21 +39,10 @@ std::vector getActiveFiles() const; /// Replace contents of the draft for \p File with \p Contents. - /// If no version is specified, one will be automatically assigned. + /// If version is empty, one will be automatically assigned. /// Returns the version. - int64_t addDraft(PathRef File, llvm::Optional 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 - updateDraft(PathRef File, llvm::Optional Version, - llvm::ArrayRef Changes); + std::string addDraft(PathRef File, llvm::StringRef Version, + StringRef Contents); /// Remove the draft from the store. void removeDraft(PathRef File); diff --git a/clang-tools-extra/clangd/DraftStore.cpp b/clang-tools-extra/clangd/DraftStore.cpp --- a/clang-tools-extra/clangd/DraftStore.cpp +++ b/clang-tools-extra/clangd/DraftStore.cpp @@ -9,6 +9,7 @@ #include "DraftStore.h" #include "SourceCode.h" #include "support/Logger.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Support/Errc.h" namespace clang { @@ -34,21 +35,44 @@ return ResultVector; } +static void increment(std::string &S) { + // Ensure there is a numeric suffix. + if (S.empty() || !llvm::isDigit(S.back())) { + S.push_back('0'); + return; + } + // Increment the numeric suffix. + auto I = S.rbegin(), E = S.rend(); + for (;;) { + if (I == E || !llvm::isDigit(*I)) { + // Reached start of numeric section, it was all 9s. + S.insert(I.base(), '1'); + break; + } + if (*I != '9') { + // Found a digit we can increment, we're done. + ++*I; + break; + } + *I = '0'; // and keep incrementing to the left. + } +} + static void updateVersion(DraftStore::Draft &D, - llvm::Optional Version) { - if (Version) { + llvm::StringRef SpecifiedVersion) { + if (!SpecifiedVersion.empty()) { // 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; + if (SpecifiedVersion.compare_numeric(D.Version) <= 0) + log("File version went from {0} to {1}", D.Version, SpecifiedVersion); + D.Version = SpecifiedVersion.str(); } else { - // Note that if D was newly-created, this will bump D.Version from -1 to 0. - ++D.Version; + // Note that if D was newly-created, this will bump D.Version from "" to 1. + increment(D.Version); } } -int64_t DraftStore::addDraft(PathRef File, llvm::Optional Version, - llvm::StringRef Contents) { +std::string DraftStore::addDraft(PathRef File, llvm::StringRef Version, + llvm::StringRef Contents) { std::lock_guard Lock(Mutex); Draft &D = Drafts[File]; @@ -57,74 +81,6 @@ return D.Version; } -llvm::Expected DraftStore::updateDraft( - PathRef File, llvm::Optional Version, - llvm::ArrayRef Changes) { - std::lock_guard Lock(Mutex); - - auto EntryIt = Drafts.find(File); - if (EntryIt == Drafts.end()) { - return error(llvm::errc::invalid_argument, - "Trying to do incremental update on non-added document: {0}", - File); - } - Draft &D = EntryIt->second; - std::string Contents = EntryIt->second.Contents; - - for (const TextDocumentContentChangeEvent &Change : Changes) { - if (!Change.range) { - Contents = Change.text; - continue; - } - - const Position &Start = Change.range->start; - llvm::Expected StartIndex = - positionToOffset(Contents, Start, false); - if (!StartIndex) - return StartIndex.takeError(); - - const Position &End = Change.range->end; - llvm::Expected EndIndex = positionToOffset(Contents, End, false); - if (!EndIndex) - return EndIndex.takeError(); - - if (*EndIndex < *StartIndex) - return error(llvm::errc::invalid_argument, - "Range's end position ({0}) is before start position ({1})", - End, Start); - - // Since the range length between two LSP positions is dependent on the - // contents of the buffer we compute the range length between the start and - // end position ourselves and compare it to the range length of the LSP - // message to verify the buffers of the client and server are in sync. - - // EndIndex and StartIndex are in bytes, but Change.rangeLength is in UTF-16 - // code units. - ssize_t ComputedRangeLength = - lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex)); - - if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength) - return error(llvm::errc::invalid_argument, - "Change's rangeLength ({0}) doesn't match the " - "computed range length ({1}).", - *Change.rangeLength, ComputedRangeLength); - - 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); - } - - updateVersion(D, Version); - D.Contents = std::move(Contents); - return D; -} - void DraftStore::removeDraft(PathRef File) { std::lock_guard Lock(Mutex); diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h --- a/clang-tools-extra/clangd/SourceCode.h +++ b/clang-tools-extra/clangd/SourceCode.h @@ -203,6 +203,10 @@ /// Replacements to formatted ones if succeeds. llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style); +/// Apply an incremental update to a text document. +llvm::Error applyChange(std::string &Contents, + const TextDocumentContentChangeEvent &Change); + /// Collects identifiers with counts in the source code. llvm::StringMap collectIdentifiers(llvm::StringRef Content, const format::FormatStyle &Style); diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -1053,6 +1053,56 @@ return llvm::Error::success(); } +llvm::Error applyChange(std::string &Contents, + const TextDocumentContentChangeEvent &Change) { + if (!Change.range) { + Contents = Change.text; + return llvm::Error::success(); + } + + const Position &Start = Change.range->start; + llvm::Expected StartIndex = positionToOffset(Contents, Start, false); + if (!StartIndex) + return StartIndex.takeError(); + + const Position &End = Change.range->end; + llvm::Expected EndIndex = positionToOffset(Contents, End, false); + if (!EndIndex) + return EndIndex.takeError(); + + if (*EndIndex < *StartIndex) + return error(llvm::errc::invalid_argument, + "Range's end position ({0}) is before start position ({1})", + End, Start); + + // Since the range length between two LSP positions is dependent on the + // contents of the buffer we compute the range length between the start and + // end position ourselves and compare it to the range length of the LSP + // message to verify the buffers of the client and server are in sync. + + // EndIndex and StartIndex are in bytes, but Change.rangeLength is in UTF-16 + // code units. + ssize_t ComputedRangeLength = + lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex)); + + if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength) + return error(llvm::errc::invalid_argument, + "Change's rangeLength ({0}) doesn't match the " + "computed range length ({1}).", + *Change.rangeLength, ComputedRangeLength); + + std::string NewContents; + NewContents.reserve(*StartIndex + Change.text.length() + + (Contents.length() - *EndIndex)); + + NewContents = Contents.substr(0, *StartIndex); + NewContents += Change.text; + NewContents += Contents.substr(*EndIndex); + + std::swap(Contents, NewContents); + return llvm::Error::success(); +} + EligibleRegion getEligiblePoints(llvm::StringRef Code, llvm::StringRef FullyQualifiedName, const LangOptions &LangOpts) { diff --git a/clang-tools-extra/clangd/test/crash-non-added-files.test b/clang-tools-extra/clangd/test/crash-non-added-files.test --- a/clang-tools-extra/clangd/test/crash-non-added-files.test +++ b/clang-tools-extra/clangd/test/crash-non-added-files.test @@ -4,28 +4,28 @@ {"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}} # CHECK: "error": { # CHECK-NEXT: "code": -32602 -# CHECK-NEXT: "message": "onCodeAction called for non-added file" +# CHECK-NEXT: "message": "trying to get AST for non-added document" # CHECK-NEXT: }, # CHECK-NEXT: "id": 2, --- {"jsonrpc":"2.0","id":3,"method":"textDocument/rangeFormatting","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":1,"character":4},"end":{"line":1,"character":12}},"options":{"tabSize":4,"insertSpaces":true}}} # CHECK: "error": { # CHECK-NEXT: "code": -32602 -# CHECK-NEXT: "message": "onDocumentRangeFormatting called for non-added file" +# CHECK-NEXT: "message": "trying to format non-added document" # CHECK-NEXT: }, # CHECK-NEXT: "id": 3, --- {"jsonrpc":"2.0","id":4,"method":"textDocument/formatting","params":{"textDocument":{"uri":"test:///foo.c"},"options":{"tabSize":4,"insertSpaces":true}}} # CHECK: "error": { # CHECK-NEXT: "code": -32602 -# CHECK-NEXT: "message": "onDocumentFormatting called for non-added file" +# CHECK-NEXT: "message": "trying to format non-added document" # CHECK-NEXT: }, # CHECK-NEXT: "id": 4, --- {"jsonrpc":"2.0","id":5,"method":"textDocument/onTypeFormatting","params":{"textDocument":{"uri":"test:///foo.c"},"position":{"line":3,"character":1},"ch":"}","options":{"tabSize":4,"insertSpaces":true}}} # CHECK: "error": { # CHECK-NEXT: "code": -32602 -# CHECK-NEXT: "message": "onDocumentOnTypeFormatting called for non-added file" +# CHECK-NEXT: "message": "trying to format non-added document" # CHECK-NEXT: }, # CHECK-NEXT: "id": 5, --- diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -945,7 +945,7 @@ FS.Files[Path] = Code; runAddDocument(Server, Path, Code); - auto Replaces = runFormatFile(Server, Path, Code); + auto Replaces = runFormatFile(Server, Path, /*Rng=*/llvm::None); EXPECT_TRUE(static_cast(Replaces)); auto Changed = tooling::applyAllReplacements(Code, *Replaces); EXPECT_TRUE(static_cast(Changed)); diff --git a/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp b/clang-tools-extra/clangd/unittests/DraftStoreTests.cpp --- 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/Support/ScopedPrinter.h" #include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -17,371 +18,26 @@ namespace clangd { namespace { -struct IncrementalTestStep { - llvm::StringRef Src; - llvm::StringRef Contents; -}; - -int rangeLength(llvm::StringRef Code, const Range &Rng) { - llvm::Expected Start = positionToOffset(Code, Rng.start); - llvm::Expected 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 Steps) { - DraftStore DS; - Annotations InitialSrc(Steps.front().Src); - constexpr llvm::StringLiteral Path("/hello.cpp"); - - // Set the initial content. - 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); - Annotations SrcAfter(Steps[i].Src); - llvm::StringRef Contents = Steps[i - 1].Contents; - TextDocumentContentChangeEvent Event{ - SrcBefore.range(), - rangeLength(SrcBefore.code(), SrcBefore.range()), - Contents.str(), - }; - - llvm::Expected Result = - DS.updateDraft(Path, llvm::None, {Event}); - ASSERT_TRUE(!!Result); - EXPECT_EQ(Result->Contents, SrcAfter.code()); - EXPECT_EQ(DS.getDraft(Path)->Contents, SrcAfter.code()); - EXPECT_EQ(Result->Version, static_cast(i)); - } -} - -/// Send all the changes at once to updateDraft, check only the final result. -void allAtOnce(llvm::ArrayRef Steps) { - DraftStore DS; - Annotations InitialSrc(Steps.front().Src); - Annotations FinalSrc(Steps.back().Src); - constexpr llvm::StringLiteral Path("/hello.cpp"); - std::vector Changes; - - for (size_t i = 0; i < Steps.size() - 1; i++) { - Annotations Src(Steps[i].Src); - llvm::StringRef Contents = Steps[i].Contents; - - Changes.push_back({ - Src.range(), - rangeLength(Src.code(), Src.range()), - Contents.str(), - }); - } - - // Set the initial content. - EXPECT_EQ(0, DS.addDraft(Path, llvm::None, InitialSrc.code())); - - llvm::Expected Result = - DS.updateDraft(Path, llvm::None, Changes); - - ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError()); - EXPECT_EQ(Result->Contents, FinalSrc.code()); - EXPECT_EQ(DS.getDraft(Path)->Contents, FinalSrc.code()); - EXPECT_EQ(Result->Version, 1); -} - -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 -)cpp" - }, - // Delete the whole file - { - R"cpp([[#include -]])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, llvm::None, "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; - - Expected Result = - DS.updateDraft(File, llvm::None, {Change}); - - EXPECT_TRUE(!Result); - EXPECT_EQ( - 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, llvm::None, "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; - - auto Result = DS.updateDraft(File, llvm::None, {Change}); - - EXPECT_TRUE(!Result); - EXPECT_EQ(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, llvm::None, "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"; - - auto Result = DS.updateDraft(File, llvm::None, {Change}); - - EXPECT_TRUE(!Result); - EXPECT_EQ(toString(Result.takeError()), - "utf-16 offset 100 is invalid for line 0"); -} - -TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) { - DraftStore DS; - Path File = "foo.cpp"; - - DS.addDraft(File, llvm::None, "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"; - - auto Result = DS.updateDraft(File, llvm::None, {Change}); - - EXPECT_TRUE(!Result); - EXPECT_EQ(toString(Result.takeError()), - "utf-16 offset 100 is invalid for line 0"); -} - -TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) { - DraftStore DS; - Path File = "foo.cpp"; - - DS.addDraft(File, llvm::None, "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"; - - auto Result = DS.updateDraft(File, llvm::None, {Change}); - - EXPECT_TRUE(!Result); - EXPECT_EQ(toString(Result.takeError()), "Line value is out of range (100)"); -} - -TEST(DraftStoreIncrementalUpdateTest, EndLineOutOfRange) { +TEST(DraftStore, Versions) { DraftStore DS; Path File = "foo.cpp"; - DS.addDraft(File, llvm::None, "int main() {}\n"); + EXPECT_EQ("25", DS.addDraft(File, "25", "")); + EXPECT_EQ("25", DS.getDraft(File)->Version); + EXPECT_EQ("", DS.getDraft(File)->Contents); - 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"; + EXPECT_EQ("26", DS.addDraft(File, "", "x")); + EXPECT_EQ("26", DS.getDraft(File)->Version); + EXPECT_EQ("x", DS.getDraft(File)->Contents); - auto Result = DS.updateDraft(File, llvm::None, {Change}); - - EXPECT_TRUE(!Result); - EXPECT_EQ(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"; - EXPECT_EQ(0, DS.addDraft(File, llvm::None, 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"; - - 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 Contents = DS.getDraft(File); - EXPECT_TRUE(Contents); - 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); + EXPECT_EQ("27", DS.addDraft(File, "", "x")) << "no-op change"; + EXPECT_EQ("27", DS.getDraft(File)->Version); + EXPECT_EQ("x", DS.getDraft(File)->Contents); // 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); + EXPECT_EQ("7", DS.addDraft(File, "7", "y")); + EXPECT_EQ("7", DS.getDraft(File)->Version); + EXPECT_EQ("y", DS.getDraft(File)->Contents); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -27,6 +27,7 @@ namespace { using llvm::Failed; +using llvm::FailedWithMessage; using llvm::HasValue; MATCHER_P2(Pos, Line, Col, "") { @@ -802,6 +803,226 @@ EXPECT_FALSE(isKeyword("override", LangOpts)); } +struct IncrementalTestStep { + llvm::StringRef Src; + llvm::StringRef Contents; +}; + +int rangeLength(llvm::StringRef Code, const Range &Rng) { + llvm::Expected Start = positionToOffset(Code, Rng.start); + llvm::Expected 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 Steps) { + std::string Code = Annotations(Steps.front().Src).code().str(); + + for (size_t I = 1; I < Steps.size(); I++) { + Annotations SrcBefore(Steps[I - 1].Src); + Annotations SrcAfter(Steps[I].Src); + llvm::StringRef Contents = Steps[I - 1].Contents; + TextDocumentContentChangeEvent Event{ + SrcBefore.range(), + rangeLength(SrcBefore.code(), SrcBefore.range()), + Contents.str(), + }; + + EXPECT_THAT_ERROR(applyChange(Code, Event), llvm::Succeeded()); + EXPECT_EQ(Code, SrcAfter.code()); + } +} + +TEST(ApplyEditsTest, 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); +} + +TEST(ApplyEditsTest, 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 +)cpp" + }, + // Delete the whole file + { + R"cpp([[#include +]])cpp", + "", + }, + // Add something to an empty file + { + "[[]]", + R"cpp(int main() { +)cpp", + }, + { + R"cpp(int main() { +)cpp", + "" + } + }; + // clang-format on + + stepByStep(Steps); +} + +TEST(ApplyEditsTest, WrongRangeLength) { + std::string Code = "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; + + EXPECT_THAT_ERROR(applyChange(Code, Change), + FailedWithMessage("Change's rangeLength (10) doesn't match " + "the computed range length (2).")); +} + +TEST(ApplyEditsTest, EndBeforeStart) { + std::string Code = "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; + + EXPECT_THAT_ERROR( + applyChange(Code, Change), + FailedWithMessage( + "Range's end position (0:3) is before start position (0:5)")); +} + +TEST(ApplyEditsTest, StartCharOutOfRange) { + std::string Code = "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"; + + EXPECT_THAT_ERROR( + applyChange(Code, Change), + FailedWithMessage("utf-16 offset 100 is invalid for line 0")); +} + +TEST(ApplyEditsTest, EndCharOutOfRange) { + std::string Code = "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"; + + EXPECT_THAT_ERROR( + applyChange(Code, Change), + FailedWithMessage("utf-16 offset 100 is invalid for line 0")); +} + +TEST(ApplyEditsTest, StartLineOutOfRange) { + std::string Code = "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"; + + EXPECT_THAT_ERROR(applyChange(Code, Change), + FailedWithMessage("Line value is out of range (100)")); +} + +TEST(ApplyEditsTest, EndLineOutOfRange) { + std::string Code = "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"; + + EXPECT_THAT_ERROR(applyChange(Code, Change), + FailedWithMessage("Line value is out of range (100)")); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h --- a/clang-tools-extra/clangd/unittests/SyncAPI.h +++ b/clang-tools-extra/clangd/unittests/SyncAPI.h @@ -50,7 +50,7 @@ const clangd::RenameOptions &RenameOpts); llvm::Expected -runFormatFile(ClangdServer &Server, PathRef File, StringRef Code); +runFormatFile(ClangdServer &Server, PathRef File, llvm::Optional); SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query); SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req); diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp --- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp +++ b/clang-tools-extra/clangd/unittests/SyncAPI.cpp @@ -115,9 +115,9 @@ } llvm::Expected -runFormatFile(ClangdServer &Server, PathRef File, StringRef Code) { +runFormatFile(ClangdServer &Server, PathRef File, llvm::Optional Rng) { llvm::Optional> Result; - Server.formatFile(File, Code, capture(Result)); + Server.formatFile(File, Rng, capture(Result)); return std::move(*Result); }