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 @@ -21,6 +21,7 @@ #include "llvm/Support/JSON.h" #include #include +#include #include #include #include @@ -197,7 +198,8 @@ Callback Reply); void bindMethods(LSPBinder &, const ClientCapabilities &Caps); - std::vector getFixes(StringRef File, const clangd::Diagnostic &D); + std::vector 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 @@ -231,10 +233,12 @@ std::atomic IsBeingDestroyed = {false}; std::mutex FixItsMutex; - typedef std::map, LSPDiagnosticCompare> + typedef std::map, + LSPDiagnosticCompare> DiagnosticToReplacementMap; /// Caches FixIts per file and diagnostics - llvm::StringMap FixItsMap; + llvm::StringMap + FixItsMap; // Last semantic-tokens response, for incremental requests. std::mutex SemanticTokensMutex; llvm::StringMap 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. 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 @@ -95,6 +95,26 @@ return CA; } +/// Convert from Fix to LSP CodeAction. +CodeAction toCodeAction(const Fix &F, const URIForFile &File, + const std::optional &Version, + bool SupportsDocumentChanges) { + CodeAction Action; + Action.title = F.Message; + Action.kind = std::string(CodeAction::QUICKFIX_KIND); + Action.edit.emplace(); + 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; +} + void adjustSymbolKinds(llvm::MutableArrayRef Syms, SymbolKindBitset Kinds) { for (auto &S : Syms) { @@ -487,6 +507,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 +782,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 +856,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 +1011,8 @@ std::vector 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)); + for (auto &CA : getFixes(File.file(), D)) { + FixIts.push_back(CA); FixIts.back().diagnostics = {D}; } } @@ -1663,19 +1690,19 @@ Server->profile(MT.child("clangd_server")); } -std::vector ClangdLSPServer::getFixes(llvm::StringRef File, - const clangd::Diagnostic &D) { - std::lock_guard Lock(FixItsMutex); - auto DiagToFixItsIter = FixItsMap.find(File); - if (DiagToFixItsIter == FixItsMap.end()) - return {}; +std::vector ClangdLSPServer::getFixes(StringRef File, + const clangd::Diagnostic &D) { +std::lock_guard 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()) - return {}; +const auto &DiagToFixItsMap = DiagToFixItsIter->second; +auto FixItsIter = DiagToFixItsMap.find(D); +if (FixItsIter == DiagToFixItsMap.end()) + return {}; - return FixItsIter->second; +return FixItsIter->second; } // A completion request is sent when the user types '>' or ':', but we only @@ -1710,8 +1737,21 @@ for (auto &Diag : Diagnostics) { toLSPDiags(Diag, Notification.uri, DiagOpts, [&](clangd::Diagnostic Diag, llvm::ArrayRef Fixes) { + std::vector CodeActions; + for (const auto &Fix : Fixes) + CodeActions.push_back(toCodeAction(Fix, Notification.uri, + Notification.version, + SupportsDocumentChanges)); + + if (DiagOpts.EmbedFixesInDiagnostics && !CodeActions.empty()) { + Diag.codeActions.emplace(CodeActions); + if (Diag.codeActions->size() == 1) + Diag.codeActions->front().isPreferred = true; + } auto &FixItsForDiagnostic = LocalFixIts[Diag]; - llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic)); + llvm::move(std::move(CodeActions), + std::back_inserter(FixItsForDiagnostic)); + Notification.diagnostics.push_back(std::move(Diag)); }); } diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -120,9 +120,6 @@ const Diag &D, const URIForFile &File, const ClangdDiagnosticOptions &Opts, llvm::function_ref)> OutFn); -/// Convert from Fix to LSP CodeAction. -CodeAction toCodeAction(const Fix &D, const URIForFile &File); - /// Convert from clang diagnostic level to LSP severity. int getSeverity(DiagnosticsEngine::Level L); diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -423,15 +423,6 @@ return OS; } -CodeAction toCodeAction(const Fix &F, const URIForFile &File) { - 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()}; - return Action; -} - Diag toDiag(const llvm::SMDiagnostic &D, Diag::DiagSource Source) { Diag Result; Result.Message = D.getMessage().str(); @@ -499,13 +490,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; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/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 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> changes; - - /// Note: "documentChanges" is not currently used because currently there is - /// no support for versioned edits. + std::optional>> 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> documentChanges; }; bool fromJSON(const llvm::json::Value &, WorkspaceEdit &, llvm::json::Path); llvm::json::Value toJSON(const WorkspaceEdit &WE); diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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,8 @@ 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 +879,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, diff --git a/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test b/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test @@ -0,0 +1,145 @@ +# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{}}},"workspace":{"workspaceEdit":{"documentChanges":true}}},"trace":"off"}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}} +# CHECK: "method": "textDocument/publishDiagnostics", +# CHECK-NEXT: "params": { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "-Wparentheses", +# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 37, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 32, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "uri": "file://{{.*}}/foo.c", +# CHECK-NEXT: "version": 1 +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"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 (fixes available)", "code": "-Wparentheses", "source": "clang"}]}}} +# CHECK: "id": 2, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "-Wparentheses", +# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 37, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 32, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "documentChanges": [ +# CHECK-NEXT: { +# CHECK-NEXT: "edits": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "(", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 32, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 32, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "newText": ")", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 37, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 37, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "textDocument": { +# CHECK-NEXT: "uri": "file:///clangd-test/foo.c", +# CHECK-NEXT: "version": 1 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: "kind": "quickfix", +# CHECK-NEXT: "title": "place parentheses around the assignment to silence this warning" +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "-Wparentheses", +# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 37, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 32, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "documentChanges": [ +# CHECK-NEXT: { +# CHECK-NEXT: "edits": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "==", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 35, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 34, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "textDocument": { +# CHECK-NEXT: "uri": "file:///clangd-test/foo.c", +# CHECK-NEXT: "version": 1 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: }, +# CHECK-NEXT: "kind": "quickfix", +# CHECK-NEXT: "title": "use '==' to turn this assignment into an equality comparison" +# CHECK-NEXT: } +# CHECK-NEXT: ] +--- +{"jsonrpc":"2.0","id":4,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} +