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 @@ -77,7 +77,7 @@ /// Transforms a tweak into a code action that would apply it if executed. /// EXPECTS: T.prepare() was called and returned true. CodeAction toCodeAction(const ClangdServer::TweakRef &T, const URIForFile &File, - Range Selection) { + Range Selection, llvm::ArrayRef Diags) { CodeAction CA; CA.title = T.Title; CA.kind = T.Kind.str(); @@ -93,6 +93,7 @@ Args.file = File; Args.tweakID = T.ID; Args.selection = Selection; + Args.diagnostics = Diags.vec(); CA.command->argument = std::move(Args); return CA; } @@ -754,7 +755,7 @@ return applyEdit(std::move(WE), "Tweak applied.", std::move(Reply)); }; Server->applyTweak(Args.file.file(), Args.selection, Args.tweakID, - std::move(Action)); + Args.diagnostics, std::move(Action)); } void ClangdLSPServer::applyEdit(WorkspaceEdit WE, llvm::json::Value Success, @@ -982,7 +983,7 @@ // Now enumerate the semantic code actions. auto ConsumeActions = [Reply = std::move(Reply), File, Selection = Params.range, - FixIts = std::move(FixIts), this]( + Diags = Params.context.diagnostics, FixIts = std::move(FixIts), this]( llvm::Expected> Tweaks) mutable { if (!Tweaks) return Reply(Tweaks.takeError()); @@ -990,7 +991,7 @@ std::vector Actions = std::move(FixIts); Actions.reserve(Actions.size() + Tweaks->size()); for (const auto &T : *Tweaks) - Actions.push_back(toCodeAction(T, File, Selection)); + Actions.push_back(toCodeAction(T, File, Selection, Diags)); // If there's exactly one quick-fix, call it "preferred". // We never consider refactorings etc as preferred. @@ -1016,7 +1017,7 @@ return Reply(llvm::json::Array(Commands)); }; Server->enumerateTweaks( - File.file(), Params.range, + File.file(), Params.range, Params.context.diagnostics, [this, KindAllowed(std::move(KindAllowed))](const Tweak &T) { return Opts.TweakFilter(T) && KindAllowed(T.kind()); }, diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -33,6 +33,7 @@ #include "support/ThreadsafeFS.h" #include "clang/Tooling/CompilationDatabase.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/Optional.h" @@ -315,12 +316,13 @@ /// Enumerate the code tweaks available to the user at a specified point. /// Tweaks where Filter returns false will not be checked or included. void enumerateTweaks(PathRef File, Range Sel, + llvm::ArrayRef Diags, llvm::unique_function Filter, Callback> CB); /// Apply the code tweak with a specified \p ID. void applyTweak(PathRef File, Range Sel, StringRef ID, - Callback CB); + llvm::ArrayRef Diags, Callback CB); /// Called when an event occurs for a watched file in the workspace. void onFileEvent(const DidChangeWatchedFilesParams &Params); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -553,7 +553,8 @@ // May generate several candidate selections, due to SelectionTree ambiguity. // vector of pointers because GCC doesn't like non-copyable Selection. static llvm::Expected>> -tweakSelection(const Range &Sel, const InputsAndAST &AST) { +tweakSelection(const Range &Sel, const InputsAndAST &AST, + llvm::ArrayRef Diags) { auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start); if (!Begin) return Begin.takeError(); @@ -565,7 +566,7 @@ AST.AST.getASTContext(), AST.AST.getTokens(), *Begin, *End, [&](SelectionTree T) { Result.push_back(std::make_unique( - AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T))); + AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T), Diags)); return false; }); assert(!Result.empty() && "Expected at least one SelectionTree"); @@ -573,18 +574,19 @@ } void ClangdServer::enumerateTweaks( - PathRef File, Range Sel, llvm::unique_function Filter, + PathRef File, Range Sel, llvm::ArrayRef Diags, + llvm::unique_function Filter, Callback> CB) { // Tracks number of times a tweak has been offered. static constexpr trace::Metric TweakAvailable( "tweak_available", trace::Metric::Counter, "tweak_id"); auto Action = [File = File.str(), Sel, CB = std::move(CB), - Filter = std::move(Filter), + Filter = std::move(Filter), Diags = Diags.vec(), FeatureModules(this->FeatureModules)]( Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - auto Selections = tweakSelection(Sel, *InpAST); + auto Selections = tweakSelection(Sel, *InpAST, Diags); if (!Selections) return CB(Selections.takeError()); std::vector Res; @@ -609,6 +611,7 @@ } void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID, + llvm::ArrayRef Diags, Callback CB) { // Tracks number of times a tweak has been attempted. static constexpr trace::Metric TweakAttempt( @@ -618,11 +621,11 @@ "tweak_failed", trace::Metric::Counter, "tweak_id"); TweakAttempt.record(1, TweakID); auto Action = [File = File.str(), Sel, TweakID = TweakID.str(), - CB = std::move(CB), + CB = std::move(CB), Diags = Diags.vec(), this](Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - auto Selections = tweakSelection(Sel, *InpAST); + auto Selections = tweakSelection(Sel, *InpAST, Diags); if (!Selections) return CB(Selections.takeError()); llvm::Optional> Effect; 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 @@ -27,6 +27,7 @@ #include "index/SymbolID.h" #include "support/MemoryTree.h" #include "clang/Index/IndexSymbol.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/Optional.h" #include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" @@ -927,6 +928,9 @@ Range selection; /// ID of the tweak that should be executed. Corresponds to Tweak::id(). std::string tweakID; + /// Set of diagnostics attached to the codeAction request when this tweak was + /// emitted. + std::vector diagnostics; }; bool fromJSON(const llvm::json::Value &, TweakArgs &, llvm::json::Path); llvm::json::Value toJSON(const TweakArgs &A); 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 @@ -819,12 +819,14 @@ llvm::json::Path P) { llvm::json::ObjectMapper O(Params, P); return O && O.map("file", A.file) && O.map("selection", A.selection) && - O.map("tweakID", A.tweakID); + O.map("tweakID", A.tweakID) && O.map("diagnostics", A.diagnostics); } llvm::json::Value toJSON(const TweakArgs &A) { - return llvm::json::Object{ - {"tweakID", A.tweakID}, {"selection", A.selection}, {"file", A.file}}; + return llvm::json::Object{{"tweakID", A.tweakID}, + {"selection", A.selection}, + {"file", A.file}, + {"diagnostics", A.diagnostics}}; } llvm::json::Value toJSON(const ApplyWorkspaceEditParams &Params) { diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h --- a/clang-tools-extra/clangd/refactor/Tweak.h +++ b/clang-tools-extra/clangd/refactor/Tweak.h @@ -51,7 +51,8 @@ /// Input to prepare and apply tweaks. struct Selection { Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin, - unsigned RangeEnd, SelectionTree ASTSelection); + unsigned RangeEnd, SelectionTree ASTSelection, + llvm::ArrayRef Diags); /// The text of the active document. llvm::StringRef Code; /// The Index for handling codebase related queries. @@ -67,6 +68,8 @@ unsigned SelectionEnd; /// The AST nodes that were selected. SelectionTree ASTSelection; + /// Diagnostics related to this selection. + llvm::ArrayRef Diags; // FIXME: provide a way to get sources and ASTs for other files. }; diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp --- a/clang-tools-extra/clangd/refactor/Tweak.cpp +++ b/clang-tools-extra/clangd/refactor/Tweak.cpp @@ -7,10 +7,12 @@ //===----------------------------------------------------------------------===// #include "Tweak.h" #include "FeatureModule.h" +#include "Protocol.h" #include "SourceCode.h" #include "index/Index.h" #include "support/Logger.h" #include "support/Path.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" @@ -61,9 +63,11 @@ Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd, - SelectionTree ASTSelection) + SelectionTree ASTSelection, + llvm::ArrayRef Diags) : Index(Index), AST(&AST), SelectionBegin(RangeBegin), - SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) { + SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)), + Diags(Diags) { auto &SM = AST.getSourceManager(); Code = SM.getBufferData(SM.getMainFileID()); Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin); diff --git a/clang-tools-extra/clangd/test/code-action-request.test b/clang-tools-extra/clangd/test/code-action-request.test --- a/clang-tools-extra/clangd/test/code-action-request.test +++ b/clang-tools-extra/clangd/test/code-action-request.test @@ -32,6 +32,7 @@ # CHECK-NEXT: { # CHECK-NEXT: "arguments": [ # CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [], # CHECK-NEXT: "file": "file://{{.*}}/clangd-test/main.cpp", # CHECK-NEXT: "selection": { # CHECK-NEXT: "end": { @@ -92,7 +93,7 @@ # CHECK-NEXT: "result": [ # CHECK-NEXT: { --- -{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} # CHECK: "newText": "int", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { diff --git a/clang-tools-extra/clangd/test/request-reply.test b/clang-tools-extra/clangd/test/request-reply.test --- a/clang-tools-extra/clangd/test/request-reply.test +++ b/clang-tools-extra/clangd/test/request-reply.test @@ -3,7 +3,7 @@ --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto i = 0;"}}} --- -{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} # CHECK: "id": 0, # CHECK: "method": "workspace/applyEdit", # CHECK: "newText": "int", @@ -25,7 +25,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "id": 4, --- -{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} +{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}} # CHECK: "id": 1, # CHECK: "method": "workspace/applyEdit", --- diff --git a/clang-tools-extra/clangd/test/tweaks-format.test b/clang-tools-extra/clangd/test/tweaks-format.test --- a/clang-tools-extra/clangd/test/tweaks-format.test +++ b/clang-tools-extra/clangd/test/tweaks-format.test @@ -5,7 +5,7 @@ --- {"jsonrpc":"2.0","id":5,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///main.cc"},"range":{"start":{"line":0,"character":11},"end":{"line":0,"character":11}},"context":{"diagnostics":[]}}} --- -{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}} +{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}} # CHECK: "newText": "\n ", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -210,7 +210,7 @@ vlog(" {0} {1}", Pos, Tok.text(AST->getSourceManager())); auto Tree = SelectionTree::createRight(AST->getASTContext(), AST->getTokens(), Start, End); - Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree)); + Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree), {}); for (const auto &T : prepareTweaks(Selection, Opts.TweakFilter, Opts.FeatureModules)) { auto Result = T->apply(Selection); diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -11,6 +11,7 @@ #include "LSPClient.h" #include "Protocol.h" #include "TestFS.h" +#include "URI.h" #include "support/Logger.h" #include "support/TestTracer.h" #include "llvm/ADT/StringRef.h" @@ -386,6 +387,62 @@ EXPECT_THAT(Client.diagnostics("foo.cpp"), llvm::ValueIs(testing::ElementsAre(DiagMessage(DiagMsg)))); } + +TEST_F(LSPTest, TweakForDiag) { + static constexpr const char *TweakID = "ModuleTweak"; + struct TweakContributingModule final : public FeatureModule { + struct ModuleTweak final : public Tweak { + const char *id() const override { return TweakID; } + bool prepare(const Selection &Sel) override { return !Sel.Diags.empty(); } + Expected apply(const Selection &Sel) override { + return error("not implemented"); + } + std::string title() const override { return id(); } + llvm::StringLiteral kind() const override { return ""; }; + }; + + void contributeTweaks(std::vector> &Out) override { + Out.emplace_back(new ModuleTweak); + } + }; + + FeatureModules.add(std::make_unique()); + auto &Client = start(); + Client.didOpen("foo.cpp", "test;"); + auto Diags = Client.diagnostics("foo.cpp"); + ASSERT_TRUE(Diags.hasValue()); + ASSERT_EQ(Diags->size(), 1U); + Diagnostic Diag; + llvm::json::Path::Root R; + + ASSERT_TRUE(fromJSON(Diags->front(), Diag, R)); + + llvm::json::Object CodeActionParams; + CodeActionParams["textDocument"] = llvm::json::Object{ + {"uri", URI::createFile(testPath("foo.cpp")).toString()}}; + CodeActionParams["range"] = Diag.range; + CodeActionParams["context"] = + llvm::json::Object{{"diagnostics", llvm::json::Array{Diag}}}; + + // Check diags is propagated to code action. + auto ActionsStorage = + Client.call("textDocument/codeAction", std::move(CodeActionParams)) + .takeValue(); + auto *Actions = ActionsStorage.getAsArray(); + ASSERT_TRUE(Actions); + ASSERT_EQ(Actions->size(), 1U); + auto *Action = Actions->front().getAsObject(); + ASSERT_TRUE(Action); + EXPECT_THAT(Action->getString("title"), llvm::ValueIs(TweakID)); + + // Ensure diags is also propagated on execute. + llvm::json::Object ExecParams; + ExecParams["command"] = *Action->getString("command"); + ExecParams["arguments"] = llvm::json::Array(*Action->getArray("arguments")); + auto Res = + Client.call("workspace/executeCommand", std::move(ExecParams)).take(); + EXPECT_EQ(llvm::toString(Res.takeError()), "not implemented"); +} } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp --- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp +++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp @@ -45,7 +45,7 @@ auto Tree = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0, 0); auto Actual = prepareTweak( - TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree)), &Set); + TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree), {}), &Set); ASSERT_TRUE(bool(Actual)); EXPECT_EQ(Actual->get()->id(), TweakID); } diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp @@ -74,7 +74,8 @@ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first, Range.second, [&](SelectionTree ST) { Tweak::Selection S(Index, AST, Range.first, - Range.second, std::move(ST)); + Range.second, std::move(ST), + {}); if (auto T = prepareTweak(TweakID, S, nullptr)) { Result = (*T)->apply(S); return true;