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); @@ -184,9 +201,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(); @@ -219,35 +233,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 @@ -414,8 +414,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); @@ -582,7 +582,7 @@ }; { - LSPBinder Binder(Handlers); + LSPBinder Binder(Handlers, *this); if (Opts.Modules) for (auto &Mod : *Opts.Modules) Mod.initializeLSP(Binder, Params.capabilities, ServerCaps); @@ -739,7 +739,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()) @@ -765,10 +765,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) { @@ -854,7 +853,7 @@ // executed after it returns. PublishDiagnosticsParams Notification; Notification.uri = URIForFile::canonicalize(File, /*TUPath=*/File); - publishDiagnostics(Notification); + PublishDiagnostics(Notification); } void ClangdLSPServer::onDocumentOnTypeFormatting( @@ -1254,11 +1253,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; @@ -1459,7 +1453,7 @@ } // clang-format off - LSPBinder Bind(this->Handlers); + LSPBinder Bind(this->Handlers, *this); Bind.method("initialize", this, &ClangdLSPServer::onInitialize); Bind.notification("initialized", this, &ClangdLSPServer::onInitialized); Bind.method("shutdown", this, &ClangdLSPServer::onShutdown); @@ -1504,6 +1498,15 @@ Bind.method("textDocument/foldingRange", this, &ClangdLSPServer::onFoldingRange); Bind.command(APPLY_FIX_COMMAND, this, &ClangdLSPServer::onCommandApplyEdit); Bind.command(APPLY_TWEAK_COMMAND, this, &ClangdLSPServer::onCommandApplyTweak); + + Bind.outgoingMethod("workspace/applyEdit", ApplyWorkspaceEdit); + Bind.outgoingNotification("textDocument/publishDiagnostics", PublishDiagnostics); + Bind.outgoingNotification("window/showMessage", ShowMessage); + Bind.outgoingNotification("textDocument/clangd.fileStatus", NotifyFileStatus); + Bind.outgoingMethod("window/workDoneProgress/create", CreateWorkDoneProgress); + Bind.outgoingNotification("$/progress", BeginWorkDoneProgress); + Bind.outgoingNotification("$/progress", ReportWorkDoneProgress); + Bind.outgoingNotification("$/progress", EndWorkDoneProgress); // clang-format on } @@ -1590,7 +1593,7 @@ } // Send a notification to the LSP client. - publishDiagnostics(Notification); + PublishDiagnostics(Notification); } void ClangdLSPServer::onBackgroundIndexProgress( @@ -1607,7 +1610,7 @@ WorkDoneProgressBegin Begin; Begin.percentage = true; Begin.title = "indexing"; - progress(ProgressToken, std::move(Begin)); + BeginWorkDoneProgress({ProgressToken, std::move(Begin)}); BackgroundIndexProgressState = BackgroundIndexProgress::Live; } @@ -1619,10 +1622,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; } }; @@ -1644,8 +1647,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) { @@ -1676,7 +1679,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,52 @@ void command(llvm::StringLiteral Command, ThisT *This, void (ThisT::*Handler)(const Param &, Callback)); + template + using OutgoingMethod = llvm::unique_function)>; + /// Bind a function object to be used for outgoing method calls. + /// e.g. Bind.outgoingMethod("edit", &Edit); + /// Function should be e.g. + /// llvm::unique_function)> Edit; + /// EditParams must be JSON-serializable, EditResult must be parseable. + template + void outgoingMethod(llvm::StringLiteral Method, + OutgoingMethod &Handler) { + Handler = [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")); + }); + }; + } + + template + using OutgoingNotification = llvm::unique_function; + /// Bind a function object to be used for outgoing notifications. + /// e.g. Bind.outgoingMethod("log", &Log); + /// Function should be e.g. + /// llvm::unique_function Log; + /// LogParams must be JSON-serializable. + template + void outgoingNotification(llvm::StringLiteral Method, + OutgoingNotification &Handler) { + Handler = [Method, Out(&Out)](Request R) { Out->notify(Method, JSON(R)); }; + } + +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 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 @@ -13,8 +13,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 and ClangdServer @@ -41,6 +39,14 @@ virtual void initializeLSP(LSPBinder &Bind, const ClientCapabilities &ClientCaps, llvm::json::Object &ServerCaps) {} + +protected: + /// Types of function objects that modules use for outgoing calls. + /// (Bound throuh LSPBinder, made available here for convenience). + template + using OutgoingNotification = LSPBinder::OutgoingNotification; + template + using OutgoingMethod = LSPBinder::OutgoingMethod; }; class ModuleSet { 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,13 +224,18 @@ TEST_F(LSPTest, ModulesTest) { class MathModule : public Module { + OutgoingNotification Changed; void initializeLSP(LSPBinder &Bind, const ClientCapabilities &ClientCaps, llvm::json::Object &ServerCaps) override { Bind.notification("add", this, &MathModule::add); Bind.method("get", this, &MathModule::get); + Bind.outgoingNotification("changed", Changed); } - void add(const int &X) { Value += X; } + void add(const int &X) { + Value += X; + Changed(Value); + } void get(const std::nullptr_t &, Callback Reply) { Reply(Value); } int Value = 0; }; @@ -242,6 +248,8 @@ Client.notify("add", 2); 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; + Binder.outgoingMethod("echo", Echo); + LSPBinder::OutgoingMethod WrongSignature; + Binder.outgoingMethod("wrongSignature", WrongSignature); + LSPBinder::OutgoingMethod Fail; + Binder.outgoingMethod("fail", 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