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 @@ -186,7 +186,7 @@ Callback Reply); void bindMethods(LSPBinder &); - 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 @@ -226,7 +226,8 @@ 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; 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 @@ -28,6 +28,7 @@ #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator_range.h" @@ -997,7 +998,7 @@ 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)); + FixIts.emplace_back(std::move(F)); FixIts.back().diagnostics = {D}; } } @@ -1532,8 +1533,8 @@ Server->profile(MT.child("clangd_server")); } -std::vector ClangdLSPServer::getFixes(llvm::StringRef File, - const clangd::Diagnostic &D) { +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()) @@ -1578,9 +1579,14 @@ DiagnosticToReplacementMap LocalFixIts; // Temporary storage for (auto &Diag : Diagnostics) { toLSPDiags(Diag, Notification.uri, DiagOpts, - [&](clangd::Diagnostic Diag, llvm::ArrayRef Fixes) { + [&](clangd::Diagnostic Diag, llvm::ArrayRef Fixes, + llvm::ArrayRef Actions) { auto &FixItsForDiagnostic = LocalFixIts[Diag]; - llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic)); + llvm::copy(Actions, std::back_inserter(FixItsForDiagnostic)); + for (const auto &Fix : Fixes) { + FixItsForDiagnostic.emplace_back( + toCodeAction(Fix, Notification.uri)); + } 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 @@ -97,6 +97,7 @@ std::vector Notes; /// *Alternative* fixes for this diagnostic, one should be chosen. std::vector Fixes; + std::vector Actions; }; llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D); @@ -107,9 +108,11 @@ /// separate diagnostics with their corresponding fixits. Notes outside main /// file do not have a corresponding LSP diagnostic, but can still be included /// as part of their main diagnostic's message. -void toLSPDiags( - const Diag &D, const URIForFile &File, const ClangdDiagnosticOptions &Opts, - llvm::function_ref)> OutFn); +void toLSPDiags(const Diag &D, const URIForFile &File, + const ClangdDiagnosticOptions &Opts, + llvm::function_ref, + llvm::ArrayRef)> + OutFn); /// Convert from Fix to LSP CodeAction. CodeAction toCodeAction(const Fix &D, const URIForFile &File); 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 @@ -403,9 +403,11 @@ return Result; } -void toLSPDiags( - const Diag &D, const URIForFile &File, const ClangdDiagnosticOptions &Opts, - llvm::function_ref)> OutFn) { +void toLSPDiags(const Diag &D, const URIForFile &File, + const ClangdDiagnosticOptions &Opts, + llvm::function_ref, + llvm::ArrayRef)> + OutFn) { clangd::Diagnostic Main; Main.severity = getSeverity(D.Severity); @@ -437,7 +439,7 @@ break; } if (Opts.EmbedFixesInDiagnostics) { - Main.codeActions.emplace(); + Main.codeActions = D.Actions; for (const auto &Fix : D.Fixes) Main.codeActions->push_back(toCodeAction(Fix, File)); if (Main.codeActions->size() == 1) @@ -462,7 +464,7 @@ Main.relatedInformation->push_back(std::move(RelInfo)); } } - OutFn(std::move(Main), D.Fixes); + OutFn(std::move(Main), D.Fixes, D.Actions); // If we didn't emit the notes as relatedLocations, emit separate diagnostics // so the user can find the locations easily. @@ -474,7 +476,7 @@ Res.severity = getSeverity(Note.Severity); Res.range = Note.Range; Res.message = noteMessage(D, Note, Opts); - OutFn(std::move(Res), llvm::ArrayRef()); + OutFn(std::move(Res), {}, {}); } } @@ -542,7 +544,7 @@ // duplicated messages due to various reasons (e.g. the check doesn't handle // template instantiations well; clang-tidy alias checks). std::set> SeenDiags; - llvm::erase_if(Output, [&](const Diag& D) { + llvm::erase_if(Output, [&](const Diag &D) { return !SeenDiags.emplace(D.Range, D.Message).second; }); return std::move(Output); diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -693,7 +693,8 @@ // Transform diagnostics and check the results. std::vector>> LSPDiags; toLSPDiags(D, MainFile, Opts, - [&](clangd::Diagnostic LSPDiag, ArrayRef Fixes) { + [&](clangd::Diagnostic LSPDiag, ArrayRef Fixes, + ArrayRef) { LSPDiags.push_back( {std::move(LSPDiag), std::vector(Fixes.begin(), Fixes.end())}); @@ -712,7 +713,8 @@ LSPDiags.clear(); Opts.EmitRelatedLocations = true; toLSPDiags(D, MainFile, Opts, - [&](clangd::Diagnostic LSPDiag, ArrayRef Fixes) { + [&](clangd::Diagnostic LSPDiag, ArrayRef Fixes, + ArrayRef) { LSPDiags.push_back( {std::move(LSPDiag), std::vector(Fixes.begin(), Fixes.end())}); @@ -1334,16 +1336,14 @@ D.InsideMainFile = true; N.InsideMainFile = false; toLSPDiags(D, {}, Opts, - [&](clangd::Diagnostic LSPDiag, ArrayRef) { - EXPECT_EQ(LSPDiag.range, D.Range); - }); + [&](clangd::Diagnostic LSPDiag, ArrayRef, + ArrayRef) { EXPECT_EQ(LSPDiag.range, D.Range); }); D.InsideMainFile = false; N.InsideMainFile = true; toLSPDiags(D, {}, Opts, - [&](clangd::Diagnostic LSPDiag, ArrayRef) { - EXPECT_EQ(LSPDiag.range, N.Range); - }); + [&](clangd::Diagnostic LSPDiag, ArrayRef, + ArrayRef) { EXPECT_EQ(LSPDiag.range, N.Range); }); } } // namespace