[clangd][NFC] Simplify handing on methods with no params

[njames93]

Summary

Add bind methods handling the case when a method has an empty params interface and when it has no parameters.

Remove ShutdownParams and ExitParams from Protocol, In LSP they aren't defined, instead the methods are defined to have void as the params. This signature now better reflects that.

[kadircet]

thanks this LG, but I suppose we can do some more trimming. after the trimming we are introducing a single partial specialization which is luckily more specialized than existing notification-binding specialization, so all should be fine.

clang-tools-extra/clangd/ClangdLSPServer.cpp

262

why do we need this one ?

321

i suppose we can also drop this one after getting rid of InitializedParams?

clang-tools-extra/clangd/ClangdLSPServer.h

95–96

any reasons for keeping InitializedParams ?

clang-tools-extra/clangd/Protocol.h

267–268

and drop this too.

[sammccall]

I'm not convinced this is clearer - these methods do in fact return null.
And it adds more duplicative code. Is it necessary?

[sammccall]

In D95270#2519052, @sammccall wrote:

I'm not convinced this is clearer - these methods do in fact return null.
And it adds more duplicative code. Is it necessary?

Doh, of course I misread - inputs rather than outputs.
Yes, this seems OK if we drop the NoParams variants.
(Though I'm still not sure it's worth the special case...)

[njames93]

Remove unnecessary specialization.

clang-tools-extra/clangd/ClangdLSPServer.cpp

262

Good point, this one is never actually used now.

clang-tools-extra/clangd/ClangdLSPServer.h

95–96

Although its empty, its defined in the protocol so we should be mirroring it.

clang-tools-extra/clangd/Protocol.h

267–268

See previous comment about mirroring the spec.

clang-tools-extra/clangd/ClangdLSPServer.h

private:
   void
   onHighlightingsReady(PathRef File, llvm::StringRef Version,
                        std::vector<HighlightingToken> Highlightings) override;
   void onBackgroundIndexProgress(const BackgroundQueue::Stats &Stats) override;
 
   // LSP methods. Notifications have signature void(const Params&).
   // Calls have signature void(const Params&, Callback<Response>).
   void onInitialize(const InitializeParams &, Callback<llvm::json::Value>);
   void onInitialized(const InitializedParams &);
-  void onShutdown(const ShutdownParams &, Callback<std::nullptr_t>);
-  void onSync(const NoParams &, Callback<std::nullptr_t>);
+  void onShutdown(Callback<std::nullptr_t>);
+  void onSync(Callback<std::nullptr_t>);
   void onDocumentDidOpen(const DidOpenTextDocumentParams &);
   void onDocumentDidChange(const DidChangeTextDocumentParams &);
   void onDocumentDidClose(const DidCloseTextDocumentParams &);
   void onDocumentDidSave(const DidSaveTextDocumentParams &);
   void onAST(const ASTParams &, Callback<llvm::Optional<ASTNode>>);
   void onDocumentOnTypeFormatting(const DocumentOnTypeFormattingParams &,
                                   Callback<std::vector<TextEdit>>);
   void onDocumentRangeFormatting(const DocumentRangeFormattingParams &,
void onSelectionRange(const SelectionRangeParams &,
                         Callback<std::vector<SelectionRange>>);
   void onDocumentLink(const DocumentLinkParams &,
                       Callback<std::vector<DocumentLink>>);
   void onSemanticTokens(const SemanticTokensParams &, Callback<SemanticTokens>);
   void onSemanticTokensDelta(const SemanticTokensDeltaParams &,
                              Callback<SemanticTokensOrDelta>);
   /// This is a clangd extension. Provides a json tree representing memory usage
   /// hierarchy.
-  void onMemoryUsage(const NoParams &, Callback<MemoryTree>);
+  void onMemoryUsage(Callback<MemoryTree>);
 
   std::vector<Fix> getFixes(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
   /// character" we specify, but for '>' and ':' we need to check they actually
   /// produce '->' and '::', respectively.
   bool shouldRunCompletion(const CompletionParams &Params) const;

clang-tools-extra/clangd/ClangdLSPServer.cpp

Calls[Method] = [Method, Handler, this](llvm::json::Value RawParams,
                                             ReplyOnce Reply) {
       auto P = parse<Param>(RawParams, Method, "request");
       if (!P)
         return Reply(P.takeError());
       (Server.*Handler)(*P, std::move(Reply));
     };
   }
 
+  template <typename Result>
+  void bind(const char *Method,
+            void (ClangdLSPServer::*Handler)(Callback<Result>)) {
+    Calls[Method] = [Handler, this](llvm::json::Value RawParams,
+                                    ReplyOnce Reply) {
+      (Server.*Handler)(std::move(Reply));
+    };
+  }
+
   // Bind a reply callback to a request. The callback will be invoked when
   // clangd receives the reply from the LSP client.
   // Return a call id of the request.
   llvm::json::Value bindReply(Callback<llvm::json::Value> Reply) {
     llvm::Optional<std::pair<int, Callback<llvm::json::Value>>> OldestCB;
     int ID;
     {
       std::lock_guard<std::mutex> Mutex(CallMutex);
Notifications[Method] = [Method, Handler,
       if (!P)
         return llvm::consumeError(P.takeError());
       trace::Span Tracer(Method, LSPLatency);
       SPAN_ATTACH(Tracer, "Params", RawParams);
       (Server.*Handler)(*P);
     };
   }
 
+  void bind(const char *Method, void (ClangdLSPServer::*Handler)()) {
+    Notifications[Method] = [Handler, this](llvm::json::Value RawParams) {
+      (Server.*Handler)();
+    };
+  }
+
+  template <>
+  void bind<NoParams>(const char *Method,
+                      void (ClangdLSPServer::*Handler)(const NoParams &)) {
+    Notifications[Method] = [Handler, this](llvm::json::Value RawParams) {
+      (Server.*Handler)(NoParams{});
+    };
+  }
+
 private:
   // Function object to reply to an LSP call.
   // Each instance must be called exactly once, otherwise:
   //  - the bug is logged, and (in debug mode) an assert will fire
   //  - if there was no reply, an error reply is sent
   //  - if there were multiple replies, only the first is sent
   class ReplyOnce {
     std::atomic<bool> Replied = {false};
Result.getObject("capabilities")
              llvm::json::Object{{"scopes", buildHighlightScopeLookupTable()}}});
   if (Opts.FoldingRanges)
     Result.getObject("capabilities")->insert({"foldingRangeProvider", true});
   Reply(std::move(Result));
 }
 
 void ClangdLSPServer::onInitialized(const InitializedParams &Params) {}
 
