simark updated this revision to Diff 137582.
simark added a comment.

Update

- Add vlog method to Logger interface
- Add method name to "method not found" error message


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226

Files:
  clangd/ClangdLSPServer.cpp
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Logger.cpp
  clangd/Logger.h
  clangd/ProtocolHandlers.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/unsupported-method.test

Index: test/clangd/unsupported-method.test
===================================================================
--- test/clangd/unsupported-method.test
+++ test/clangd/unsupported-method.test
@@ -6,7 +6,7 @@
 {"jsonrpc":"2.0","id":1,"method":"textDocument/jumpInTheAirLikeYouJustDontCare","params":{}}
 #      CHECK:  "error": {
 # CHECK-NEXT:    "code": -32601,
-# CHECK-NEXT:    "message": "method not found"
+# CHECK-NEXT:    "message": "method not found (textDocument/jumpInTheAirLikeYouJustDontCare)"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "id": 1,
 # CHECK-NEXT:  "jsonrpc": "2.0"
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -76,6 +76,9 @@
                    "messages delimited by --- lines, with # comment support")),
     llvm::cl::init(JSONStreamStyle::Standard));
 
+static llvm::cl::opt<bool> Verbose("v", llvm::cl::desc("Be more verbose"),
+                                   llvm::cl::init(false));
+
 static llvm::cl::opt<bool>
     PrettyPrint("pretty", llvm::cl::desc("Pretty-print JSON output"),
                 llvm::cl::init(false));
@@ -190,9 +193,9 @@
 
   JSONOutput Out(llvm::outs(), llvm::errs(),
                  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
-                 PrettyPrint);
+                 PrettyPrint, Verbose);
 
-  clangd::LoggingSession LoggingSession(Out);
+  clangd::LoggingSession LoggingSession(Out, Verbose);
 
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -27,14 +27,15 @@
   void operator()(StringRef Method, void (ProtocolCallbacks::*Handler)(Param)) {
     // Capture pointers by value, as the lambda will outlive this object.
     auto *Callbacks = this->Callbacks;
-    Dispatcher.registerHandler(Method, [=](const json::Expr &RawParams) {
-      typename std::remove_reference<Param>::type P;
-      if (fromJSON(RawParams, P)) {
-        (Callbacks->*Handler)(P);
-      } else {
-        log("Failed to decode " + Method + " request.");
-      }
-    });
+    Dispatcher.registerHandler(
+        Method, [=](StringRef Method, const json::Expr &RawParams) {
+          typename std::remove_reference<Param>::type P;
+          if (fromJSON(RawParams, P)) {
+            (Callbacks->*Handler)(P);
+          } else {
+            log("Failed to decode " + Method + " request.");
+          }
+        });
   }
 
   JSONRPCDispatcher &Dispatcher;
