Adds a json::Expr type to represent intermediate JSON expressions.
Summary:
This form can be created with a nice clang-format-friendly literal syntax,
and gets escaping right. It knows how to call unparse() on our Protocol types.
All the places where we pass around JSON internally now use this type.
Object properties are sorted (stored as std::map) and so serialization is
canonicalized, with optional prettyprinting (triggered by a -pretty flag).
This makes the lit tests much nicer to read and somewhat nicer to debug.
(Unfortunately the completion tests use CHECK-DAG, which only has
line-granularity, so pretty-printing is disabled there. In future we
could make completion ordering deterministic, or switch to unittests).
Compared to the current approach, it has some efficiencies like avoiding copies
of string literals used as object keys, but is probably slower overall.
I think the code/test quality benefits are worth it.
This patch doesn't attempt to do anything about JSON *parsing*.
It takes direction from the proposal in this doc[1], but is limited in scope
and visibility, for now.
I am of half a mind just to use Expr as the target of a parser, and maybe do a
little string deduplication, but not bother with clever memory allocation.
That would be simple, and fast enough for clangd...
[1] https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4/edit
+cc d0k so he can tell me not to use std::map.
Reviewers: ioeric, malaperle
Subscribers: bkramer, ilya-biryukov, mgorny, klimek
Differential Revision: https://reviews.llvm.org/D39435
llvm-svn: 317486
diff --git a/clang-tools-extra/clangd/JSONRPCDispatcher.cpp b/clang-tools-extra/clangd/JSONRPCDispatcher.cpp
index 74d1dc8..4abe7b7 100644
--- a/clang-tools-extra/clangd/JSONRPCDispatcher.cpp
+++ b/clang-tools-extra/clangd/JSONRPCDispatcher.cpp
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "JSONRPCDispatcher.h"
+#include "JSONExpr.h"
#include "ProtocolHandlers.h"
#include "Trace.h"
#include "llvm/ADT/SmallString.h"
@@ -18,17 +19,22 @@
using namespace clang;
using namespace clangd;
-void JSONOutput::writeMessage(const Twine &Message) {
- llvm::SmallString<128> Storage;
- StringRef M = Message.toStringRef(Storage);
+void JSONOutput::writeMessage(const json::Expr &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);
// Log without headers.
- Logs << "--> " << M << '\n';
+ Logs << "--> " << S << '\n';
Logs.flush();
// Emit message with header.
- Outs << "Content-Length: " << M.size() << "\r\n\r\n" << M;
+ Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S;
Outs.flush();
}
@@ -47,30 +53,38 @@
InputMirror->flush();
}
-void RequestContext::reply(const llvm::Twine &Result) {
- if (ID.empty()) {
+void RequestContext::reply(json::Expr &&Result) {
+ if (!ID) {
Out.log("Attempted to reply to a notification!\n");
return;
}
- Out.writeMessage(llvm::Twine(R"({"jsonrpc":"2.0","id":)") + ID +
- R"(,"result":)" + Result + "}");
+ Out.writeMessage(json::obj{
+ {"jsonrpc", "2.0"},
+ {"id", *ID},
+ {"result", std::move(Result)},
+ });
}
void RequestContext::replyError(int code, const llvm::StringRef &Message) {
Out.log("Error " + llvm::Twine(code) + ": " + Message + "\n");
- if (!ID.empty()) {
- Out.writeMessage(llvm::Twine(R"({"jsonrpc":"2.0","id":)") + ID +
- R"(,"error":{"code":)" + llvm::Twine(code) +
- R"(,"message":")" + llvm::yaml::escape(Message) +
- R"("}})");
+ if (ID) {
+ Out.writeMessage(json::obj{
+ {"jsonrpc", "2.0"},
+ {"id", *ID},
+ {"error", json::obj{{"code", code}, {"message", Message}}},
+ });
}
}
-void RequestContext::call(StringRef Method, StringRef Params) {
+void RequestContext::call(StringRef Method, json::Expr &&Params) {
// FIXME: Generate/Increment IDs for every request so that we can get proper
// replies once we need to.
- Out.writeMessage(llvm::Twine(R"({"jsonrpc":"2.0","id":1,"method":")" +
- Method + R"(","params":)" + Params + R"(})"));
+ Out.writeMessage(json::obj{
+ {"jsonrpc", "2.0"},
+ {"id", 1},
+ {"method", Method},
+ {"params", std::move(Params)},
+ });
}
void JSONRPCDispatcher::registerHandler(StringRef Method, Handler H) {
@@ -80,7 +94,7 @@
static void
callHandler(const llvm::StringMap<JSONRPCDispatcher::Handler> &Handlers,
- llvm::yaml::ScalarNode *Method, llvm::yaml::ScalarNode *Id,
+ llvm::yaml::ScalarNode *Method, llvm::Optional<json::Expr> ID,
llvm::yaml::MappingNode *Params,
const JSONRPCDispatcher::Handler &UnknownHandler, JSONOutput &Out) {
llvm::SmallString<64> MethodStorage;
@@ -88,7 +102,7 @@
auto I = Handlers.find(MethodStr);
auto &Handler = I != Handlers.end() ? I->second : UnknownHandler;
trace::Span Tracer(MethodStr);
- Handler(RequestContext(Out, Id ? Id->getRawValue() : ""), Params);
+ Handler(RequestContext(Out, std::move(ID)), Params);
}
bool JSONRPCDispatcher::call(StringRef Content, JSONOutput &Out) const {
@@ -106,7 +120,7 @@
llvm::yaml::ScalarNode *Version = nullptr;
llvm::yaml::ScalarNode *Method = nullptr;
llvm::yaml::MappingNode *Params = nullptr;
- llvm::yaml::ScalarNode *Id = nullptr;
+ llvm::Optional<json::Expr> ID;
for (auto &NextKeyValue : *Object) {
auto *KeyString =
dyn_cast_or_null<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
@@ -127,7 +141,18 @@
} else if (KeyValue == "method") {
Method = dyn_cast<llvm::yaml::ScalarNode>(Value);
} else if (KeyValue == "id") {
- Id = dyn_cast<llvm::yaml::ScalarNode>(Value);
+ // ID may be either a string or a number.
+ if (auto *IdNode = dyn_cast<llvm::yaml::ScalarNode>(Value)) {
+ llvm::SmallString<32> S;
+ llvm::StringRef V = IdNode->getValue(S);
+ if (IdNode->getRawValue().startswith("\"")) {
+ ID.emplace(V.str());
+ } else {
+ double D;
+ if (!V.getAsDouble(D))
+ ID.emplace(D);
+ }
+ }
} else if (KeyValue == "params") {
if (!Method)
return false;
@@ -136,7 +161,7 @@
// because it will break clients that put the id after params. A possible
// fix would be to split the parsing and execution phases.
Params = dyn_cast<llvm::yaml::MappingNode>(Value);
- callHandler(Handlers, Method, Id, Params, UnknownHandler, Out);
+ callHandler(Handlers, Method, std::move(ID), Params, UnknownHandler, Out);
return true;
} else {
return false;
@@ -147,7 +172,7 @@
// leftovers.
if (!Method)
return false;
- callHandler(Handlers, Method, Id, nullptr, UnknownHandler, Out);
+ callHandler(Handlers, Method, std::move(ID), nullptr, UnknownHandler, Out);
return true;
}