mirror of
https://github.com/capstone-engine/llvm-capstone.git
synced 2025-02-17 00:10:28 +00:00
[clangd] Refine the workflow for diagnostic Fixits.
- No longer store the diagnostic fixits in the clangdLSPServer - When propagating the fixit via the code action, we use the Diag information stored in the ParsedAST (in clangdServer.cpp) Differential Revision: https://reviews.llvm.org/D155173
This commit is contained in:
Haojian Wu
parent
793a349e6f
commit
f4f6c229bd
@ -41,6 +41,7 @@
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <functional>
|
||||
#include <map>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
#include <optional>
|
||||
@ -886,8 +887,8 @@ void ClangdLSPServer::onDocumentDidClose(
|
||||
Server->removeDocument(File);
|
||||
|
||||
{
|
||||
std::lock_guard<std::mutex> Lock(FixItsMutex);
|
||||
FixItsMap.erase(File);
|
||||
std::lock_guard<std::mutex> Lock(DiagRefMutex);
|
||||
DiagRefMap.erase(File);
|
||||
}
|
||||
{
|
||||
std::lock_guard<std::mutex> HLock(SemanticTokensMutex);
|
||||
@ -1010,72 +1011,70 @@ static std::optional<Command> asCommand(const CodeAction &Action) {
|
||||
void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
|
||||
Callback<llvm::json::Value> Reply) {
|
||||
URIForFile File = Params.textDocument.uri;
|
||||
// Checks whether a particular CodeActionKind is included in the response.
|
||||
auto KindAllowed = [Only(Params.context.only)](llvm::StringRef Kind) {
|
||||
if (Only.empty())
|
||||
return true;
|
||||
return llvm::any_of(Only, [&](llvm::StringRef Base) {
|
||||
return Kind.consume_front(Base) && (Kind.empty() || Kind.startswith("."));
|
||||
});
|
||||
};
|
||||
std::map<ClangdServer::DiagRef, clangd::Diagnostic> ToLSPDiags;
|
||||
ClangdServer::CodeActionInputs Inputs;
|
||||
|
||||
// We provide a code action for Fixes on the specified diagnostics.
|
||||
std::vector<CodeAction> FixIts;
|
||||
if (KindAllowed(CodeAction::QUICKFIX_KIND)) {
|
||||
for (const Diagnostic &D : Params.context.diagnostics) {
|
||||
for (auto &CA : getFixes(File.file(), D)) {
|
||||
FixIts.push_back(CA);
|
||||
FixIts.back().diagnostics = {D};
|
||||
}
|
||||
for (const auto& LSPDiag : Params.context.diagnostics) {
|
||||
if (auto DiagRef = getDiagRef(File.file(), LSPDiag)) {
|
||||
ToLSPDiags[*DiagRef] = LSPDiag;
|
||||
Inputs.Diagnostics.push_back(*DiagRef);
|
||||
}
|
||||
}
|
||||
Inputs.File = File.file();
|
||||
Inputs.Selection = Params.range;
|
||||
Inputs.RequestedActionKinds = Params.context.only;
|
||||
Inputs.TweakFilter = [this](const Tweak &T) {
|
||||
return Opts.TweakFilter(T);
|
||||
};
|
||||
auto CB = [this,
|
||||
Reply = std::move(Reply),
|
||||
ToLSPDiags = std::move(ToLSPDiags), File,
|
||||
Selection = Params.range](
|
||||
llvm::Expected<ClangdServer::CodeActionResult> Fixits) mutable {
|
||||
if (!Fixits)
|
||||
return Reply(Fixits.takeError());
|
||||
std::vector<CodeAction> CAs;
|
||||
auto Version = decodeVersion(Fixits->Version);
|
||||
for (const auto &QF : Fixits->QuickFixes) {
|
||||
CAs.push_back(toCodeAction(QF.F, File, Version, SupportsDocumentChanges,
|
||||
SupportsChangeAnnotation));
|
||||
if (auto It = ToLSPDiags.find(QF.Diag);
|
||||
It != ToLSPDiags.end()) {
|
||||
CAs.back().diagnostics = {It->second};
|
||||
}
|
||||
}
|
||||
for (const auto &TR : Fixits->TweakRefs)
|
||||
CAs.push_back(toCodeAction(TR, File, Selection));
|
||||
|
||||
// Now enumerate the semantic code actions.
|
||||
auto ConsumeActions =
|
||||
[Diags = Params.context.diagnostics, Reply = std::move(Reply), File,
|
||||
Selection = Params.range, FixIts = std::move(FixIts), this](
|
||||
llvm::Expected<std::vector<ClangdServer::TweakRef>> Tweaks) mutable {
|
||||
if (!Tweaks)
|
||||
return Reply(Tweaks.takeError());
|
||||
|
||||
std::vector<CodeAction> Actions = std::move(FixIts);
|
||||
Actions.reserve(Actions.size() + Tweaks->size());
|
||||
for (const auto &T : *Tweaks)
|
||||
Actions.push_back(toCodeAction(T, File, Selection));
|
||||
|
||||
// If there's exactly one quick-fix, call it "preferred".
|
||||
// We never consider refactorings etc as preferred.
|
||||
CodeAction *OnlyFix = nullptr;
|
||||
for (auto &Action : Actions) {
|
||||
if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
|
||||
if (OnlyFix) {
|
||||
OnlyFix = nullptr;
|
||||
break;
|
||||
}
|
||||
OnlyFix = &Action;
|
||||
}
|
||||
}
|
||||
// If there's exactly one quick-fix, call it "preferred".
|
||||
// We never consider refactorings etc as preferred.
|
||||
CodeAction *OnlyFix = nullptr;
|
||||
for (auto &Action : CAs) {
|
||||
if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
|
||||
if (OnlyFix) {
|
||||
OnlyFix->isPreferred = true;
|
||||
if (Diags.size() == 1 && Diags.front().range == Selection)
|
||||
OnlyFix->diagnostics = {Diags.front()};
|
||||
OnlyFix = nullptr;
|
||||
break;
|
||||
}
|
||||
OnlyFix = &Action;
|
||||
}
|
||||
}
|
||||
if (OnlyFix) {
|
||||
OnlyFix->isPreferred = true;
|
||||
if (ToLSPDiags.size() == 1 &&
|
||||
ToLSPDiags.begin()->second.range == Selection)
|
||||
OnlyFix->diagnostics = {ToLSPDiags.begin()->second};
|
||||
}
|
||||
|
||||
if (SupportsCodeAction)
|
||||
return Reply(llvm::json::Array(Actions));
|
||||
std::vector<Command> Commands;
|
||||
for (const auto &Action : Actions) {
|
||||
if (auto Command = asCommand(Action))
|
||||
Commands.push_back(std::move(*Command));
|
||||
}
|
||||
return Reply(llvm::json::Array(Commands));
|
||||
};
|
||||
Server->enumerateTweaks(
|
||||
File.file(), Params.range,
|
||||
[this, KindAllowed(std::move(KindAllowed))](const Tweak &T) {
|
||||
return Opts.TweakFilter(T) && KindAllowed(T.kind());
|
||||
},
|
||||
std::move(ConsumeActions));
|
||||
if (SupportsCodeAction)
|
||||
return Reply(llvm::json::Array(CAs));
|
||||
std::vector<Command> Commands;
|
||||
for (const auto &Action : CAs) {
|
||||
if (auto Command = asCommand(Action))
|
||||
Commands.push_back(std::move(*Command));
|
||||
}
|
||||
return Reply(llvm::json::Array(Commands));
|
||||
};
|
||||
Server->codeAction(Inputs, std::move(CB));
|
||||
}
|
||||
|
||||
void ClangdLSPServer::onCompletion(const CompletionParams &Params,
|
||||
@ -1704,17 +1703,17 @@ void ClangdLSPServer::profile(MemoryTree &MT) const {
|
||||
Server->profile(MT.child("clangd_server"));
|
||||
}
|
||||
|
||||
std::vector<CodeAction> 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 {};
|
||||
std::optional<ClangdServer::DiagRef>
|
||||
ClangdLSPServer::getDiagRef(StringRef File, const clangd::Diagnostic &D) {
|
||||
std::lock_guard<std::mutex> Lock(DiagRefMutex);
|
||||
auto DiagToDiagRefIter = DiagRefMap.find(File);
|
||||
if (DiagToDiagRefIter == DiagRefMap.end())
|
||||
return std::nullopt;
|
||||
|
||||
const auto &DiagToFixItsMap = DiagToFixItsIter->second;
|
||||
auto FixItsIter = DiagToFixItsMap.find(D);
|
||||
if (FixItsIter == DiagToFixItsMap.end())
|
||||
return {};
|
||||
const auto &DiagToDiagRefMap = DiagToDiagRefIter->second;
|
||||
auto FixItsIter = DiagToDiagRefMap.find(toDiagKey(D));
|
||||
if (FixItsIter == DiagToDiagRefMap.end())
|
||||
return std::nullopt;
|
||||
|
||||
return FixItsIter->second;
|
||||
}
|
||||
@ -1747,31 +1746,29 @@ void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
|
||||
PublishDiagnosticsParams Notification;
|
||||
Notification.version = decodeVersion(Version);
|
||||
Notification.uri = URIForFile::canonicalize(File, /*TUPath=*/File);
|
||||
DiagnosticToReplacementMap LocalFixIts; // Temporary storage
|
||||
DiagnosticToDiagRefMap LocalDiagMap; // Temporary storage
|
||||
for (auto &Diag : Diagnostics) {
|
||||
toLSPDiags(Diag, Notification.uri, DiagOpts,
|
||||
[&](clangd::Diagnostic Diag, llvm::ArrayRef<Fix> Fixes) {
|
||||
std::vector<CodeAction> CodeActions;
|
||||
for (const auto &Fix : Fixes)
|
||||
CodeActions.push_back(toCodeAction(Fix, Notification.uri,
|
||||
Notification.version,
|
||||
SupportsDocumentChanges,
|
||||
SupportsChangeAnnotation));
|
||||
|
||||
[&](clangd::Diagnostic LSPDiag, llvm::ArrayRef<Fix> Fixes) {
|
||||
if (DiagOpts.EmbedFixesInDiagnostics) {
|
||||
Diag.codeActions.emplace(CodeActions);
|
||||
if (Diag.codeActions->size() == 1)
|
||||
Diag.codeActions->front().isPreferred = true;
|
||||
std::vector<CodeAction> CodeActions;
|
||||
for (const auto &Fix : Fixes)
|
||||
CodeActions.push_back(toCodeAction(
|
||||
Fix, Notification.uri, Notification.version,
|
||||
SupportsDocumentChanges, SupportsChangeAnnotation));
|
||||
LSPDiag.codeActions.emplace(std::move(CodeActions));
|
||||
if (LSPDiag.codeActions->size() == 1)
|
||||
LSPDiag.codeActions->front().isPreferred = true;
|
||||
}
|
||||
LocalFixIts[Diag] = std::move(CodeActions);
|
||||
Notification.diagnostics.push_back(std::move(Diag));
|
||||
LocalDiagMap[toDiagKey(LSPDiag)] = {Diag.Range, Diag.Message};
|
||||
Notification.diagnostics.push_back(std::move(LSPDiag));
|
||||
});
|
||||
}
|
||||
|
||||
// Cache FixIts
|
||||
// Cache DiagRefMap
|
||||
{
|
||||
std::lock_guard<std::mutex> Lock(FixItsMutex);
|
||||
FixItsMap[File] = LocalFixIts;
|
||||
std::lock_guard<std::mutex> Lock(DiagRefMutex);
|
||||
DiagRefMap[File] = LocalDiagMap;
|
||||
}
|
||||
|
||||
// Send a notification to the LSP client.
|
||||
|
||||
@ -197,7 +197,8 @@ private:
|
||||
Callback<llvm::json::Value> Reply);
|
||||
|
||||
void bindMethods(LSPBinder &, const ClientCapabilities &Caps);
|
||||
std::vector<CodeAction> getFixes(StringRef File, const clangd::Diagnostic &D);
|
||||
std::optional<ClangdServer::DiagRef> getDiagRef(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
|
||||
@ -230,13 +231,28 @@ private:
|
||||
/// Used to indicate the ClangdLSPServer is being destroyed.
|
||||
std::atomic<bool> IsBeingDestroyed = {false};
|
||||
|
||||
std::mutex FixItsMutex;
|
||||
typedef std::map<clangd::Diagnostic, std::vector<CodeAction>,
|
||||
LSPDiagnosticCompare>
|
||||
DiagnosticToReplacementMap;
|
||||
/// Caches FixIts per file and diagnostics
|
||||
llvm::StringMap<DiagnosticToReplacementMap>
|
||||
FixItsMap;
|
||||
// FIXME: The caching is a temporary solution to get corresponding clangd
|
||||
// diagnostic from a LSP diagnostic.
|
||||
// Ideally, ClangdServer can generate an identifier for each diagnostic,
|
||||
// emit them via the LSP's data field (which was newly added in LSP 3.16).
|
||||
std::mutex DiagRefMutex;
|
||||
struct DiagKey {
|
||||
clangd::Range Range;
|
||||
std::string Message;
|
||||
bool operator<(const DiagKey &Other) const {
|
||||
return std::tie(Range, Message) < std::tie(Other.Range, Other.Message);
|
||||
}
|
||||
};
|
||||
DiagKey toDiagKey(const clangd::Diagnostic &LSPDiag) {
|
||||
return {LSPDiag.range, LSPDiag.message};
|
||||
}
|
||||
/// A map from LSP diagnostic to clangd-naive diagnostic.
|
||||
typedef std::map<DiagKey, ClangdServer::DiagRef>
|
||||
DiagnosticToDiagRefMap;
|
||||
/// Caches the mapping LSP and clangd-naive diagnostics per file.
|
||||
llvm::StringMap<DiagnosticToDiagRefMap>
|
||||
DiagRefMap;
|
||||
|
||||
// Last semantic-tokens response, for incremental requests.
|
||||
std::mutex SemanticTokensMutex;
|
||||
llvm::StringMap<SemanticTokens> LastSemanticTokens;
|
||||
|
||||
@ -52,11 +52,16 @@
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <type_traits>
|
||||
#include <vector>
|
||||
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
namespace {
|
||||
|
||||
// Tracks number of times a tweak has been offered.
|
||||
static constexpr trace::Metric TweakAvailable(
|
||||
"tweak_available", trace::Metric::Counter, "tweak_id");
|
||||
|
||||
// Update the FileIndex with new ASTs and plumb the diagnostics responses.
|
||||
struct UpdateIndexCallbacks : public ParsingCallbacks {
|
||||
UpdateIndexCallbacks(FileIndex *FIndex,
|
||||
@ -643,12 +648,65 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST,
|
||||
return std::move(Result);
|
||||
}
|
||||
|
||||
void ClangdServer::codeAction(const CodeActionInputs &Params,
|
||||
Callback<CodeActionResult> CB) {
|
||||
auto Action = [Params, CB = std::move(CB),
|
||||
FeatureModules(this->FeatureModules)](
|
||||
Expected<InputsAndAST> InpAST) mutable {
|
||||
if (!InpAST)
|
||||
return CB(InpAST.takeError());
|
||||
auto KindAllowed =
|
||||
[Only(Params.RequestedActionKinds)](llvm::StringRef Kind) {
|
||||
if (Only.empty())
|
||||
return true;
|
||||
return llvm::any_of(Only, [&](llvm::StringRef Base) {
|
||||
return Kind.consume_front(Base) &&
|
||||
(Kind.empty() || Kind.startswith("."));
|
||||
});
|
||||
};
|
||||
|
||||
CodeActionResult Result;
|
||||
Result.Version = InpAST->AST.version().str();
|
||||
if (KindAllowed(CodeAction::QUICKFIX_KIND)) {
|
||||
auto FindMatchedFixes =
|
||||
[&InpAST](const DiagRef &DR) -> llvm::ArrayRef<Fix> {
|
||||
for (const auto &Diag : InpAST->AST.getDiagnostics())
|
||||
if (Diag.Range == DR.Range && Diag.Message == DR.Message)
|
||||
return Diag.Fixes;
|
||||
return {};
|
||||
};
|
||||
for (const auto &Diag : Params.Diagnostics)
|
||||
for (const auto &Fix : FindMatchedFixes(Diag))
|
||||
Result.QuickFixes.push_back({Diag, Fix});
|
||||
}
|
||||
|
||||
// Collect Tweaks
|
||||
auto Selections = tweakSelection(Params.Selection, *InpAST, /*FS=*/nullptr);
|
||||
if (!Selections)
|
||||
return CB(Selections.takeError());
|
||||
// Don't allow a tweak to fire more than once across ambiguous selections.
|
||||
llvm::DenseSet<llvm::StringRef> PreparedTweaks;
|
||||
auto DeduplicatingFilter = [&](const Tweak &T) {
|
||||
return KindAllowed(T.kind()) && Params.TweakFilter(T) &&
|
||||
!PreparedTweaks.count(T.id());
|
||||
};
|
||||
for (const auto &Sel : *Selections) {
|
||||
for (auto &T : prepareTweaks(*Sel, DeduplicatingFilter, FeatureModules)) {
|
||||
Result.TweakRefs.push_back(TweakRef{T->id(), T->title(), T->kind()});
|
||||
PreparedTweaks.insert(T->id());
|
||||
TweakAvailable.record(1, T->id());
|
||||
}
|
||||
}
|
||||
CB(std::move(Result));
|
||||
};
|
||||
|
||||
WorkScheduler->runWithAST("codeAction", Params.File, std::move(Action),
|
||||
Transient);
|
||||
}
|
||||
|
||||
void ClangdServer::enumerateTweaks(
|
||||
PathRef File, Range Sel, llvm::unique_function<bool(const Tweak &)> Filter,
|
||||
Callback<std::vector<TweakRef>> CB) {
|
||||
// Tracks number of times a tweak has been offered.
|
||||
static constexpr trace::Metric TweakAvailable(
|
||||
"tweak_available", trace::Metric::Counter, "tweak_id");
|
||||
auto Action = [Sel, CB = std::move(CB), Filter = std::move(Filter),
|
||||
FeatureModules(this->FeatureModules)](
|
||||
Expected<InputsAndAST> InpAST) mutable {
|
||||
|
||||
@ -37,8 +37,7 @@
|
||||
#include <memory>
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <type_traits>
|
||||
#include <utility>
|
||||
#include <tuple>
|
||||
#include <vector>
|
||||
|
||||
namespace clang {
|
||||
@ -351,8 +350,49 @@ public:
|
||||
std::string Title; /// A single-line message to show in the UI.
|
||||
llvm::StringLiteral Kind;
|
||||
};
|
||||
|
||||
// Ref to the clangd::Diag.
|
||||
struct DiagRef {
|
||||
Range Range;
|
||||
std::string Message;
|
||||
bool operator==(const DiagRef &Other) const {
|
||||
return std::tie(Range, Message) == std::tie(Other.Range, Other.Message);
|
||||
}
|
||||
bool operator<(const DiagRef &Other) const {
|
||||
return std::tie(Range, Message) < std::tie(Other.Range, Other.Message);
|
||||
}
|
||||
};
|
||||
|
||||
struct CodeActionInputs {
|
||||
std::string File;
|
||||
Range Selection;
|
||||
|
||||
/// Requested kind of actions to return.
|
||||
std::vector<std::string> RequestedActionKinds;
|
||||
|
||||
/// Diagnostics attached to the code action request.
|
||||
std::vector<DiagRef> Diagnostics;
|
||||
|
||||
/// Tweaks where Filter returns false will not be checked or included.
|
||||
std::function<bool(const Tweak &)> TweakFilter;
|
||||
};
|
||||
struct CodeActionResult {
|
||||
std::string Version;
|
||||
struct QuickFix {
|
||||
DiagRef Diag;
|
||||
Fix F;
|
||||
};
|
||||
std::vector<QuickFix> QuickFixes;
|
||||
std::vector<TweakRef> TweakRefs;
|
||||
};
|
||||
/// Surface code actions (quick-fixes for diagnostics, or available code
|
||||
/// tweaks) for a given range in a file.
|
||||
void codeAction(const CodeActionInputs &Inputs,
|
||||
Callback<CodeActionResult> CB);
|
||||
|
||||
/// Enumerate the code tweaks available to the user at a specified point.
|
||||
/// Tweaks where Filter returns false will not be checked or included.
|
||||
/// Deprecated, use codeAction instead.
|
||||
void enumerateTweaks(PathRef File, Range Sel,
|
||||
llvm::unique_function<bool(const Tweak &)> Filter,
|
||||
Callback<std::vector<TweakRef>> CB);
|
||||
|
||||
@ -962,16 +962,6 @@ struct Diagnostic {
|
||||
};
|
||||
llvm::json::Value toJSON(const Diagnostic &);
|
||||
|
||||
/// A LSP-specific comparator used to find diagnostic in a container like
|
||||
/// std:map.
|
||||
/// We only use the required fields of Diagnostic to do the comparison to avoid
|
||||
/// any regression issues from LSP clients (e.g. VScode), see
|
||||
/// https://git.io/vbr29
|
||||
struct LSPDiagnosticCompare {
|
||||
bool operator()(const Diagnostic &LHS, const Diagnostic &RHS) const {
|
||||
return std::tie(LHS.range, LHS.message) < std::tie(RHS.range, RHS.message);
|
||||
}
|
||||
};
|
||||
bool fromJSON(const llvm::json::Value &, Diagnostic &, llvm::json::Path);
|
||||
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Diagnostic &);
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user