Index: clangd/Logger.h
===================================================================
--- clangd/Logger.h
+++ clangd/Logger.h
@@ -15,24 +15,28 @@
 namespace clang {
 namespace clangd {
 
-/// Main logging function.
+/// Main logging functions.
 /// Logs messages to a global logger, which can be set up by LoggingSesssion.
 /// If no logger is registered, writes to llvm::errs().
 void log(const llvm::Twine &Message);
 
+/// Same as the above, but for verbose messages.
+void vlog(const llvm::Twine &Message);
+
 /// Interface to allow custom logging in clangd.
 class Logger {
 public:
   virtual ~Logger() = default;
 
-  /// Implementations of this method must be thread-safe.
+  /// Implementations of these methods must be thread-safe.
   virtual void log(const llvm::Twine &Message) = 0;
+  virtual void vlog(const llvm::Twine &Message) = 0;
 };
 
 /// Only one LoggingSession can be active at a time.
 class LoggingSession {
 public:
-  LoggingSession(clangd::Logger &Instance);
+  LoggingSession(clangd::Logger &Instance, bool Verbose);
   ~LoggingSession();
 
   LoggingSession(LoggingSession &&) = delete;
Index: clangd/Logger.cpp
===================================================================
--- clangd/Logger.cpp
+++ clangd/Logger.cpp
@@ -16,11 +16,14 @@
 
 namespace {
 Logger *L = nullptr;
+bool Verbose_ = false;
+
 } // namespace
 
-LoggingSession::LoggingSession(clangd::Logger &Instance) {
+LoggingSession::LoggingSession(clangd::Logger &Instance, bool Verbose) {
   assert(!L);
   L = &Instance;
+  Verbose_ = Verbose;
 }
 
 LoggingSession::~LoggingSession() { L = nullptr; }
@@ -35,5 +38,12 @@
   }
 }
 
+void vlog(const llvm::Twine &Message) {
+  if (L)
+    L->vlog(Message);
+  else
+    log(Message);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/JSONRPCDispatcher.h
===================================================================
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -30,14 +30,17 @@
   // JSONOutput now that we pass Context everywhere.
 public:
   JSONOutput(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs,
-             llvm::raw_ostream *InputMirror = nullptr, bool Pretty = false)
-      : Pretty(Pretty), Outs(Outs), Logs(Logs), InputMirror(InputMirror) {}
+             llvm::raw_ostream *InputMirror = nullptr, bool Pretty = false,
+             bool Verbose = false)
+      : Pretty(Pretty), Verbose(Verbose), Outs(Outs), Logs(Logs),
+        InputMirror(InputMirror) {}
 
   /// Emit a JSONRPC message.
   void writeMessage(const json::Expr &Result);
 
   /// Write a line to the logging stream.
   void log(const Twine &Message) override;
+  void vlog(const Twine &Message) override;
 
   /// Mirror \p Message into InputMirror stream. Does nothing if InputMirror is
   /// null.
@@ -47,6 +50,9 @@
   // Whether output should be pretty-printed.
   const bool Pretty;
 
+  /// Whether we should log verbosely.
+  const bool Verbose;
+
 private:
   llvm::raw_ostream &Outs;
   llvm::raw_ostream &Logs;
@@ -70,7 +76,7 @@
 class JSONRPCDispatcher {
 public:
   // A handler responds to requests for a particular method name.
-  using Handler = std::function<void(const json::Expr &)>;
+  using Handler = std::function<void(StringRef Method, const json::Expr &)>;
 
   /// Create a new JSONRPCDispatcher. UnknownHandler is called when an unknown
   /// method is received.
Index: clangd/JSONRPCDispatcher.cpp
===================================================================
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -66,7 +66,7 @@
     Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S;
     Outs.flush();
   }
-  log(llvm::Twine("--> ") + S);
+  vlog(llvm::Twine("--> ") + S);
 }
 
 void JSONOutput::log(const Twine &Message) {
@@ -77,6 +77,11 @@
   Logs.flush();
 }
 
+void JSONOutput::vlog(const Twine &Message) {
+  if (Verbose)
+    log(Message);
+}
+
 void JSONOutput::mirrorInput(const Twine &Message) {
   if (!InputMirror)
     return;
@@ -176,7 +181,7 @@
 
   // Stash a reference to the span args, so later calls can add metadata.
   WithContext WithRequestSpan(RequestSpan::stash(Tracer));
-  Handler(std::move(Params));
+  Handler(*Method, std::move(Params));
   return true;
 }
 
@@ -306,7 +311,7 @@
     if (auto JSON = ReadMessage(In, Out)) {
       if (auto Doc = json::parse(*JSON)) {
         // Log the formatted message.
-        log(llvm::formatv(Out.Pretty ? "<-- {0:2}\n" : "<-- {0}\n", *Doc));
+        vlog(llvm::formatv(Out.Pretty ? "<-- {0:2}\n" : "<-- {0}\n", *Doc));
         // Finally, execute the action for this JSON message.
         if (!Dispatcher.call(*Doc, Out))
           log("JSON dispatch failed!\n");
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -406,8 +406,11 @@
   assert(!IsDone && "Run was called before");
 
   // Set up JSONRPCDispatcher.
-  JSONRPCDispatcher Dispatcher([](const json::Expr &Params) {
-    replyError(ErrorCode::MethodNotFound, "method not found");
+  JSONRPCDispatcher Dispatcher([](StringRef Method, const json::Expr &Params) {
+    std::string Message;
+    llvm::raw_string_ostream OS(Message);
+    OS << "method not found (" << Method << ")";
+    replyError(ErrorCode::MethodNotFound, OS.str());
   });
   registerCallbackHandlers(Dispatcher, /*Callbacks=*/*this);
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to