[clang-tools-extra] r344620 - [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 16 09:48:07 PDT 2018


Author: sammccall
Date: Tue Oct 16 09:48:06 2018
New Revision: 344620

URL: http://llvm.org/viewvc/llvm-project?rev=344620&view=rev
Log:
[clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.

Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.

This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.

The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.

Reviewers: jkorous, ioeric, hokein

Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D53286

Added:
    clang-tools-extra/trunk/clangd/JSONTransport.cpp
    clang-tools-extra/trunk/clangd/Transport.h
    clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
Modified:
    clang-tools-extra/trunk/clangd/CMakeLists.txt
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
    clang-tools-extra/trunk/clangd/ClangdLSPServer.h
    clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
    clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
    clang-tools-extra/trunk/clangd/Protocol.cpp
    clang-tools-extra/trunk/clangd/Protocol.h
    clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
    clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
    clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test
    clang-tools-extra/trunk/test/clangd/completion-snippets.test
    clang-tools-extra/trunk/test/clangd/completion.test
    clang-tools-extra/trunk/test/clangd/crash-non-added-files.test
    clang-tools-extra/trunk/test/clangd/execute-command.test
    clang-tools-extra/trunk/test/clangd/input-mirror.test
    clang-tools-extra/trunk/test/clangd/signature-help.test
    clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test
    clang-tools-extra/trunk/test/clangd/trace.test
    clang-tools-extra/trunk/test/clangd/xrefs.test
    clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Oct 16 09:48:06 2018
@@ -26,6 +26,7 @@ add_clang_library(clangDaemon
   GlobalCompilationDatabase.cpp
   Headers.cpp
   JSONRPCDispatcher.cpp
+  JSONTransport.cpp
   Logger.cpp
   Protocol.cpp
   ProtocolHandlers.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Oct 16 09:48:06 2018
@@ -162,7 +162,10 @@ void ClangdLSPServer::onShutdown(Shutdow
   reply(nullptr);
 }
 
-void ClangdLSPServer::onExit(ExitParams &Params) { IsDone = true; }
+void ClangdLSPServer::onExit(ExitParams &Params) {
+  // No work to do.
+  // JSONRPCDispatcher shuts down the transport after this notification.
+}
 
 void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
   PathRef File = Params.textDocument.uri.file();
@@ -497,39 +500,41 @@ void ClangdLSPServer::onReference(Refere
                          });
 }
 
-ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
+ClangdLSPServer::ClangdLSPServer(class Transport &Transport,
                                  const clangd::CodeCompleteOptions &CCOpts,
                                  llvm::Optional<Path> CompileCommandsDir,
                                  bool ShouldUseInMemoryCDB,
                                  const ClangdServer::Options &Opts)
-    : Out(Out), CDB(ShouldUseInMemoryCDB ? CompilationDB::makeInMemory()
-                                         : CompilationDB::makeDirectoryBased(
-                                               std::move(CompileCommandsDir))),
+    : Transport(Transport),
+      CDB(ShouldUseInMemoryCDB ? CompilationDB::makeInMemory()
+                               : CompilationDB::makeDirectoryBased(
+                                     std::move(CompileCommandsDir))),
       CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
       SupportedCompletionItemKinds(defaultCompletionItemKinds()),
       Server(new ClangdServer(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this,
                               Opts)) {}
 
-bool ClangdLSPServer::run(std::FILE *In, JSONStreamStyle InputStyle) {
-  assert(!IsDone && "Run was called before");
+bool ClangdLSPServer::run() {
   assert(Server);
 
   // Set up JSONRPCDispatcher.
   JSONRPCDispatcher Dispatcher([](const json::Value &Params) {
     replyError(ErrorCode::MethodNotFound, "method not found");
+    return true;
   });
   registerCallbackHandlers(Dispatcher, /*Callbacks=*/*this);
 
   // Run the Language Server loop.
-  runLanguageServerLoop(In, Out, InputStyle, Dispatcher, IsDone);
+  bool CleanExit = true;
+  if (auto Err = Dispatcher.runLanguageServerLoop(Transport)) {
+    elog("Transport error: {0}", std::move(Err));
+    CleanExit = false;
+  }
 
-  // Make sure IsDone is set to true after this method exits to ensure assertion
-  // at the start of the method fires if it's ever executed again.
-  IsDone = true;
   // Destroy ClangdServer to ensure all worker threads finish.
   Server.reset();
 
-  return ShutdownRequestReceived;
+  return CleanExit && ShutdownRequestReceived;
 }
 
 std::vector<Fix> ClangdLSPServer::getFixes(StringRef File,
@@ -589,15 +594,11 @@ void ClangdLSPServer::onDiagnosticsReady
   }
 
   // Publish diagnostics.
-  Out.writeMessage(json::Object{
-      {"jsonrpc", "2.0"},
-      {"method", "textDocument/publishDiagnostics"},
-      {"params",
-       json::Object{
-           {"uri", URIForFile{File}},
-           {"diagnostics", std::move(DiagnosticsJSON)},
-       }},
-  });
+  Transport.notify("textDocument/publishDiagnostics",
+                   json::Object{
+                       {"uri", URIForFile{File}},
+                       {"diagnostics", std::move(DiagnosticsJSON)},
+                   });
 }
 
 void ClangdLSPServer::reparseOpenedFiles() {

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Tue Oct 16 09:48:06 2018
@@ -37,18 +37,16 @@ public:
   /// If \p CompileCommandsDir has a value, compile_commands.json will be
   /// loaded only from \p CompileCommandsDir. Otherwise, clangd will look
   /// for compile_commands.json in all parent directories of each file.
-  ClangdLSPServer(JSONOutput &Out, const clangd::CodeCompleteOptions &CCOpts,
+  ClangdLSPServer(Transport &Transport,
+                  const clangd::CodeCompleteOptions &CCOpts,
                   llvm::Optional<Path> CompileCommandsDir,
                   bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts);
 
-  /// Run LSP server loop, receiving input for it from \p In. \p In must be
-  /// opened in binary mode. Output will be written using Out variable passed to
-  /// class constructor. This method must not be executed more than once for
-  /// each instance of ClangdLSPServer.
+  /// Run LSP server loop, communicating with the Transport provided in the
+  /// constructor. This method must not be executed more than once.
   ///
-  /// \return Whether we received a 'shutdown' request before an 'exit' request.
-  bool run(std::FILE *In,
-           JSONStreamStyle InputStyle = JSONStreamStyle::Standard);
+  /// \return Whether we shut down cleanly with a 'shutdown' -> 'exit' sequence.
+  bool run();
 
 private:
   // Implement DiagnosticsConsumer.
@@ -89,16 +87,10 @@ private:
   void reparseOpenedFiles();
   void applyConfiguration(const ClangdConfigurationParamsChange &Settings);
 
-  JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.
   bool ShutdownRequestReceived = false;
 
-  /// Used to indicate that the 'exit' notification was received from the
-  /// Language Server client.
-  /// It's used to break out of the LSP parsing loop.
-  bool IsDone = false;
-
   std::mutex FixItsMutex;
   typedef std::map<clangd::Diagnostic, std::vector<Fix>, LSPDiagnosticCompare>
       DiagnosticToReplacementMap;
@@ -153,6 +145,7 @@ private:
     bool IsDirectoryBased;
   };
 
+  clangd::Transport &Transport;
   // Various ClangdServer parameters go here. It's important they're created
   // before ClangdServer.
   CompilationDB CDB;

Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp (original)
+++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Tue Oct 16 09:48:06 2018
@@ -11,6 +11,7 @@
 #include "Cancellation.h"
 #include "ProtocolHandlers.h"
 #include "Trace.h"
+#include "Transport.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -28,7 +29,7 @@ using namespace clangd;
 
 namespace {
 static Key<json::Value> RequestID;
-static Key<JSONOutput *> RequestOut;
+static Key<Transport *> CurrentTransport;
 
 // When tracing, we trace a request and attach the response in reply().
 // Because the Span isn't available, we find the current request using Context.
@@ -58,23 +59,6 @@ public:
 Key<std::unique_ptr<RequestSpan>> RequestSpan::RSKey;
 } // namespace
 
-void JSONOutput::writeMessage(const json::Value &Message) {
-  std::string S;
-  llvm::raw_string_ostream OS(S);
-  if (Pretty)
-    OS << llvm::formatv("{0:2}", Message);
-  else
-    OS << Message;
-  OS.flush();
-
-  {
-    std::lock_guard<std::mutex> Guard(StreamMutex);
-    Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S;
-    Outs.flush();
-  }
-  vlog(">>> {0}\n", S);
-}
-
 void JSONOutput::log(Logger::Level Level,
                      const llvm::formatv_object_base &Message) {
   if (Level < MinLevel)
@@ -87,14 +71,6 @@ void JSONOutput::log(Logger::Level Level
   Logs.flush();
 }
 
-void JSONOutput::mirrorInput(const Twine &Message) {
-  if (!InputMirror)
-    return;
-
-  *InputMirror << Message;
-  InputMirror->flush();
-}
-
 void clangd::reply(json::Value &&Result) {
   auto ID = getRequestId();
   if (!ID) {
@@ -104,12 +80,8 @@ void clangd::reply(json::Value &&Result)
   RequestSpan::attach([&](json::Object &Args) { Args["Reply"] = Result; });
   log("--> reply({0})", *ID);
   Context::current()
-      .getExisting(RequestOut)
-      ->writeMessage(json::Object{
-          {"jsonrpc", "2.0"},
-          {"id", *ID},
-          {"result", std::move(Result)},
-      });
+      .getExisting(CurrentTransport)
+      ->reply(std::move(*ID), std::move(Result));
 }
 
 void clangd::replyError(ErrorCode Code, const llvm::StringRef &Message) {
@@ -122,13 +94,8 @@ void clangd::replyError(ErrorCode Code,
   if (auto ID = getRequestId()) {
     log("--> reply({0}) error: {1}", *ID, Message);
     Context::current()
-        .getExisting(RequestOut)
-        ->writeMessage(json::Object{
-            {"jsonrpc", "2.0"},
-            {"id", *ID},
-            {"error", json::Object{{"code", static_cast<int>(Code)},
-                                   {"message", Message}}},
-        });
+        .getExisting(CurrentTransport)
+        ->reply(std::move(*ID), make_error<LSPError>(Message, Code));
   }
 }
 
@@ -151,22 +118,20 @@ void clangd::call(StringRef Method, json
   auto ID = 1;
   log("--> {0}({1})", Method, ID);
   Context::current()
-      .getExisting(RequestOut)
-      ->writeMessage(json::Object{
-          {"jsonrpc", "2.0"},
-          {"id", ID},
-          {"method", Method},
-          {"params", std::move(Params)},
-      });
+      .getExisting(CurrentTransport)
+      ->call(Method, std::move(Params), ID);
 }
 
 JSONRPCDispatcher::JSONRPCDispatcher(Handler UnknownHandler)
     : UnknownHandler(std::move(UnknownHandler)) {
   registerHandler("$/cancelRequest", [this](const json::Value &Params) {
     if (auto *O = Params.getAsObject())
-      if (auto *ID = O->get("id"))
-        return cancelRequest(*ID);
+      if (auto *ID = O->get("id")) {
+        cancelRequest(*ID);
+        return true;
+      }
     log("Bad cancellation request: {0}", Params);
+    return true;
   });
 }
 
@@ -175,64 +140,48 @@ void JSONRPCDispatcher::registerHandler(
   Handlers[Method] = std::move(H);
 }
 
-static void logIncomingMessage(const llvm::Optional<json::Value> &ID,
-                               llvm::Optional<StringRef> Method,
-                               const json::Object *Error) {
-  if (Method) { // incoming request
-    if (ID)     // call
-      log("<-- {0}({1})", *Method, *ID);
-    else // notification
-      log("<-- {0}", *Method);
-  } else if (ID) { // response, ID must be provided
-    if (Error)
-      log("<-- reply({0}) error: {1}", *ID,
-          Error->getString("message").getValueOr("<no message>"));
-    else
-      log("<-- reply({0})", *ID);
-  }
-}
-
-bool JSONRPCDispatcher::call(const json::Value &Message, JSONOutput &Out) {
-  // Message must be an object with "jsonrpc":"2.0".
-  auto *Object = Message.getAsObject();
-  if (!Object || Object->getString("jsonrpc") != Optional<StringRef>("2.0"))
-    return false;
-  // ID may be any JSON value. If absent, this is a notification.
-  llvm::Optional<json::Value> ID;
-  if (auto *I = Object->get("id"))
-    ID = std::move(*I);
-  auto Method = Object->getString("method");
-  logIncomingMessage(ID, Method, Object->getObject("error"));
-  if (!Method) // We only handle incoming requests, and ignore responses.
-    return false;
-  // Params should be given, use null if not.
-  json::Value Params = nullptr;
-  if (auto *P = Object->get("params"))
-    Params = std::move(*P);
-
-  auto I = Handlers.find(*Method);
+bool JSONRPCDispatcher::onCall(StringRef Method, json::Value Params,
+                               json::Value ID) {
+  log("<-- {0}({1})", Method, ID);
+  auto I = Handlers.find(Method);
   auto &Handler = I != Handlers.end() ? I->second : UnknownHandler;
 
   // Create a Context that contains request information.
-  WithContextValue WithRequestOut(RequestOut, &Out);
-  llvm::Optional<WithContextValue> WithID;
-  if (ID)
-    WithID.emplace(RequestID, *ID);
+  WithContextValue WithID(RequestID, ID);
 
   // Create a tracing Span covering the whole request lifetime.
-  trace::Span Tracer(*Method);
-  if (ID)
-    SPAN_ATTACH(Tracer, "ID", *ID);
+  trace::Span Tracer(Method);
+  SPAN_ATTACH(Tracer, "ID", ID);
   SPAN_ATTACH(Tracer, "Params", Params);
 
-  // Requests with IDs can be canceled by the client. Add cancellation context.
-  llvm::Optional<WithContext> WithCancel;
-  if (ID)
-    WithCancel.emplace(cancelableRequestContext(*ID));
+  // Calls can be canceled by the client. Add cancellation context.
+  WithContext WithCancel(cancelableRequestContext(ID));
+
+  // Stash a reference to the span args, so later calls can add metadata.
+  WithContext WithRequestSpan(RequestSpan::stash(Tracer));
+  return Handler(std::move(Params));
+}
+
+bool JSONRPCDispatcher::onNotify(StringRef Method, json::Value Params) {
+  log("<-- {0}", Method);
+  auto I = Handlers.find(Method);
+  auto &Handler = I != Handlers.end() ? I->second : UnknownHandler;
+
+  // Create a tracing Span covering the whole request lifetime.
+  trace::Span Tracer(Method);
+  SPAN_ATTACH(Tracer, "Params", Params);
 
   // Stash a reference to the span args, so later calls can add metadata.
   WithContext WithRequestSpan(RequestSpan::stash(Tracer));
-  Handler(std::move(Params));
+  return Handler(std::move(Params));
+}
+
+bool JSONRPCDispatcher::onReply(json::Value ID, Expected<json::Value> Result) {
+  // We ignore replies, just log them.
+  if (Result)
+    log("<-- reply({0})", ID);
+  else
+    log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError()));
   return true;
 }
 
@@ -266,162 +215,10 @@ void JSONRPCDispatcher::cancelRequest(co
     It->second.first(); // Invoke the canceler.
 }
 
-// Tries to read a line up to and including \n.
-// If failing, feof() or ferror() will be set.
-static bool readLine(std::FILE *In, std::string &Out) {
-  static constexpr int BufSize = 1024;
-  size_t Size = 0;
-  Out.clear();
-  for (;;) {
-    Out.resize(Size + BufSize);
-    // Handle EINTR which is sent when a debugger attaches on some platforms.
-    if (!llvm::sys::RetryAfterSignal(nullptr, ::fgets, &Out[Size], BufSize, In))
-      return false;
-    clearerr(In);
-    // If the line contained null bytes, anything after it (including \n) will
-    // be ignored. Fortunately this is not a legal header or JSON.
-    size_t Read = std::strlen(&Out[Size]);
-    if (Read > 0 && Out[Size + Read - 1] == '\n') {
-      Out.resize(Size + Read);
-      return true;
-    }
-    Size += Read;
-  }
-}
-
-// Returns None when:
-//  - ferror() or feof() are set.
-//  - Content-Length is missing or empty (protocol error)
-static llvm::Optional<std::string> readStandardMessage(std::FILE *In,
-                                                       JSONOutput &Out) {
-  // A Language Server Protocol message starts with a set of HTTP headers,
-  // delimited  by \r\n, and terminated by an empty line (\r\n).
-  unsigned long long ContentLength = 0;
-  std::string Line;
-  while (true) {
-    if (feof(In) || ferror(In) || !readLine(In, Line))
-      return llvm::None;
-
-    Out.mirrorInput(Line);
-    llvm::StringRef LineRef(Line);
-
-    // We allow comments in headers. Technically this isn't part
-    // of the LSP specification, but makes writing tests easier.
-    if (LineRef.startswith("#"))
-      continue;
-
-    // Content-Length is a mandatory header, and the only one we handle.
-    if (LineRef.consume_front("Content-Length: ")) {
-      if (ContentLength != 0) {
-        elog("Warning: Duplicate Content-Length header received. "
-             "The previous value for this message ({0}) was ignored.",
-             ContentLength);
-      }
-      llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
-      continue;
-    } else if (!LineRef.trim().empty()) {
-      // It's another header, ignore it.
-      continue;
-    } else {
-      // An empty line indicates the end of headers.
-      // Go ahead and read the JSON.
-      break;
-    }
-  }
-
-  // The fuzzer likes crashing us by sending "Content-Length: 9999999999999999"
-  if (ContentLength > 1 << 30) { // 1024M
-    elog("Refusing to read message with long Content-Length: {0}. "
-         "Expect protocol errors",
-         ContentLength);
-    return llvm::None;
-  }
-  if (ContentLength == 0) {
-    log("Warning: Missing Content-Length header, or zero-length message.");
-    return llvm::None;
-  }
-
-  std::string JSON(ContentLength, '\0');
-  for (size_t Pos = 0, Read; Pos < ContentLength; Pos += Read) {
-    // Handle EINTR which is sent when a debugger attaches on some platforms.
-    Read = llvm::sys::RetryAfterSignal(0u, ::fread, &JSON[Pos], 1,
-                                       ContentLength - Pos, In);
-    Out.mirrorInput(StringRef(&JSON[Pos], Read));
-    if (Read == 0) {
-      elog("Input was aborted. Read only {0} bytes of expected {1}.", Pos,
-           ContentLength);
-      return llvm::None;
-    }
-    clearerr(In); // If we're done, the error was transient. If we're not done,
-                  // either it was transient or we'll see it again on retry.
-    Pos += Read;
-  }
-  return std::move(JSON);
-}
-
-// For lit tests we support a simplified syntax:
-// - messages are delimited by '---' on a line by itself
-// - lines starting with # are ignored.
-// This is a testing path, so favor simplicity over performance here.
-// When returning None, feof() or ferror() will be set.
-static llvm::Optional<std::string> readDelimitedMessage(std::FILE *In,
-                                                        JSONOutput &Out) {
-  std::string JSON;
-  std::string Line;
-  while (readLine(In, Line)) {
-    auto LineRef = llvm::StringRef(Line).trim();
-    if (LineRef.startswith("#")) // comment
-      continue;
-
-    // found a delimiter
-    if (LineRef.rtrim() == "---")
-      break;
-
-    JSON += Line;
-  }
-
-  if (ferror(In)) {
-    elog("Input error while reading message!");
-    return llvm::None;
-  } else { // Including EOF
-    Out.mirrorInput(
-        llvm::formatv("Content-Length: {0}\r\n\r\n{1}", JSON.size(), JSON));
-    return std::move(JSON);
-  }
-}
-
-// The use of C-style std::FILE* IO deserves some explanation.
-// Previously, std::istream was used. When a debugger attached on MacOS, the
-// process received EINTR, the stream went bad, and clangd exited.
-// A retry-on-EINTR loop around reads solved this problem, but caused clangd to
-// sometimes hang rather than exit on other OSes. The interaction between
-// istreams and signals isn't well-specified, so it's hard to get this right.
-// The C APIs seem to be clearer in this respect.
-void clangd::runLanguageServerLoop(std::FILE *In, JSONOutput &Out,
-                                   JSONStreamStyle InputStyle,
-                                   JSONRPCDispatcher &Dispatcher,
-                                   bool &IsDone) {
-  auto &ReadMessage =
-      (InputStyle == Delimited) ? readDelimitedMessage : readStandardMessage;
-  while (!IsDone && !feof(In)) {
-    if (ferror(In)) {
-      elog("IO error: {0}", llvm::sys::StrError());
-      return;
-    }
-    if (auto JSON = ReadMessage(In, Out)) {
-      if (auto Doc = json::parse(*JSON)) {
-        // Log the formatted message.
-        vlog(Out.Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc);
-        // Finally, execute the action for this JSON message.
-        if (!Dispatcher.call(*Doc, Out))
-          elog("JSON dispatch failed!");
-      } else {
-        // Parse error. Log the raw message.
-        vlog("<<< {0}\n", *JSON);
-        elog("JSON parse error: {0}", llvm::toString(Doc.takeError()));
-      }
-    }
-  }
+llvm::Error JSONRPCDispatcher::runLanguageServerLoop(Transport &Transport) {
+  // Propagate transport to all handlers so they can reply.
+  WithContextValue WithTransport(CurrentTransport, &Transport);
+  return Transport.loop(*this);
 }
 
 const json::Value *clangd::getRequestId() {

Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h (original)
+++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h Tue Oct 16 09:48:06 2018
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "Protocol.h"
 #include "Trace.h"
+#include "Transport.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -24,37 +25,19 @@
 namespace clang {
 namespace clangd {
 
-/// Encapsulates output and logs streams and provides thread-safe access to
-/// them.
+// Logs to an output stream, such as stderr.
+// FIXME: Rename to StreamLogger or such, and move to Logger.h.
 class JSONOutput : public Logger {
-  // FIXME(ibiryukov): figure out if we can shrink the public interface of
-  // JSONOutput now that we pass Context everywhere.
 public:
-  JSONOutput(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs,
-             Logger::Level MinLevel, llvm::raw_ostream *InputMirror = nullptr,
-             bool Pretty = false)
-      : Pretty(Pretty), MinLevel(MinLevel), Outs(Outs), Logs(Logs),
-        InputMirror(InputMirror) {}
-
-  /// Emit a JSONRPC message.
-  void writeMessage(const llvm::json::Value &Result);
+  JSONOutput(llvm::raw_ostream &Logs, Logger::Level MinLevel)
+      : MinLevel(MinLevel), Logs(Logs) {}
 
   /// Write a line to the logging stream.
   void log(Level, const llvm::formatv_object_base &Message) override;
 
-  /// Mirror \p Message into InputMirror stream. Does nothing if InputMirror is
-  /// null.
-  /// Unlike other methods of JSONOutput, mirrorInput is not thread-safe.
-  void mirrorInput(const Twine &Message);
-
-  // Whether output should be pretty-printed.
-  const bool Pretty;
-
 private:
   Logger::Level MinLevel;
-  llvm::raw_ostream &Outs;
   llvm::raw_ostream &Logs;
-  llvm::raw_ostream *InputMirror;
 
   std::mutex StreamMutex;
 };
@@ -81,14 +64,15 @@ void call(llvm::StringRef Method, llvm::
 ///
 /// The `$/cancelRequest` notification is handled by the dispatcher itself.
 /// It marks the matching request as cancelled, if it's still running.
-class JSONRPCDispatcher {
+class JSONRPCDispatcher : private Transport::MessageHandler {
 public:
   /// A handler responds to requests for a particular method name.
+  /// It returns false if the server should now shut down.
   ///
   /// JSONRPCDispatcher will mark the handler's context as cancelled if a
   /// matching cancellation request is received. Handlers are encouraged to
   /// check for cancellation and fail quickly in this case.
-  using Handler = std::function<void(const llvm::json::Value &)>;
+  using Handler = std::function<bool(const llvm::json::Value &)>;
 
   /// Create a new JSONRPCDispatcher. UnknownHandler is called when an unknown
   /// method is received.
@@ -97,10 +81,22 @@ public:
   /// Registers a Handler for the specified Method.
   void registerHandler(StringRef Method, Handler H);
 
-  /// Parses a JSONRPC message and calls the Handler for it.
-  bool call(const llvm::json::Value &Message, JSONOutput &Out);
+  /// Parses input queries from LSP client (coming from \p In) and runs call
+  /// method for each query.
+  ///
+  /// Input stream(\p In) must be opened in binary mode to avoid
+  /// preliminary replacements of \r\n with \n. We use C-style FILE* for reading
+  /// as std::istream has unclear interaction with signals, which are sent by
+  /// debuggers on some OSs.
+  llvm::Error runLanguageServerLoop(Transport &);
 
 private:
+  bool onReply(llvm::json::Value ID,
+               llvm::Expected<llvm::json::Value> Result) override;
+  bool onNotify(llvm::StringRef Method, llvm::json::Value Message) override;
+  bool onCall(llvm::StringRef Method, llvm::json::Value Message,
+              llvm::json::Value ID) override;
+
   // Tracking cancellations needs a mutex: handlers may finish on a different
   // thread, and that's when we clean up entries in the map.
   mutable std::mutex RequestCancelersMutex;
@@ -113,25 +109,6 @@ private:
   Handler UnknownHandler;
 };
 
-/// Controls the way JSON-RPC messages are encoded (both input and output).
-enum JSONStreamStyle {
-  /// Encoding per the LSP specification, with mandatory Content-Length header.
-  Standard,
-  /// Messages are delimited by a '---' line. Comment lines start with #.
-  Delimited
-};
-
-/// Parses input queries from LSP client (coming from \p In) and runs call
-/// method of \p Dispatcher for each query.
-/// After handling each query checks if \p IsDone is set true and exits the loop
-/// if it is.
-/// Input stream(\p In) must be opened in binary mode to avoid preliminary
-/// replacements of \r\n with \n.
-/// We use C-style FILE* for reading as std::istream has unclear interaction
-/// with signals, which are sent by debuggers on some OSs.
-void runLanguageServerLoop(std::FILE *In, JSONOutput &Out,
-                           JSONStreamStyle InputStyle,
-                           JSONRPCDispatcher &Dispatcher, bool &IsDone);
 } // namespace clangd
 } // namespace clang
 

Added: clang-tools-extra/trunk/clangd/JSONTransport.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONTransport.cpp?rev=344620&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/JSONTransport.cpp (added)
+++ clang-tools-extra/trunk/clangd/JSONTransport.cpp Tue Oct 16 09:48:06 2018
@@ -0,0 +1,298 @@
+//===--- JSONTransport.cpp - sending and receiving LSP messages over JSON -===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "Logger.h"
+#include "Protocol.h" // For LSPError
+#include "Transport.h"
+#include "llvm/Support/Errno.h"
+
+using namespace llvm;
+namespace clang {
+namespace clangd {
+namespace {
+
+json::Object encodeError(Error E) {
+  std::string Message;
+  ErrorCode Code = ErrorCode::UnknownErrorCode;
+  if (Error Unhandled =
+          handleErrors(std::move(E), [&](const LSPError &L) -> Error {
+            Message = L.Message;
+            Code = L.Code;
+            return Error::success();
+          }))
+    Message = llvm::toString(std::move(Unhandled));
+
+  return json::Object{
+      {"message", std::move(Message)},
+      {"code", int64_t(Code)},
+  };
+}
+
+Error decodeError(const json::Object &O) {
+  std::string Msg = O.getString("message").getValueOr("Unspecified error");
+  if (auto Code = O.getInteger("code"))
+    return make_error<LSPError>(std::move(Msg), ErrorCode(*Code));
+  return make_error<StringError>(std::move(Msg), inconvertibleErrorCode());
+}
+
+class JSONTransport : public Transport {
+public:
+  JSONTransport(std::FILE *In, llvm::raw_ostream &Out,
+                llvm::raw_ostream *InMirror, bool Pretty, JSONStreamStyle Style)
+      : In(In), Out(Out), InMirror(InMirror ? *InMirror : nulls()),
+        Pretty(Pretty), Style(Style) {}
+
+  void notify(StringRef Method, json::Value Params) override {
+    sendMessage(json::Object{
+        {"jsonrpc", "2.0"},
+        {"method", Method},
+        {"params", std::move(Params)},
+    });
+  }
+  void call(StringRef Method, json::Value Params, json::Value ID) override {
+    sendMessage(json::Object{
+        {"jsonrpc", "2.0"},
+        {"id", std::move(ID)},
+        {"method", Method},
+        {"params", std::move(Params)},
+    });
+  }
+  void reply(json::Value ID, Expected<json::Value> Result) override {
+    if (Result) {
+      sendMessage(json::Object{
+          {"jsonrpc", "2.0"},
+          {"id", std::move(ID)},
+          {"result", std::move(*Result)},
+      });
+    } else {
+      sendMessage(json::Object{
+          {"jsonrpc", "2.0"},
+          {"id", std::move(ID)},
+          {"error", encodeError(Result.takeError())},
+      });
+    }
+  }
+
+  Error loop(MessageHandler &Handler) override {
+    while (!feof(In)) {
+      if (ferror(In))
+        return errorCodeToError(std::error_code(errno, std::system_category()));
+      if (auto JSON = readRawMessage()) {
+        if (auto Doc = json::parse(*JSON)) {
+          vlog(Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc);
+          if (!handleMessage(std::move(*Doc), Handler))
+            return Error::success(); // we saw the "exit" notification.
+        } else {
+          // Parse error. Log the raw message.
+          vlog("<<< {0}\n", *JSON);
+          elog("JSON parse error: {0}", llvm::toString(Doc.takeError()));
+        }
+      }
+    }
+    return errorCodeToError(std::make_error_code(std::errc::io_error));
+  }
+
+private:
+  // Dispatches incoming message to Handler onNotify/onCall/onReply.
+  bool handleMessage(llvm::json::Value Message, MessageHandler &Handler);
+  // Writes outgoing message to Out stream.
+  void sendMessage(llvm::json::Value Message) {
+    std::string S;
+    llvm::raw_string_ostream OS(S);
+    OS << llvm::formatv(Pretty ? "{0:2}" : "{0}", Message);
+    OS.flush();
+    Out << "Content-Length: " << S.size() << "\r\n\r\n" << S;
+    Out.flush();
+    vlog(">>> {0}\n", S);
+  }
+
+  // Read raw string messages from input stream.
+  llvm::Optional<std::string> readRawMessage() {
+    return Style == JSONStreamStyle::Delimited ? readDelimitedMessage()
+                                               : readStandardMessage();
+  }
+  llvm::Optional<std::string> readDelimitedMessage();
+  llvm::Optional<std::string> readStandardMessage();
+
+  std::FILE *In;
+  llvm::raw_ostream &Out;
+  llvm::raw_ostream &InMirror;
+  bool Pretty;
+  JSONStreamStyle Style;
+};
+
+bool JSONTransport::handleMessage(llvm::json::Value Message,
+                                  MessageHandler &Handler) {
+  // Message must be an object with "jsonrpc":"2.0".
+  auto *Object = Message.getAsObject();
+  if (!Object || Object->getString("jsonrpc") != Optional<StringRef>("2.0")) {
+    elog("Not a JSON-RPC 2.0 message: {0:2}", Message);
+    return false;
+  }
+  // ID may be any JSON value. If absent, this is a notification.
+  llvm::Optional<json::Value> ID;
+  if (auto *I = Object->get("id"))
+    ID = std::move(*I);
+  auto Method = Object->getString("method");
+  if (!Method) { // This is a response.
+    if (!ID) {
+      elog("No method and no response ID: {0:2}", Message);
+      return false;
+    }
+    if (auto *Err = Object->getObject("error"))
+      return Handler.onReply(std::move(*ID), decodeError(*Err));
+    // Result should be given, use null if not.
+    json::Value Result = nullptr;
+    if (auto *R = Object->get("result"))
+      Result = std::move(*R);
+    return Handler.onReply(std::move(*ID), std::move(Result));
+  }
+  // Params should be given, use null if not.
+  json::Value Params = nullptr;
+  if (auto *P = Object->get("params"))
+    Params = std::move(*P);
+
+  if (ID)
+    return Handler.onCall(*Method, std::move(Params), std::move(*ID));
+  else
+    return Handler.onNotify(*Method, std::move(Params));
+}
+
+// Tries to read a line up to and including \n.
+// If failing, feof() or ferror() will be set.
+bool readLine(std::FILE *In, std::string &Out) {
+  static constexpr int BufSize = 1024;
+  size_t Size = 0;
+  Out.clear();
+  for (;;) {
+    Out.resize(Size + BufSize);
+    // Handle EINTR which is sent when a debugger attaches on some platforms.
+    if (!llvm::sys::RetryAfterSignal(nullptr, ::fgets, &Out[Size], BufSize, In))
+      return false;
+    clearerr(In);
+    // If the line contained null bytes, anything after it (including \n) will
+    // be ignored. Fortunately this is not a legal header or JSON.
+    size_t Read = std::strlen(&Out[Size]);
+    if (Read > 0 && Out[Size + Read - 1] == '\n') {
+      Out.resize(Size + Read);
+      return true;
+    }
+    Size += Read;
+  }
+}
+
+// Returns None when:
+//  - ferror() or feof() are set.
+//  - Content-Length is missing or empty (protocol error)
+llvm::Optional<std::string> JSONTransport::readStandardMessage() {
+  // A Language Server Protocol message starts with a set of HTTP headers,
+  // delimited  by \r\n, and terminated by an empty line (\r\n).
+  unsigned long long ContentLength = 0;
+  std::string Line;
+  while (true) {
+    if (feof(In) || ferror(In) || !readLine(In, Line))
+      return llvm::None;
+    InMirror << Line;
+
+    llvm::StringRef LineRef(Line);
+
+    // We allow comments in headers. Technically this isn't part
+
+    // of the LSP specification, but makes writing tests easier.
+    if (LineRef.startswith("#"))
+      continue;
+
+    // Content-Length is a mandatory header, and the only one we handle.
+    if (LineRef.consume_front("Content-Length: ")) {
+      if (ContentLength != 0) {
+        elog("Warning: Duplicate Content-Length header received. "
+             "The previous value for this message ({0}) was ignored.",
+             ContentLength);
+      }
+      llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
+      continue;
+    } else if (!LineRef.trim().empty()) {
+      // It's another header, ignore it.
+      continue;
+    } else {
+      // An empty line indicates the end of headers.
+      // Go ahead and read the JSON.
+      break;
+    }
+  }
+
+  // The fuzzer likes crashing us by sending "Content-Length: 9999999999999999"
+  if (ContentLength > 1 << 30) { // 1024M
+    elog("Refusing to read message with long Content-Length: {0}. "
+         "Expect protocol errors",
+         ContentLength);
+    return llvm::None;
+  }
+  if (ContentLength == 0) {
+    log("Warning: Missing Content-Length header, or zero-length message.");
+    return llvm::None;
+  }
+
+  std::string JSON(ContentLength, '\0');
+  for (size_t Pos = 0, Read; Pos < ContentLength; Pos += Read) {
+    // Handle EINTR which is sent when a debugger attaches on some platforms.
+    Read = llvm::sys::RetryAfterSignal(0u, ::fread, &JSON[Pos], 1,
+                                       ContentLength - Pos, In);
+    if (Read == 0) {
+      elog("Input was aborted. Read only {0} bytes of expected {1}.", Pos,
+           ContentLength);
+      return llvm::None;
+    }
+    InMirror << StringRef(&JSON[Pos], Read);
+    clearerr(In); // If we're done, the error was transient. If we're not done,
+                  // either it was transient or we'll see it again on retry.
+    Pos += Read;
+  }
+  return std::move(JSON);
+}
+
+// For lit tests we support a simplified syntax:
+// - messages are delimited by '---' on a line by itself
+// - lines starting with # are ignored.
+// This is a testing path, so favor simplicity over performance here.
+// When returning None, feof() or ferror() will be set.
+llvm::Optional<std::string> JSONTransport::readDelimitedMessage() {
+  std::string JSON;
+  std::string Line;
+  while (readLine(In, Line)) {
+    InMirror << Line;
+    auto LineRef = llvm::StringRef(Line).trim();
+    if (LineRef.startswith("#")) // comment
+      continue;
+
+    // found a delimiter
+    if (LineRef.rtrim() == "---")
+      break;
+
+    JSON += Line;
+  }
+
+  if (ferror(In)) {
+    elog("Input error while reading message!");
+    return llvm::None;
+  }
+  return std::move(JSON); // Including at EOF
+}
+
+} // namespace
+
+std::unique_ptr<Transport> newJSONTransport(std::FILE *In,
+                                            llvm::raw_ostream &Out,
+                                            llvm::raw_ostream *InMirror,
+                                            bool Pretty,
+                                            JSONStreamStyle Style) {
+  return llvm::make_unique<JSONTransport>(In, Out, InMirror, Pretty, Style);
+}
+
+} // namespace clangd
+} // namespace clang

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Tue Oct 16 09:48:06 2018
@@ -25,6 +25,8 @@ namespace clang {
 namespace clangd {
 using namespace llvm;
 
+char LSPError::ID;
+
 URIForFile::URIForFile(std::string AbsPath) {
   assert(llvm::sys::path::is_absolute(AbsPath) && "the path is relative");
   File = std::move(AbsPath);

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Tue Oct 16 09:48:06 2018
@@ -48,6 +48,23 @@ enum class ErrorCode {
   // Defined by the protocol.
   RequestCancelled = -32800,
 };
+// Models an LSP error as an llvm::Error.
+class LSPError : public llvm::ErrorInfo<LSPError> {
+public:
+  std::string Message;
+  ErrorCode Code;
+  static char ID;
+
+  LSPError(std::string Message, ErrorCode Code)
+      : Message(std::move(Message)), Code(Code) {}
+
+  void log(llvm::raw_ostream &OS) const override {
+    OS << int(Code) << ": " << Message;
+  }
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
 
 struct URIForFile {
   URIForFile() = default;

Modified: clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp (original)
+++ clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp Tue Oct 16 09:48:06 2018
@@ -35,6 +35,7 @@ struct HandlerRegisterer {
       } else {
         elog("Failed to decode {0} request.", Method);
       }
+      return Method != "exit"; // Shut down after exit notification.
     });
   }
 

Added: clang-tools-extra/trunk/clangd/Transport.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Transport.h?rev=344620&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/Transport.h (added)
+++ clang-tools-extra/trunk/clangd/Transport.h Tue Oct 16 09:48:06 2018
@@ -0,0 +1,92 @@
+//===--- Transport.h - sending and receiving LSP messages -------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// The language server protocol is usually implemented by writing messages as
+// JSON-RPC over the stdin/stdout of a subprocess. However other communications
+// mechanisms are possible, such as XPC on mac.
+//
+// The Transport interface allows the mechanism to be replaced, and the JSONRPC
+// Transport is the standard implementation.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+
+// A transport is responsible for maintaining the connection to a client
+// application, and reading/writing structured messages to it.
+//
+// Transports have limited thread safety requirements:
+//  - messages will not be sent concurrently
+//  - messages MAY be sent while loop() is reading, or its callback is active
+class Transport {
+public:
+  virtual ~Transport() = default;
+
+  // Called by Clangd to send messages to the client.
+  virtual void notify(llvm::StringRef Method, llvm::json::Value Params) = 0;
+  virtual void call(llvm::StringRef Method, llvm::json::Value Params,
+                    llvm::json::Value ID) = 0;
+  virtual void reply(llvm::json::Value ID,
+                     llvm::Expected<llvm::json::Value> Result) = 0;
+
+  // Implemented by Clangd to handle incoming messages. (See loop() below).
+  class MessageHandler {
+  public:
+    virtual ~MessageHandler() = default;
+    // Handler returns true to keep processing messages, or false to shut down.
+    virtual bool onNotify(llvm::StringRef Method, llvm::json::Value) = 0;
+    virtual bool onCall(llvm::StringRef Method, llvm::json::Value Params,
+                        llvm::json::Value ID) = 0;
+    virtual bool onReply(llvm::json::Value ID,
+                         llvm::Expected<llvm::json::Value> Result) = 0;
+  };
+  // Called by Clangd to receive messages from the client.
+  // The transport should in turn invoke the handler to process messages.
+  // If handler returns false, the transport should immedately exit the loop.
+  // (This is used to implement the `exit` notification).
+  // Otherwise, it returns an error when the transport becomes unusable.
+  virtual llvm::Error loop(MessageHandler &) = 0;
+};
+
+// Controls the way JSON-RPC messages are encoded (both input and output).
+enum JSONStreamStyle {
+  // Encoding per the LSP specification, with mandatory Content-Length header.
+  Standard,
+  // Messages are delimited by a '---' line. Comment lines start with #.
+  Delimited
+};
+
+// Returns a Transport that speaks JSON-RPC over a pair of streams.
+// The input stream must be opened in binary mode.
+// If InMirror is set, data read will be echoed to it.
+//
+// The use of C-style std::FILE* input deserves some explanation.
+// Previously, std::istream was used. When a debugger attached on MacOS, the
+// process received EINTR, the stream went bad, and clangd exited.
+// A retry-on-EINTR loop around reads solved this problem, but caused clangd to
+// sometimes hang rather than exit on other OSes. The interaction between
+// istreams and signals isn't well-specified, so it's hard to get this right.
+// The C APIs seem to be clearer in this respect.
+std::unique_ptr<Transport>
+newJSONTransport(std::FILE *In, llvm::raw_ostream &Out,
+                 llvm::raw_ostream *InMirror, bool Pretty,
+                 JSONStreamStyle = JSONStreamStyle::Standard);
+
+} // namespace clangd
+} // namespace clang
+
+#endif

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Oct 16 09:48:06 2018
@@ -253,11 +253,8 @@ int main(int argc, char *argv[]) {
   // Use buffered stream to stderr (we still flush each log message). Unbuffered
   // stream can cause significant (non-deterministic) latency for the logger.
   llvm::errs().SetBuffered();
-  JSONOutput Out(llvm::outs(), llvm::errs(), LogLevel,
-                 InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
-                 PrettyPrint);
-
-  clangd::LoggingSession LoggingSession(Out);
+  JSONOutput Logger(llvm::errs(), LogLevel);
+  clangd::LoggingSession LoggingSession(Logger);
 
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
@@ -317,12 +314,16 @@ int main(int argc, char *argv[]) {
   CCOpts.AllScopes = AllScopesCompletion;
 
   // Initialize and run ClangdLSPServer.
+  // Change stdin to binary to not lose \r\n on windows.
+  llvm::sys::ChangeStdinToBinary();
+  auto Transport = newJSONTransport(
+      stdin, llvm::outs(),
+      InputMirrorStream ? InputMirrorStream.getPointer() : nullptr, PrettyPrint,
+      InputStyle);
   ClangdLSPServer LSPServer(
-      Out, CCOpts, CompileCommandsDirPath,
+      *Transport, CCOpts, CompileCommandsDirPath,
       /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
-  // Change stdin to binary to not lose \r\n on windows.
-  llvm::sys::ChangeStdinToBinary();
-  return LSPServer.run(stdin, InputStyle) ? 0 : NoShutdownRequestErrorCode;
+  return LSPServer.run() ? 0 : NoShutdownRequestErrorCode;
 }

Modified: clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test (original)
+++ clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test Tue Oct 16 09:48:06 2018
@@ -24,3 +24,5 @@
 # CHECK-NEXT:         "message": "MACRO is one",
 ---
 {"jsonrpc":"2.0","id":10000,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/completion-snippets.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-snippets.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/completion-snippets.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-snippets.test Tue Oct 16 09:48:06 2018
@@ -52,3 +52,5 @@
 # CHECK-NEXT:  }
 ---
 {"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/completion.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/completion.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion.test Tue Oct 16 09:48:06 2018
@@ -68,3 +68,5 @@
 # CHECK-NEXT:  ]
 ---
 {"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/crash-non-added-files.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/crash-non-added-files.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/crash-non-added-files.test (original)
+++ clang-tools-extra/trunk/test/clangd/crash-non-added-files.test Tue Oct 16 09:48:06 2018
@@ -32,3 +32,5 @@
 {"jsonrpc":"2.0","id":6,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/execute-command.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/execute-command.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/execute-command.test (original)
+++ clang-tools-extra/trunk/test/clangd/execute-command.test Tue Oct 16 09:48:06 2018
@@ -62,3 +62,5 @@
 {"jsonrpc":"2.0","id":9,"method":"workspace/executeCommand","params":{"arguments":[{"custom":"foo", "changes":{"test:///foo.c":[{"range":{"start":{"line":0,"character":32},"end":{"line":0,"character":32}},"newText":"("},{"range":{"start":{"line":0,"character":37},"end":{"line":0,"character":37}},"newText":")"}]}}],"command":"clangd.applyFix"}}
 ---
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/input-mirror.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/input-mirror.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/input-mirror.test (original)
+++ clang-tools-extra/trunk/test/clangd/input-mirror.test Tue Oct 16 09:48:06 2018
@@ -12,3 +12,6 @@ Content-Length: 172
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
+Content-Length: 33
+
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/signature-help.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/signature-help.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/signature-help.test (original)
+++ clang-tools-extra/trunk/test/clangd/signature-help.test Tue Oct 16 09:48:06 2018
@@ -23,3 +23,5 @@
 # CHECK-NEXT: }
 ---
 {"jsonrpc":"2.0","id":100000,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test (original)
+++ clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test Tue Oct 16 09:48:06 2018
@@ -35,3 +35,5 @@
 # CHECK-NEXT:}
 ---
 {"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/trace.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/trace.test (original)
+++ clang-tools-extra/trunk/test/clangd/trace.test Tue Oct 16 09:48:06 2018
@@ -21,3 +21,5 @@
 # CHECK: },
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/test/clangd/xrefs.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/xrefs.test?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/xrefs.test (original)
+++ clang-tools-extra/trunk/test/clangd/xrefs.test Tue Oct 16 09:48:06 2018
@@ -55,3 +55,5 @@
 # CHECK-NEXT: ]
 ---
 {"jsonrpc":"2.0","id":10000,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=344620&r1=344619&r2=344620&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Tue Oct 16 09:48:06 2018
@@ -27,6 +27,7 @@ add_extra_unittest(ClangdTests
   GlobalCompilationDatabaseTests.cpp
   HeadersTests.cpp
   IndexTests.cpp
+  JSONTransportTests.cpp
   QualityTests.cpp
   RIFFTests.cpp
   SerializationTests.cpp

Added: clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp?rev=344620&view=auto
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp (added)
+++ clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp Tue Oct 16 09:48:06 2018
@@ -0,0 +1,199 @@
+//===-- JSONTransportTests.cpp  -------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "Protocol.h"
+#include "Transport.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <stdio.h>
+
+using namespace llvm;
+namespace clang {
+namespace clangd {
+namespace {
+
+// No fmemopen on windows, so we can't easily run this test.
+#ifndef WIN32
+
+// Fixture takes care of managing the input/output buffers for the transport.
+class JSONTransportTest : public ::testing::Test {
+  std::string InBuf, OutBuf, MirrorBuf;
+  llvm::raw_string_ostream Out, Mirror;
+  std::unique_ptr<FILE, int (*)(FILE *)> In;
+
+protected:
+  JSONTransportTest() : Out(OutBuf), Mirror(MirrorBuf), In(nullptr, nullptr) {}
+
+  template <typename... Args>
+  std::unique_ptr<Transport> transport(std::string InData, bool Pretty,
+                                       JSONStreamStyle Style) {
+    InBuf = std::move(InData);
+    In = {fmemopen(&InBuf[0], InBuf.size(), "r"), &fclose};
+    return newJSONTransport(In.get(), Out, &Mirror, Pretty, Style);
+  }
+
+  std::string input() const { return InBuf; }
+  std::string output() { return Out.str(); }
+  std::string input_mirror() { return Mirror.str(); }
+};
+
+// Echo is a simple server running on a transport:
+//   - logs each message it gets.
+//   - when it gets a call, replies to it
+//   - when it gets a notification for method "call", makes a call on Target
+// Hangs up when it gets an exit notification.
+class Echo : public Transport::MessageHandler {
+  Transport &Target;
+  std::string LogBuf;
+  raw_string_ostream Log;
+
+public:
+  Echo(Transport &Target) : Target(Target), Log(LogBuf) {}
+
+  std::string log() { return Log.str(); }
+
+  bool onNotify(StringRef Method, json::Value Params) override {
+    Log << "Notification " << Method << ": " << Params << "\n";
+    if (Method == "call")
+      Target.call("echo call", std::move(Params), 42);
+    return Method != "exit";
+  }
+
+  bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+    Log << "Call " << Method << "(" << ID << "): " << Params << "\n";
+    if (Method == "err")
+      Target.reply(ID, make_error<LSPError>("trouble at mill", ErrorCode(88)));
+    else
+      Target.reply(ID, std::move(Params));
+    return true;
+  }
+
+  bool onReply(json::Value ID, Expected<json::Value> Params) override {
+    if (Params)
+      Log << "Reply(" << ID << "): " << *Params << "\n";
+    else
+      Log << "Reply(" << ID
+          << "): error = " << llvm::toString(Params.takeError()) << "\n";
+    return true;
+  }
+};
+
+std::string trim(StringRef S) { return S.trim().str(); }
+
+// Runs an Echo session using the standard JSON-RPC format we use in production.
+TEST_F(JSONTransportTest, StandardDense) {
+  auto T = transport(
+      "Content-Length: 52\r\n\r\n"
+      R"({"jsonrpc": "2.0", "method": "call", "params": 1234})"
+      "Content-Length: 46\r\n\r\n"
+      R"({"jsonrpc": "2.0", "id": 1234, "result": 5678})"
+      "Content-Length: 67\r\n\r\n"
+      R"({"jsonrpc": "2.0", "method": "foo", "id": "abcd", "params": "efgh"})"
+      "Content-Length: 73\r\n\r\n"
+      R"({"jsonrpc": "2.0", "id": "xyz", "error": {"code": 99, "message": "bad!"}})"
+      "Content-Length: 68\r\n\r\n"
+      R"({"jsonrpc": "2.0", "method": "err", "id": "wxyz", "params": "boom!"})"
+      "Content-Length: 36\r\n\r\n"
+      R"({"jsonrpc": "2.0", "method": "exit"})",
+      /*Pretty=*/false, JSONStreamStyle::Standard);
+  Echo E(*T);
+  auto Err = T->loop(E);
+  EXPECT_FALSE(bool(Err)) << llvm::toString(std::move(Err));
+
+  EXPECT_EQ(trim(E.log()), trim(R"(
+Notification call: 1234
+Reply(1234): 5678
+Call foo("abcd"): "efgh"
+Reply("xyz"): error = 99: bad!
+Call err("wxyz"): "boom!"
+Notification exit: null
+  )"));
+  EXPECT_EQ(
+      trim(output()),
+      "Content-Length: 60\r\n\r\n"
+      R"({"id":42,"jsonrpc":"2.0","method":"echo call","params":1234})"
+      "Content-Length: 45\r\n\r\n"
+      R"({"id":"abcd","jsonrpc":"2.0","result":"efgh"})"
+      "Content-Length: 77\r\n\r\n"
+      R"({"error":{"code":88,"message":"trouble at mill"},"id":"wxyz","jsonrpc":"2.0"})");
+  EXPECT_EQ(trim(input_mirror()), trim(input()));
+}
+
+// Runs an Echo session using the "delimited" input and pretty-printed output
+// that we use in lit tests.
+TEST_F(JSONTransportTest, DelimitedPretty) {
+  auto T = transport(R"jsonrpc(
+{"jsonrpc": "2.0", "method": "call", "params": 1234}
+---
+{"jsonrpc": "2.0", "id": 1234, "result": 5678}
+---
+{"jsonrpc": "2.0", "method": "foo", "id": "abcd", "params": "efgh"}
+---
+{"jsonrpc": "2.0", "id": "xyz", "error": {"code": 99, "message": "bad!"}}
+---
+{"jsonrpc": "2.0", "method": "err", "id": "wxyz", "params": "boom!"}
+---
+{"jsonrpc": "2.0", "method": "exit"}
+  )jsonrpc",
+                     /*Pretty=*/true, JSONStreamStyle::Delimited);
+  Echo E(*T);
+  auto Err = T->loop(E);
+  EXPECT_FALSE(bool(Err)) << llvm::toString(std::move(Err));
+
+  EXPECT_EQ(trim(E.log()), trim(R"(
+Notification call: 1234
+Reply(1234): 5678
+Call foo("abcd"): "efgh"
+Reply("xyz"): error = 99: bad!
+Call err("wxyz"): "boom!"
+Notification exit: null
+  )"));
+  EXPECT_EQ(trim(output()), "Content-Length: 77\r\n\r\n"
+                            R"({
+  "id": 42,
+  "jsonrpc": "2.0",
+  "method": "echo call",
+  "params": 1234
+})"
+                            "Content-Length: 58\r\n\r\n"
+                            R"({
+  "id": "abcd",
+  "jsonrpc": "2.0",
+  "result": "efgh"
+})"
+                            "Content-Length: 105\r\n\r\n"
+                            R"({
+  "error": {
+    "code": 88,
+    "message": "trouble at mill"
+  },
+  "id": "wxyz",
+  "jsonrpc": "2.0"
+})");
+  EXPECT_EQ(trim(input_mirror()), trim(input()));
+}
+
+// IO errors such as EOF ane reported.
+// The only successful return from loop() is if a handler returned false.
+TEST_F(JSONTransportTest, EndOfFile) {
+  auto T = transport("Content-Length: 52\r\n\r\n"
+                     R"({"jsonrpc": "2.0", "method": "call", "params": 1234})",
+                     /*Pretty=*/false, JSONStreamStyle::Standard);
+  Echo E(*T);
+  auto Err = T->loop(E);
+  EXPECT_EQ(trim(E.log()), "Notification call: 1234");
+  EXPECT_TRUE(bool(Err)); // Ran into EOF with no handler signalling done.
+  consumeError(std::move(Err));
+  EXPECT_EQ(trim(input_mirror()), trim(input()));
+}
+
+#endif
+
+} // namespace
+} // namespace clangd
+} // namespace clang




More information about the cfe-commits mailing list