[PATCH] D96608: [clangd] Delay binding LSP methods until initialize. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

This is NFC because the MessageHandler refused to dispatch to them until the
server is initialized anyway.

This is a more natural time to bind them - it's when they become callable, and
it's when client capabalities are available and server ones can be set.

One module-lifecycle function will be responsible for all three.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96608

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -171,6 +171,7 @@
   void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
                  Callback<llvm::json::Value> Reply);
 
+  void bindMethods();
   std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -162,15 +162,15 @@
     log("<-- {0}", Method);
     if (Method == "exit")
       return false;
-    if (!Server.Server) {
-      elog("Notification {0} before initialization", Method);
-    } else if (Method == "$/cancelRequest") {
+    if (Method == "$/cancelRequest") {
       onCancel(std::move(Params));
     } else if (auto Handler = Notifications.lookup(Method)) {
       Handler(std::move(Params));
       Server.maybeExportMemoryProfile();
       Server.maybeCleanupMemory();
-    } else {
+    } else if (!Server.Server) {
+      elog("Notification {0} before initialization", Method);
+    } else if (Method == "$/cancelRequest") {
       log("unhandled notification {0}", Method);
     }
     return true;
@@ -185,15 +185,16 @@
     SPAN_ATTACH(Tracer, "Params", Params);
     ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
     log("<-- {0}({1})", Method, ID);
-    if (!Server.Server && Method != "initialize") {
+    if (auto Handler = Calls.lookup(Method)) {
+      Handler(std::move(Params), std::move(Reply));
+    } else if (!Server.Server) {
       elog("Call {0} before initialization.", Method);
       Reply(llvm::make_error<LSPError>("server not initialized",
                                        ErrorCode::ServerNotInitialized));
-    } else if (auto Handler = Calls.lookup(Method))
-      Handler(std::move(Params), std::move(Reply));
-    else
+    } else {
       Reply(llvm::make_error<LSPError>("method not found",
                                        ErrorCode::MethodNotFound));
+    }
     return true;
   }
 
@@ -568,6 +569,8 @@
     BackgroundIndexProgressState = BackgroundIndexProgress::Empty;
   BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
 
+  bindMethods();
+
   // Per LSP, renameProvider can be either boolean or RenameOptions.
   // RenameOptions will be specified if the client states it supports prepare.
   llvm::json::Value RenameProvider =
@@ -1490,9 +1493,11 @@
     this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider(
         Opts.ConfigProvider, this);
   }
+  MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
+}
 
+void ClangdLSPServer::bindMethods() {
   // clang-format off
-  MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
   MsgHandler->bind("initialized", &ClangdLSPServer::onInitialized);
   MsgHandler->bind("shutdown", &ClangdLSPServer::onShutdown);
   MsgHandler->bind("sync", &ClangdLSPServer::onSync);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96608.323330.patch
Type: text/x-patch
Size: 3214 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210212/ab54e064/attachment.bin>