From dc10bf1a4ed0b34b27284b5260ce5c6cc132bd6f Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Tue, 1 Jun 2021 13:23:59 +0200 Subject: [PATCH] [clangd][Protocol] Drop optional from WorkspaceEdit::changes This is causing weird code patterns in various places and I can't see any difference between None and empty change list. Neither in the current use cases nor in the spec. Differential Revision: https://reviews.llvm.org/D103449 --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 33 +++++++++----------- clang-tools-extra/clangd/Diagnostics.cpp | 3 +- clang-tools-extra/clangd/Protocol.cpp | 4 +-- clang-tools-extra/clangd/Protocol.h | 2 +- 4 files changed, 18 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index c25195cd338f..f70fd0018cfd 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -748,9 +748,8 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args, return Reply(std::move(Err)); WorkspaceEdit WE; - WE.changes.emplace(); for (const auto &It : R->ApplyEdits) { - (*WE.changes)[URI::createFile(It.first()).toString()] = + WE.changes[URI::createFile(It.first()).toString()] = It.second.asTextEdits(); } // ApplyEdit will take care of calling Reply(). @@ -813,22 +812,20 @@ void ClangdLSPServer::onRename(const RenameParams &Params, if (!Server->getDraft(File)) return Reply(llvm::make_error( "onRename called for non-added file", ErrorCode::InvalidParams)); - Server->rename( - File, Params.position, Params.newName, Opts.Rename, - [File, Params, Reply = std::move(Reply), - this](llvm::Expected R) mutable { - if (!R) - return Reply(R.takeError()); - if (auto Err = validateEdits(*Server, R->GlobalChanges)) - return Reply(std::move(Err)); - WorkspaceEdit Result; - Result.changes.emplace(); - for (const auto &Rep : R->GlobalChanges) { - (*Result.changes)[URI::createFile(Rep.first()).toString()] = - Rep.second.asTextEdits(); - } - Reply(Result); - }); + Server->rename(File, Params.position, Params.newName, Opts.Rename, + [File, Params, Reply = std::move(Reply), + this](llvm::Expected R) mutable { + if (!R) + return Reply(R.takeError()); + if (auto Err = validateEdits(*Server, R->GlobalChanges)) + return Reply(std::move(Err)); + WorkspaceEdit Result; + for (const auto &Rep : R->GlobalChanges) { + Result.changes[URI::createFile(Rep.first()).toString()] = + Rep.second.asTextEdits(); + } + Reply(Result); + }); } void ClangdLSPServer::onDocumentDidClose( diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index 453b5a644d09..88ec7aaa9b26 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -372,8 +372,7 @@ CodeAction toCodeAction(const Fix &F, const URIForFile &File) { Action.title = F.Message; Action.kind = std::string(CodeAction::QUICKFIX_KIND); Action.edit.emplace(); - Action.edit->changes.emplace(); - (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()}; + Action.edit->changes[File.uri()] = {F.Edits.begin(), F.Edits.end()}; return Action; } diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 06dbdaebb9a4..7c7f41795608 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -808,10 +808,8 @@ llvm::json::Value toJSON(const DocumentSymbol &S) { } llvm::json::Value toJSON(const WorkspaceEdit &WE) { - if (!WE.changes) - return llvm::json::Object{}; llvm::json::Object FileChanges; - for (auto &Change : *WE.changes) + for (auto &Change : WE.changes) FileChanges[Change.first] = llvm::json::Array(Change.second); return llvm::json::Object{{"changes", std::move(FileChanges)}}; } diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 3f6ce566ef2f..4831ea6de750 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -908,7 +908,7 @@ bool fromJSON(const llvm::json::Value &, CodeActionParams &, llvm::json::Path); struct WorkspaceEdit { /// Holds changes to existing resources. - llvm::Optional>> changes; + std::map> changes; /// Note: "documentChanges" is not currently used because currently there is /// no support for versioned edits.