hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This is an initial patch to add TextDocumentEdit (versioned edits) support in
clangd, the scope is minimal:
- add and extend the corresponding protocol structures
- propagate the documentChanges for diagnostic codeactions (our motivated case)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148783
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -254,6 +254,17 @@
llvm::json::Value toJSON(const TextEdit &);
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const TextEdit &);
+struct TextDocumentEdit {
+ /// The text document to change.
+ VersionedTextDocumentIdentifier textDocument;
+
+ /// The edits to be applied.
+ /// FIXME: support the AnnotatedTextEdit variant.
+ std::vector<TextEdit> edits;
+};
+bool fromJSON(const llvm::json::Value &, TextDocumentEdit &, llvm::json::Path);
+llvm::json::Value toJSON(const TextDocumentEdit &);
+
struct TextDocumentItem {
/// The text document's URI.
URIForFile uri;
@@ -517,6 +528,9 @@
/// server to the client.
bool SemanticTokenRefreshSupport = false;
+ /// The client supports versioned document changes for WorkspaceEdit.
+ bool DocumentChanges = false;
+
/// Whether the client supports the textDocument/inactiveRegions
/// notification. This is a clangd extension.
bool InactiveRegions = false;
@@ -970,12 +984,18 @@
};
bool fromJSON(const llvm::json::Value &, CodeActionParams &, llvm::json::Path);
+/// The edit should either provide changes or documentChanges. If the client
+/// can handle versioned document edits and if documentChanges are present,
+/// the latter are preferred over changes.
struct WorkspaceEdit {
/// Holds changes to existing resources.
- std::map<std::string, std::vector<TextEdit>> changes;
-
- /// Note: "documentChanges" is not currently used because currently there is
- /// no support for versioned edits.
+ std::optional<std::map<std::string, std::vector<TextEdit>>> changes;
+ /// Versioned document edits.
+ ///
+ /// If a client neither supports `documentChanges` nor
+ /// `workspace.workspaceEdit.resourceOperations` then only plain `TextEdit`s
+ /// using the `changes` property are supported.
+ std::optional<std::vector<TextDocumentEdit>> documentChanges;
};
bool fromJSON(const llvm::json::Value &, WorkspaceEdit &, llvm::json::Path);
llvm::json::Value toJSON(const WorkspaceEdit &WE);
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -197,6 +197,17 @@
};
}
+bool fromJSON(const llvm::json::Value &Params, TextDocumentEdit &R,
+ llvm::json::Path P) {
+ llvm::json::ObjectMapper O(Params, P);
+ return O && O.map("textDocument", R.textDocument) && O.map("edits", R.edits);
+}
+llvm::json::Value toJSON(const TextDocumentEdit &P) {
+ llvm::json::Object Result{{"textDocument", P.textDocument},
+ {"edits", P.edits}};
+ return Result;
+}
+
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const TextEdit &TE) {
OS << TE.range << " => \"";
llvm::printEscapedString(TE.newText, OS);
@@ -444,6 +455,10 @@
if (auto RefreshSupport = SemanticTokens->getBoolean("refreshSupport"))
R.SemanticTokenRefreshSupport = *RefreshSupport;
}
+ if (auto *WorkspaceEdit = Workspace->getObject("workspaceEdit")) {
+ if (auto DocumentChanges = WorkspaceEdit->getBoolean("documentChanges"))
+ R.DocumentChanges = *DocumentChanges;
+ }
}
if (auto *Window = O->getObject("window")) {
if (auto WorkDoneProgress = Window->getBoolean("workDoneProgress"))
@@ -717,7 +732,7 @@
bool fromJSON(const llvm::json::Value &Params, WorkspaceEdit &R,
llvm::json::Path P) {
llvm::json::ObjectMapper O(Params, P);
- return O && O.map("changes", R.changes);
+ return O && O.map("changes", R.changes) && O.map("documentChanges", R.documentChanges);
}
bool fromJSON(const llvm::json::Value &Params, ExecuteCommandParams &R,
@@ -863,10 +878,16 @@
}
llvm::json::Value toJSON(const WorkspaceEdit &WE) {
- llvm::json::Object FileChanges;
- for (auto &Change : WE.changes)
- FileChanges[Change.first] = llvm::json::Array(Change.second);
- return llvm::json::Object{{"changes", std::move(FileChanges)}};
+ llvm::json::Object Result;
+ if (WE.changes) {
+ llvm::json::Object FileChanges;
+ for (auto &Change : *WE.changes)
+ FileChanges[Change.first] = llvm::json::Array(Change.second);
+ Result["changes"] = std::move(FileChanges);
+ }
+ if (WE.documentChanges)
+ Result["documentChanges"] = *WE.documentChanges;
+ return Result;
}
bool fromJSON(const llvm::json::Value &Params, TweakArgs &A,
Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -121,7 +121,9 @@
llvm::function_ref<void(clangd::Diagnostic, llvm::ArrayRef<Fix>)> OutFn);
/// Convert from Fix to LSP CodeAction.
-CodeAction toCodeAction(const Fix &D, const URIForFile &File);
+CodeAction toCodeAction(const Fix &D, const URIForFile &File,
+ const std::optional<int64_t> &Version,
+ bool SupportsDocumentChanges);
/// Convert from clang diagnostic level to LSP severity.
int getSeverity(DiagnosticsEngine::Level L);
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -423,12 +423,22 @@
return OS;
}
-CodeAction toCodeAction(const Fix &F, const URIForFile &File) {
+CodeAction toCodeAction(const Fix &F, const URIForFile &File,
+ const std::optional<int64_t> &Version,
+ bool SupportsDocumentChanges) {
CodeAction Action;
Action.title = F.Message;
Action.kind = std::string(CodeAction::QUICKFIX_KIND);
Action.edit.emplace();
- Action.edit->changes[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+ if (!SupportsDocumentChanges) {
+ Action.edit->changes.emplace();
+ (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+ } else {
+ Action.edit->documentChanges.emplace();
+ TextDocumentEdit& Edit = Action.edit->documentChanges->emplace_back();
+ Edit.textDocument = VersionedTextDocumentIdentifier{{File}, Version};
+ Edit.edits = {F.Edits.begin(), F.Edits.end()};
+ }
return Action;
}
@@ -499,13 +509,6 @@
case Diag::Unknown:
break;
}
- if (Opts.EmbedFixesInDiagnostics) {
- Main.codeActions.emplace();
- for (const auto &Fix : D.Fixes)
- Main.codeActions->push_back(toCodeAction(Fix, File));
- if (Main.codeActions->size() == 1)
- Main.codeActions->front().isPreferred = true;
- }
if (Opts.SendDiagnosticCategory && !D.Category.empty())
Main.category = D.Category;
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -21,6 +21,7 @@
#include "llvm/Support/JSON.h"
#include <chrono>
#include <cstddef>
+#include <cstdint>
#include <memory>
#include <optional>
#include <vector>
@@ -197,7 +198,8 @@
Callback<llvm::json::Value> Reply);
void bindMethods(LSPBinder &, const ClientCapabilities &Caps);
- std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
+ std::pair<std::optional<int64_t>, std::vector<Fix>>
+ getFixes(StringRef File, const clangd::Diagnostic &D);
/// Checks if completion request should be ignored. We need this due to the
/// limitation of the LSP. Per LSP, a client sends requests for all "trigger
@@ -234,7 +236,9 @@
typedef std::map<clangd::Diagnostic, std::vector<Fix>, LSPDiagnosticCompare>
DiagnosticToReplacementMap;
/// Caches FixIts per file and diagnostics
- llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
+ llvm::StringMap<
+ std::pair</*version*/ std::optional<int64_t>, DiagnosticToReplacementMap>>
+ FixItsMap;
// Last semantic-tokens response, for incremental requests.
std::mutex SemanticTokensMutex;
llvm::StringMap<SemanticTokens> LastSemanticTokens;
@@ -271,6 +275,8 @@
MarkupKind HoverContentFormat = MarkupKind::PlainText;
/// Whether the client supports offsets for parameter info labels.
bool SupportsOffsetsInSignatureHelp = false;
+ /// Whether the client supports the versioned document changes.
+ bool SupportsDocumentChanges = false;
std::mutex BackgroundIndexProgressMutex;
enum class BackgroundIndexProgress {
// Client doesn't support reporting progress. No transitions possible.
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -487,6 +487,7 @@
Params.capabilities.HierarchicalDocumentSymbol;
SupportsReferenceContainer = Params.capabilities.ReferenceContainer;
SupportFileStatus = Params.initializationOptions.FileStatus;
+ SupportsDocumentChanges = Params.capabilities.DocumentChanges;
HoverContentFormat = Params.capabilities.HoverContentFormat;
Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp;
@@ -761,8 +762,10 @@
return Reply(std::move(Err));
WorkspaceEdit WE;
+ // FIXME: use documentChanges when SupportDocumentChanges is true.
+ 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().
@@ -833,8 +836,12 @@
if (auto Err = validateEdits(*Server, R->GlobalChanges))
return Reply(std::move(Err));
WorkspaceEdit Result;
+ // FIXME: use documentChanges if SupportDocumentChanges is
+ // true.
+ Result.changes.emplace();
for (const auto &Rep : R->GlobalChanges) {
- Result.changes[URI::createFile(Rep.first()).toString()] =
+ (*Result
+ .changes)[URI::createFile(Rep.first()).toString()] =
Rep.second.asTextEdits();
}
Reply(Result);
@@ -984,8 +991,10 @@
std::vector<CodeAction> FixIts;
if (KindAllowed(CodeAction::QUICKFIX_KIND)) {
for (const Diagnostic &D : Params.context.diagnostics) {
- for (auto &F : getFixes(File.file(), D)) {
- FixIts.push_back(toCodeAction(F, Params.textDocument.uri));
+ const auto& [Version, Fixes] = getFixes(File.file(), D);
+ for (auto &F : Fixes) {
+ FixIts.push_back(toCodeAction(F, Params.textDocument.uri, Version,
+ SupportsDocumentChanges));
FixIts.back().diagnostics = {D};
}
}
@@ -1663,19 +1672,19 @@
Server->profile(MT.child("clangd_server"));
}
-std::vector<Fix> ClangdLSPServer::getFixes(llvm::StringRef File,
- const clangd::Diagnostic &D) {
+std::pair<std::optional<int64_t>, std::vector<Fix>>
+ClangdLSPServer::getFixes(llvm::StringRef File, const clangd::Diagnostic &D) {
std::lock_guard<std::mutex> Lock(FixItsMutex);
auto DiagToFixItsIter = FixItsMap.find(File);
if (DiagToFixItsIter == FixItsMap.end())
return {};
const auto &DiagToFixItsMap = DiagToFixItsIter->second;
- auto FixItsIter = DiagToFixItsMap.find(D);
- if (FixItsIter == DiagToFixItsMap.end())
+ auto FixItsIter = DiagToFixItsMap.second.find(D);
+ if (FixItsIter == DiagToFixItsMap.second.end())
return {};
- return FixItsIter->second;
+ return {DiagToFixItsMap.first, FixItsIter->second};
}
// A completion request is sent when the user types '>' or ':', but we only
@@ -1710,8 +1719,18 @@
for (auto &Diag : Diagnostics) {
toLSPDiags(Diag, Notification.uri, DiagOpts,
[&](clangd::Diagnostic Diag, llvm::ArrayRef<Fix> Fixes) {
+ if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
+ Diag.codeActions.emplace();
+ for (const auto &Fix : Fixes)
+ Diag.codeActions->push_back(toCodeAction(
+ Fix, Notification.uri, Notification.version,
+ SupportsDocumentChanges));
+ if (Diag.codeActions->size() == 1)
+ Diag.codeActions->front().isPreferred = true;
+ }
auto &FixItsForDiagnostic = LocalFixIts[Diag];
llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
+
Notification.diagnostics.push_back(std::move(Diag));
});
}
@@ -1719,7 +1738,7 @@
// Cache FixIts
{
std::lock_guard<std::mutex> Lock(FixItsMutex);
- FixItsMap[File] = LocalFixIts;
+ FixItsMap[File] = {Notification.version, LocalFixIts};
}
// Send a notification to the LSP client.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
