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 @@ -39,7 +39,8 @@ /// MessageHandler binds the implemented LSP methods (e.g. onInitialize) to /// corresponding JSON-RPC methods ("initialize"). /// The server also supports $/cancelRequest (MessageHandler provides this). -class ClangdLSPServer : private ClangdServer::Callbacks { +class ClangdLSPServer : private ClangdServer::Callbacks, + private LSPBinder::RawOutgoing { public: struct Options : ClangdServer::Options { /// Supplies configuration (overrides ClangdServer::ContextProvider). @@ -165,6 +166,22 @@ void onCommandApplyEdit(const WorkspaceEdit &, Callback); void onCommandApplyTweak(const TweakArgs &, Callback); + /// Outgoing LSP calls. + LSPBinder::OutgoingMethod + ApplyWorkspaceEdit; + LSPBinder::OutgoingNotification ShowMessage; + LSPBinder::OutgoingNotification PublishDiagnostics; + LSPBinder::OutgoingNotification NotifyFileStatus; + LSPBinder::OutgoingMethod + CreateWorkDoneProgress; + LSPBinder::OutgoingNotification> + BeginWorkDoneProgress; + LSPBinder::OutgoingNotification> + ReportWorkDoneProgress; + LSPBinder::OutgoingNotification> + EndWorkDoneProgress; + void applyEdit(WorkspaceEdit WE, llvm::json::Value Success, Callback Reply); @@ -185,9 +202,6 @@ llvm::function_ref Filter); void applyConfiguration(const ConfigurationSettings &Settings); - /// Sends a "publishDiagnostics" notification to the LSP client. - void publishDiagnostics(const PublishDiagnosticsParams &); - /// Runs profiling and exports memory usage metrics if tracing is enabled and /// profiling hasn't happened recently. void maybeExportMemoryProfile(); @@ -220,35 +234,16 @@ std::mutex SemanticTokensMutex; llvm::StringMap LastSemanticTokens; - // Most code should not deal with Transport directly. - // MessageHandler deals with incoming messages, use call() etc for outgoing. + // Most code should not deal with Transport, callMethod, notify directly. + // Use LSPBinder to handle incoming and outgoing calls. clangd::Transport &Transp; class MessageHandler; std::unique_ptr MsgHandler; std::mutex TranspWriter; - template - void call(StringRef Method, llvm::json::Value Params, Callback CB) { - // Wrap the callback with LSP conversion and error-handling. - auto HandleReply = - [CB = std::move(CB), Ctx = Context::current().clone(), - Method = Method.str()]( - llvm::Expected RawResponse) mutable { - if (!RawResponse) - return CB(RawResponse.takeError()); - CB(LSPBinder::parse(*RawResponse, Method, "response")); - }; - callRaw(Method, std::move(Params), std::move(HandleReply)); - } - void callRaw(StringRef Method, llvm::json::Value Params, - Callback CB); - void notify(StringRef Method, llvm::json::Value Params); - template void progress(const llvm::json::Value &Token, T Value) { - ProgressParams Params; - Params.token = Token; - Params.value = std::move(Value); - notify("$/progress", Params); - } + void callMethod(StringRef Method, llvm::json::Value Params, + Callback CB) override; + void notify(StringRef Method, llvm::json::Value Params) override; LSPBinder::RawHandlers Handlers; 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 @@ -410,8 +410,8 @@ constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks; // call(), notify(), and reply() wrap the Transport, adding logging and locking. -void ClangdLSPServer::callRaw(StringRef Method, llvm::json::Value Params, - Callback CB) { +void ClangdLSPServer::callMethod(StringRef Method, llvm::json::Value Params, + Callback CB) { auto ID = MsgHandler->bindReply(std::move(CB)); log("--> {0}({1})", Method, ID); std::lock_guard Lock(TranspWriter); @@ -578,7 +578,7 @@ }; { - LSPBinder Binder(Handlers); + LSPBinder Binder(Handlers, *this); bindMethods(Binder); if (Opts.Modules) for (auto &Mod : *Opts.Modules) @@ -736,7 +736,7 @@ ShowMessageParams Msg; Msg.message = *R->ShowMessage; Msg.type = MessageType::Info; - notify("window/showMessage", Msg); + ShowMessage(Msg); } // When no edit is specified, make sure we Reply(). if (R->ApplyEdits.empty()) @@ -762,10 +762,9 @@ Callback Reply) { ApplyWorkspaceEditParams Edit; Edit.edit = std::move(WE); - call( - "workspace/applyEdit", std::move(Edit), - [Reply = std::move(Reply), SuccessMessage = std::move(Success)]( - llvm::Expected Response) mutable { + ApplyWorkspaceEdit( + Edit, [Reply = std::move(Reply), SuccessMessage = std::move(Success)]( + llvm::Expected Response) mutable { if (!Response) return Reply(Response.takeError()); if (!Response->applied) { @@ -851,7 +850,7 @@ // executed after it returns. PublishDiagnosticsParams Notification; Notification.uri = URIForFile::canonicalize(File, /*TUPath=*/File); - publishDiagnostics(Notification); + PublishDiagnostics(Notification); } void ClangdLSPServer::onDocumentOnTypeFormatting( @@ -1251,11 +1250,6 @@ [&](llvm::StringRef File) { return ModifiedFiles.count(File) != 0; }); } -void ClangdLSPServer::publishDiagnostics( - const PublishDiagnosticsParams &Params) { - notify("textDocument/publishDiagnostics", Params); -} - void ClangdLSPServer::maybeExportMemoryProfile() { if (!trace::enabled() || !ShouldProfile()) return; @@ -1454,7 +1448,7 @@ this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider( Opts.ConfigProvider, this); } - LSPBinder Bind(this->Handlers); + LSPBinder Bind(this->Handlers, *this); Bind.method("initialize", this, &ClangdLSPServer::onInitialize); } @@ -1503,6 +1497,15 @@ Bind.method("textDocument/foldingRange", this, &ClangdLSPServer::onFoldingRange); Bind.command(APPLY_FIX_COMMAND, this, &ClangdLSPServer::onCommandApplyEdit); Bind.command(APPLY_TWEAK_COMMAND, this, &ClangdLSPServer::onCommandApplyTweak); + + ApplyWorkspaceEdit = Bind.outgoingMethod("workspace/applyEdit"); + PublishDiagnostics = Bind.outgoingNotification("textDocument/publishDiagnostics"); + ShowMessage = Bind.outgoingNotification("window/showMessage"); + NotifyFileStatus = Bind.outgoingNotification("textDocument/clangd.fileStatus"); + CreateWorkDoneProgress = Bind.outgoingMethod("window/workDoneProgress/create"); + BeginWorkDoneProgress = Bind.outgoingNotification("$/progress"); + ReportWorkDoneProgress = Bind.outgoingNotification("$/progress"); + EndWorkDoneProgress = Bind.outgoingNotification("$/progress"); // clang-format on } @@ -1589,7 +1592,7 @@ } // Send a notification to the LSP client. - publishDiagnostics(Notification); + PublishDiagnostics(Notification); } void ClangdLSPServer::onBackgroundIndexProgress( @@ -1606,7 +1609,7 @@ WorkDoneProgressBegin Begin; Begin.percentage = true; Begin.title = "indexing"; - progress(ProgressToken, std::move(Begin)); + BeginWorkDoneProgress({ProgressToken, std::move(Begin)}); BackgroundIndexProgressState = BackgroundIndexProgress::Live; } @@ -1618,10 +1621,10 @@ Report.message = llvm::formatv("{0}/{1}", Stats.Completed - Stats.LastIdle, Stats.Enqueued - Stats.LastIdle); - progress(ProgressToken, std::move(Report)); + ReportWorkDoneProgress({ProgressToken, std::move(Report)}); } else { assert(Stats.Completed == Stats.Enqueued); - progress(ProgressToken, WorkDoneProgressEnd()); + EndWorkDoneProgress({ProgressToken, WorkDoneProgressEnd()}); BackgroundIndexProgressState = BackgroundIndexProgress::Empty; } }; @@ -1643,8 +1646,8 @@ BackgroundIndexProgressState = BackgroundIndexProgress::Creating; WorkDoneProgressCreateParams CreateRequest; CreateRequest.token = ProgressToken; - call( - "window/workDoneProgress/create", CreateRequest, + CreateWorkDoneProgress( + CreateRequest, [this, NotifyProgress](llvm::Expected E) { std::lock_guard Lock(BackgroundIndexProgressMutex); if (E) { @@ -1675,7 +1678,7 @@ (Status.ASTActivity.K == ASTAction::Building || Status.ASTActivity.K == ASTAction::RunningAction)) return; - notify("textDocument/clangd.fileStatus", Status.render(File)); + NotifyFileStatus(Status.render(File)); } void ClangdLSPServer::reparseOpenFilesIfNeeded( diff --git a/clang-tools-extra/clangd/LSPBinder.h b/clang-tools-extra/clangd/LSPBinder.h --- a/clang-tools-extra/clangd/LSPBinder.h +++ b/clang-tools-extra/clangd/LSPBinder.h @@ -43,8 +43,15 @@ HandlerMap)> MethodHandlers; HandlerMap)> CommandHandlers; }; + class RawOutgoing { + public: + virtual ~RawOutgoing() = default; + virtual void callMethod(llvm::StringRef Method, JSON Params, + Callback Reply) = 0; + virtual void notify(llvm::StringRef Method, JSON Params) = 0; + }; - LSPBinder(RawHandlers &Raw) : Raw(Raw) {} + LSPBinder(RawHandlers &Raw, RawOutgoing &Out) : Raw(Raw), Out(Out) {} /// Bind a handler for an LSP method. /// e.g. Bind.method("peek", this, &ThisModule::peek); @@ -70,14 +77,33 @@ void command(llvm::StringLiteral Command, ThisT *This, void (ThisT::*Handler)(const Param &, Callback)); + template + using OutgoingMethod = llvm::unique_function)>; + /// UntypedOutgoingMethod is convertible to OutgoingMethod. + class UntypedOutgoingMethod; + /// Bind a function object to be used for outgoing method calls. + /// e.g. OutgoingMethod Edit = Bind.outgoingMethod("edit"); + /// EParams must be JSON-serializable, EResult must be parseable. + UntypedOutgoingMethod outgoingMethod(llvm::StringLiteral Method); + + template + using OutgoingNotification = llvm::unique_function; + /// UntypedOutgoingNotification is convertible to OutgoingNotification. + class UntypedOutgoingNotification; + /// Bind a function object to be used for outgoing notifications. + /// e.g. OutgoingNotification Log = Bind.outgoingMethod("log"); + /// LogParams must be JSON-serializable. + UntypedOutgoingNotification outgoingNotification(llvm::StringLiteral Method); + +private: // FIXME: remove usage from ClangdLSPServer and make this private. template static llvm::Expected parse(const llvm::json::Value &Raw, llvm::StringRef PayloadName, llvm::StringRef PayloadKind); -private: RawHandlers &Raw; + RawOutgoing &Out; }; template @@ -141,6 +167,56 @@ }; } +class LSPBinder::UntypedOutgoingNotification { + llvm::StringLiteral Method; + RawOutgoing *Out; + UntypedOutgoingNotification(llvm::StringLiteral Method, RawOutgoing *Out) + : Method(Method), Out(Out) {} + friend UntypedOutgoingNotification + LSPBinder::outgoingNotification(llvm::StringLiteral); + +public: + template operator OutgoingNotification() && { + return + [Method(Method), Out(Out)](Request R) { Out->notify(Method, JSON(R)); }; + } +}; + +inline LSPBinder::UntypedOutgoingNotification +LSPBinder::outgoingNotification(llvm::StringLiteral Method) { + return UntypedOutgoingNotification(Method, &Out); +} + +class LSPBinder::UntypedOutgoingMethod { + llvm::StringLiteral Method; + RawOutgoing *Out; + UntypedOutgoingMethod(llvm::StringLiteral Method, RawOutgoing *Out) + : Method(Method), Out(Out) {} + friend UntypedOutgoingMethod LSPBinder::outgoingMethod(llvm::StringLiteral); + +public: + template + operator OutgoingMethod() && { + return [Method(Method), Out(Out)](Request R, Callback Reply) { + Out->callMethod( + Method, JSON(R), + // FIXME: why keep ctx alive but not restore it for the callback? + [Reply(std::move(Reply)), Ctx(Context::current().clone()), + Method](llvm::Expected RawRsp) mutable { + if (!RawRsp) + return Reply(RawRsp.takeError()); + Reply(LSPBinder::parse(std::move(*RawRsp), Method, + "reply")); + }); + }; + } +}; + +inline LSPBinder::UntypedOutgoingMethod +LSPBinder::outgoingMethod(llvm::StringLiteral Method) { + return UntypedOutgoingMethod(Method, &Out); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Module.h b/clang-tools-extra/clangd/Module.h --- a/clang-tools-extra/clangd/Module.h +++ b/clang-tools-extra/clangd/Module.h @@ -9,6 +9,8 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H +#include "support/Function.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/JSON.h" @@ -25,8 +27,6 @@ /// A Module contributes a vertical feature to clangd. /// -/// FIXME: Extend this to support outgoing LSP calls. -/// /// The lifetime of a module is roughly: /// - modules are created before the LSP server, in ClangdMain.cpp /// - these modules are then passed to ClangdLSPServer in a ModuleSet @@ -73,6 +73,13 @@ /// The filesystem is used to read source files on disk. const ThreadsafeFS &fs() { return facilities().FS; } + /// Types of function objects that modules use for outgoing calls. + /// (Bound throuh LSPBinder, made available here for convenience). + template + using OutgoingNotification = llvm::unique_function; + template + using OutgoingMethod = llvm::unique_function)>; + private: llvm::Optional Fac; }; 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 @@ -23,6 +23,7 @@ namespace clang { namespace clangd { namespace { +using testing::ElementsAre; MATCHER_P(DiagMessage, M, "") { if (const auto *O = arg.getAsObject()) { @@ -223,16 +224,20 @@ TEST_F(LSPTest, ModulesTest) { class MathModule final : public Module { + OutgoingNotification Changed; void initializeLSP(LSPBinder &Bind, const llvm::json::Object &ClientCaps, llvm::json::Object &ServerCaps) override { Bind.notification("add", this, &MathModule::add); Bind.method("get", this, &MathModule::get); + Changed = Bind.outgoingNotification("changed"); } int Value = 0; - public: - void add(const int &X) { Value += X; } + void add(const int &X) { + Value += X; + Changed(Value); + } void get(const std::nullptr_t &, Callback Reply) { scheduler().runQuick( "get", "", @@ -245,8 +250,10 @@ auto &Client = start(); Client.notify("add", 2); - Mods.get()->add(8); + Client.notify("add", 8); EXPECT_EQ(10, Client.call("get", nullptr).takeValue()); + EXPECT_THAT(Client.takeNotifications("changed"), + ElementsAre(llvm::json::Value(2), llvm::json::Value(10))); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/LSPBinderTests.cpp b/clang-tools-extra/clangd/unittests/LSPBinderTests.cpp --- a/clang-tools-extra/clangd/unittests/LSPBinderTests.cpp +++ b/clang-tools-extra/clangd/unittests/LSPBinderTests.cpp @@ -15,12 +15,15 @@ namespace clangd { namespace { +using testing::ElementsAre; using testing::HasSubstr; +using testing::IsEmpty; using testing::UnorderedElementsAre; // JSON-serializable type for testing. struct Foo { int X; + friend bool operator==(Foo A, Foo B) { return A.X == B.X; } }; bool fromJSON(const llvm::json::Value &V, Foo &F, llvm::json::Path P) { return fromJSON(V, F.X, P.field("X")); @@ -35,9 +38,31 @@ return [&Out](llvm::Expected V) { Out.emplace(std::move(V)); }; } +struct OutgoingRecorder : public LSPBinder::RawOutgoing { + llvm::StringMap> Received; + + void callMethod(llvm::StringRef Method, llvm::json::Value Params, + Callback Reply) override { + Received[Method].push_back(Params); + if (Method == "fail") + return Reply(error("Params={0}", Params)); + Reply(Params); // echo back the request + } + void notify(llvm::StringRef Method, llvm::json::Value Params) override { + Received[Method].push_back(std::move(Params)); + } + + std::vector take(llvm::StringRef Method) { + std::vector Result = Received.lookup(Method); + Received.erase(Method); + return Result; + } +}; + TEST(LSPBinderTest, IncomingCalls) { LSPBinder::RawHandlers RawHandlers; - LSPBinder Binder{RawHandlers}; + OutgoingRecorder RawOutgoing; + LSPBinder Binder{RawHandlers, RawOutgoing}; struct Handler { void plusOne(const Foo &Params, Callback Reply) { Reply(Foo{Params.X + 1}); @@ -94,7 +119,45 @@ RawCmdPlusOne(1, capture(Reply)); ASSERT_TRUE(Reply.hasValue()); EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2)); + + // None of this generated any outgoing traffic. + EXPECT_THAT(RawOutgoing.Received, IsEmpty()); +} + +TEST(LSPBinderTest, OutgoingCalls) { + LSPBinder::RawHandlers RawHandlers; + OutgoingRecorder RawOutgoing; + LSPBinder Binder{RawHandlers, RawOutgoing}; + + LSPBinder::OutgoingMethod Echo; + Echo = Binder.outgoingMethod("echo"); + LSPBinder::OutgoingMethod WrongSignature; + WrongSignature = Binder.outgoingMethod("wrongSignature"); + LSPBinder::OutgoingMethod Fail; + Fail = Binder.outgoingMethod("fail"); + + llvm::Optional> Reply; + Echo(Foo{2}, capture(Reply)); + EXPECT_THAT(RawOutgoing.take("echo"), ElementsAre(llvm::json::Value(2))); + ASSERT_TRUE(Reply.hasValue()); + EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(Foo{2})); + + // JSON response is integer, can't be parsed as string. + llvm::Optional> WrongTypeReply; + WrongSignature(Foo{2}, capture(WrongTypeReply)); + EXPECT_THAT(RawOutgoing.take("wrongSignature"), + ElementsAre(llvm::json::Value(2))); + ASSERT_TRUE(Reply.hasValue()); + EXPECT_THAT_EXPECTED(WrongTypeReply.getValue(), + llvm::FailedWithMessage( + HasSubstr("failed to decode wrongSignature reply"))); + + Fail(Foo{2}, capture(Reply)); + EXPECT_THAT(RawOutgoing.take("fail"), ElementsAre(llvm::json::Value(2))); + ASSERT_TRUE(Reply.hasValue()); + EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::FailedWithMessage("Params=2")); } + } // namespace } // namespace clangd } // namespace clang