-void ClangdLSPServer::onShutdown(const ShutdownParams &Params,
-                                 Callback<std::nullptr_t> Reply) {
+void ClangdLSPServer::onShutdown(Callback<std::nullptr_t> Reply) {
   // Do essentially nothing, just say we're ready to exit.
   ShutdownRequestReceived = true;
   Reply(nullptr);
 }
 
 // sync is a clangd extension: it blocks until all background work completes.
 // It blocks the calling thread, so no messages are processed until it returns!
-void ClangdLSPServer::onSync(const NoParams &Params,
-                             Callback<std::nullptr_t> Reply) {
+void ClangdLSPServer::onSync(Callback<std::nullptr_t> Reply) {
   if (Server->blockUntilIdleForTest(/*TimeoutSeconds=*/60))
     Reply(nullptr);
   else
     Reply(error("Not idle after a minute"));
 }
 
 void ClangdLSPServer::onDocumentDidOpen(
     const DidOpenTextDocumentParams &Params) {
Server->semanticHighlights(
           increment(Last.resultId);
           Result.resultId = Last.resultId;
         }
 
         CB(std::move(Result));
       });
 }
 
-void ClangdLSPServer::onMemoryUsage(const NoParams &,
-                                    Callback<MemoryTree> Reply) {
+void ClangdLSPServer::onMemoryUsage(Callback<MemoryTree> Reply) {
   llvm::BumpPtrAllocator DetailAlloc;
   MemoryTree MT(&DetailAlloc);
   profile(MT);
   Reply(std::move(MT));
 }
 
 void ClangdLSPServer::onAST(const ASTParams &Params,
                             Callback<llvm::Optional<ASTNode>> CB) {

clang-tools-extra/clangd/Protocol.h

enum class TraceLevel {
   Verbose = 2,
 };
 bool fromJSON(const llvm::json::Value &E, TraceLevel &Out, llvm::json::Path);
 
 struct NoParams {};
 inline bool fromJSON(const llvm::json::Value &, NoParams &, llvm::json::Path) {
   return true;
 }
 using InitializedParams = NoParams;
-using ShutdownParams = NoParams;
-using ExitParams = NoParams;
 
 /// Defines how the host (editor) should sync document changes to the language
 /// server.
 enum class TextDocumentSyncKind {
   /// Documents should not be synced at all.
   None = 0,
 
   /// Documents are synced by always sending the full content of the document.
   Full = 1,