From 64f0f6a13e8581e175cc5044eadf520cbba66ada Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 18 May 2017 18:57:28 -0400
Subject: [PATCH 09/31] FIXME: better json error handling; WIP on server

---
 gcc/http-server.c                |   7 +-
 gcc/http-server.h                |   5 ++
 gcc/json-rpc.c                   |  66 ++++++++++++---
 gcc/json-rpc.h                   |   2 +-
 gcc/json.c                       | 170 ++++++++++++++++++++++++++++-----------
 gcc/json.h                       |   5 +-
 gcc/lsp-main.c                   |   4 +-
 gcc/testsuite/gcc.dg/lsp/test.py |  30 +++++++
 8 files changed, 227 insertions(+), 62 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/lsp/test.py

diff --git a/gcc/http-server.c b/gcc/http-server.c
index 1e1e8c8..3f7f7cd 100644
--- a/gcc/http-server.c
+++ b/gcc/http-server.c
@@ -155,7 +155,8 @@ server::read_from_client (int filedes)
 void
 httpish_server::on_read (size_t sz, const char *buf)
 {
-  fprintf (stderr, "httpish_server::on_read: `%s'\n", buf); // FIXME: respect sz
+  if (m_verbose)
+    fprintf (stderr, "httpish_server::on_read: `%s'\n", buf); // FIXME: respect sz
   size_t line_start = 0;
   while (line_start < sz)
     {
@@ -204,7 +205,9 @@ namespace {
 class test_server : public httpish_server
 {
  public:
-  test_server () : m_user_agent (NULL), m_content (NULL) {}
+  test_server ()
+  : httpish_server (false), m_user_agent (NULL), m_content (NULL) {}
+
   ~test_server () { free (m_user_agent); free (m_content); }
 
   void on_header (size_t key_sz, const char *key,
diff --git a/gcc/http-server.h b/gcc/http-server.h
index 233f194..08d27b0 100644
--- a/gcc/http-server.h
+++ b/gcc/http-server.h
@@ -41,6 +41,8 @@ class server
 class httpish_server : public server
 {
  public:
+  httpish_server (bool verbose) : m_verbose (verbose) {}
+
   void on_read (size_t sz, const char *buf) OVERRIDE FINAL;
 
   virtual void on_header (size_t key_sz, const char *key,
@@ -49,6 +51,9 @@ class httpish_server : public server
 
  private:
   void parse_header (size_t sz, const char *buf);
+
+ private:
+  bool m_verbose;
 };
 
 #endif  /* GCC_HTTP_SERVER_H  */
diff --git a/gcc/json-rpc.c b/gcc/json-rpc.c
index c92b2bb..a9bafbd 100644
--- a/gcc/json-rpc.c
+++ b/gcc/json-rpc.c
@@ -47,9 +47,9 @@ jsonrpc::make_failure (int code, const char *message, const json::value *id)
 }
 
 json::value *
-jsonrpc::make_parse_error ()
+jsonrpc::make_parse_error (const char *msg)
 {
-  return make_failure (PARSE_ERROR, "parse error", NULL);
+  return make_failure (PARSE_ERROR, msg, NULL);
 }
 
 json::value *
@@ -111,7 +111,7 @@ jsonrpc::server::handle_request_1 (json::value *request)
 {
   // FIXME: handle batch requests
   if (request->get_kind () != json::JSON_OBJECT)
-    return make_parse_error ();
+    return make_parse_error ("not an object");
 
   json::object *reqobj = static_cast <json::object *> (request);
 
@@ -146,7 +146,16 @@ jsonrpc::server::handle_request_1 (json::value *request)
 json::value *
 jsonrpc::server::handle_request (size_t sz, const char *buf)
 {
-  json::value *request = json::parse_utf8_string (sz, buf);
+  char *err = NULL;
+  json::value *request = json::parse_utf8_string (sz, buf, &err);
+  if (!request)
+    {
+      gcc_assert (err);
+      json::value *response = make_parse_error (err);
+      free (err);
+      return response;
+    }
+  gcc_assert (err == NULL);
   json::value *response = handle_request (request);
   delete request;
   return response;
@@ -155,10 +164,7 @@ jsonrpc::server::handle_request (size_t sz, const char *buf)
 json::value *
 jsonrpc::server::handle_request_string (const char *utf8)
 {
-  json::value *request = json::parse_utf8_string (utf8);
-  json::value *response = handle_request (request);
-  delete request;
-  return response;
+  return handle_request (strlen (utf8), utf8);
 }
 
 #if CHECKING_P
@@ -184,6 +190,22 @@ assert_is_response (const json::value *response, int id)
   return obj;
 }
 
+/* Assert that RESPONSE is a non-NULL response, with no ID.
+   Verify that the jsonrpc value is "2.0".
+   Return RESPONSE, cast to an object *.  */
+
+static const json::object *
+assert_is_response (const json::value *response)
+{
+  ASSERT_TRUE (response != NULL);
+  const json::object *obj = response->as_object ();
+  ASSERT_TRUE (obj != NULL);
+  json::value *jsonrpc = obj->get ("jsonrpc");
+  ASSERT_TRUE (jsonrpc != NULL);
+  ASSERT_STREQ ("2.0", jsonrpc->as_string ()->get_string ());
+  return obj;
+}
+
 /* Assert that RESPONSE is a non-NULL successful response for ID.
    Verify that the jsonrpc value is "2.0".
    Return the "result" value.  */
@@ -211,6 +233,20 @@ assert_is_failure (const json::value *response, int code, const char *message,
   ASSERT_STREQ (message, err->get ("message")->as_string ()->get_string ());
 }
 
+/* Assert that RESPONSE is a non-NULL failure response, with no ID.
+   Verify that the jsonrpc value is "2.0".
+   Verify that CODE and MESSAGE match the given values.  */
+
+static void
+assert_is_failure (const json::value *response, int code, const char *message)
+{
+  const json::object *obj = assert_is_response (response);
+  ASSERT_EQ (NULL, obj->get ("result"));
+  const json::object *err = obj->get ("error")->as_object ();
+  ASSERT_EQ (code, err->get ("code")->as_number ()->get ());
+  ASSERT_STREQ (message, err->get ("message")->as_string ()->get_string ());
+}
+
 using namespace jsonrpc;
 
 namespace {
@@ -256,7 +292,7 @@ class test_server : public jsonrpc::server
 static void
 test_simple ()
 {
-  test_server s (false);
+  test_server s (true);
   const char *request
     = ("{\"jsonrpc\": \"2.0\", \"method\": \"subtract\","
        " \"params\": [42, 23], \"id\": 1}");
@@ -315,6 +351,17 @@ test_method_not_found ()
   delete response;
 }
 
+static void
+test_malformed_json ()
+{
+  test_server s (false);
+  const char *request = "{";
+  json::value *response = s.handle_request_string (request);
+  assert_is_failure (response, PARSE_ERROR,
+    "error at index 1: expected string for object key");
+  delete response;
+}
+
 /* Run all of the selftests within this file.  */
 
 void
@@ -325,6 +372,7 @@ json_rpc_c_tests ()
   test_bad_version ();
   test_method_not_a_string ();
   test_method_not_found ();
+  test_malformed_json ();
   // TODO: bogus JSON
 }
 
diff --git a/gcc/json-rpc.h b/gcc/json-rpc.h
index b1bdb10..4b5fd4b 100644
--- a/gcc/json-rpc.h
+++ b/gcc/json-rpc.h
@@ -52,7 +52,7 @@ const int METHOD_NOT_FOUND = -32601;
 const int INVALID_PARAMS = -32602;
 const int INTERNAL_ERROR = -32603;
 
-extern json::value *make_parse_error ();
+extern json::value *make_parse_error (const char *msg);
 
 extern json::value *make_invalid_request (const json::value *id);
 
diff --git a/gcc/json.c b/gcc/json.c
index 400a636..542b936 100644
--- a/gcc/json.c
+++ b/gcc/json.c
@@ -164,7 +164,7 @@ string::clone () const
 void
 literal::dump (FILE *outf) const
 {
-  fprintf (outf, "FIXME"); // FIXME: escaping
+  fprintf (outf, "FIXME"); // FIXME
 }
 
 value *
@@ -202,6 +202,22 @@ enum token_id
   TOK_NUMBER
 };
 
+static const char *token_id_name[] = {
+  "error",
+  "EOF",
+  "'['",
+  "'{'",
+  "']'",
+  "'}'",
+  "':'",
+  "','",
+  "'true'",
+  "'false'",
+  "'null'",
+  "string",
+  "number"
+};
+
 struct token
 {
   enum token_id id;
@@ -219,7 +235,6 @@ class lexer
  public:
   lexer ();
   void add_utf8 (size_t sz, const char *utf8_buf);
-  void add_utf8 (const char *utf8);
 
   const token *peek ();
   void consume ();
@@ -244,22 +259,21 @@ class lexer
 class parser
 {
  public:
-  parser ();
+  parser (char **err_out);
   void add_utf8 (size_t sz, const char *utf8_buf);
-  void add_utf8 (const char *utf8);
   value *parse_value ();
   object *parse_object ();
   array *parse_array ();
 
-  bool seen_error_p () const { return m_first_error; }
+  bool seen_error_p () const { return *m_err_out; }
 
  private:
   void require (enum token_id tok_id);
-  void error (const char *);
+  void error_at (int, const char *, ...);
 
  private:
   lexer m_lexer;
-  const char *m_first_error;
+  char **m_err_out;
 };
 
 } // namespace json
@@ -293,7 +307,7 @@ lexer::consume ()
   gcc_assert (m_num_next_tokens > 0);
   gcc_assert (m_num_next_tokens <= MAX_TOKENS);
 
-  if (1)
+  if (0)
     {
       fprintf (stderr, "consuming token: ");
       dump_token (stderr, &m_next_tokens[0]);
@@ -313,15 +327,7 @@ lexer::add_utf8 (size_t sz, const char *utf8_buf)
 {
   // FIXME: this assumes Latin-1.
   for (size_t i = 0; i < sz; i++)
-    m_buffer.safe_push (utf8_buf[sz]);
-}
-
-void
-lexer::add_utf8 (const char *utf8)
-{
-  // FIXME: this assumes Latin-1.
-  for (const char *ptr = utf8; *ptr; ptr++)
-    m_buffer.safe_push (*ptr);
+    m_buffer.safe_push (utf8_buf[i]);
 }
 
 bool
@@ -406,12 +412,11 @@ lexer::dump_token (FILE *outf, const token *tok)
 void
 lexer::lex_token (token *out)
 {
-  out->index = m_next_char_idx;
-
   /* Skip to next non-whitespace char.  */
   unichar next_char;
   while (1)
     {
+      out->index = m_next_char_idx;
       if (!get_char (next_char))
 	{
 	  out->id = TOK_EOF;
@@ -566,9 +571,12 @@ lexer::lex_number (token *out, unichar first_char)
   out->u.number = value;
 }
 
-parser::parser ()
-: m_lexer (), m_first_error (NULL)
+parser::parser (char **err_out)
+: m_lexer (), m_err_out (err_out)
 {
+  gcc_assert (err_out);
+  gcc_assert (*err_out == NULL);
+  *err_out = NULL;
 }
 
 void
@@ -577,12 +585,6 @@ parser::add_utf8 (size_t sz, const char *utf8_buf)
   m_lexer.add_utf8 (sz, utf8_buf);
 }
 
-void
-parser::add_utf8 (const char *utf8)
-{
-  m_lexer.add_utf8 (utf8);
-}
-
 value *
 parser::parse_value ()
 {
@@ -604,8 +606,7 @@ parser::parse_value ()
       return result;
     }
 
-  // FIXME
-  error ("error in parse_value");
+  error_at (tok->index, "unexpected token: %s", token_id_name[tok->id]);
   return NULL; // FIXME
 }
 
@@ -624,14 +625,14 @@ parser::parse_object ()
     }
   if (tok->id != TOK_STRING)
     {
-      error ("expected string for object key");
+      error_at (tok->index, "expected string for object key");
       return result;
     }
   while (!seen_error_p ())
     {
       tok = m_lexer.peek ();
       if (tok->id != TOK_STRING)
-	error ("expected string for object key");
+	error_at (tok->index, "expected string for object key");
       char *key = xstrdup (tok->u.string);
       m_lexer.consume ();
 
@@ -696,32 +697,54 @@ parser::require (enum token_id tok_id)
 {
   const token *tok = m_lexer.peek ();
   if (tok->id != tok_id)
-    error ("expected FIXME");
+    error_at (tok->index, "expected %s; got %s", token_id_name[tok_id],
+	      token_id_name[tok->id]);
   m_lexer.consume ();
 }
 
 void
-parser::error (const char *msg)
+parser::error_at (int index, const char *fmt, ...)
 {
-  m_first_error = msg;
-  fprintf (stderr, "parsing error: %s\n", msg);
+  va_list ap;
+  va_start (ap, fmt);
+  char *formatted = xvasprintf (fmt, ap);
+  va_end (ap);
+
+  char *msg_with_index = xasprintf ("error at index %i: %s",
+				    index, formatted);
+  free (formatted);
+
+  fprintf (stderr, "%s\n", msg_with_index);
+  if (*m_err_out == NULL)
+    *m_err_out = msg_with_index;
+  else
+    free (msg_with_index);
 }
 
 
 value *
-json::parse_utf8_string (size_t sz, const char *utf8_buf)
+json::parse_utf8_string (size_t sz, const char *utf8_buf,
+			 char **err_out)
 {
-  parser p;
+  gcc_assert (err_out);
+  gcc_assert (*err_out == NULL);
+
+  parser p (err_out);
   p.add_utf8 (sz, utf8_buf);
-  return p.parse_value ();
+  value *result = p.parse_value ();
+  if (p.seen_error_p ())
+    {
+      gcc_assert (*err_out);
+      delete result;
+      return NULL;
+    }
+  return result;
 }
 
 value *
-json::parse_utf8_string (const char *utf8)
+json::parse_utf8_string (const char *utf8, char **err_out)
 {
-  parser p;
-  p.add_utf8 (utf8);
-  return p.parse_value ();
+  return parse_utf8_string (strlen (utf8), utf8, err_out);
 }
 
 
@@ -734,7 +757,9 @@ namespace selftest {
 static void
 test_parse_string ()
 {
-  json::value *jv = parse_utf8_string ("\"foo\"");
+  char *err = NULL;
+  json::value *jv = parse_utf8_string ("\"foo\"", &err);
+  ASSERT_EQ (NULL, err);
   ASSERT_EQ (JSON_STRING, jv->get_kind ());
   ASSERT_STREQ ("foo", ((json::string *)jv)->get_string ());
   delete jv;
@@ -743,7 +768,9 @@ test_parse_string ()
 static void
 test_parse_number ()
 {
-  json::value *jv = parse_utf8_string ("42");
+  char *err = NULL;
+  json::value *jv = parse_utf8_string ("42", &err);
+  ASSERT_EQ (NULL, err);
   ASSERT_EQ (JSON_NUMBER, jv->get_kind ());
   ASSERT_EQ (42.0, ((json::number *)jv)->get ());
   delete jv;
@@ -752,7 +779,9 @@ test_parse_number ()
 static void
 test_parse_array ()
 {
-  json::value *jv = parse_utf8_string ("[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]");
+  char *err = NULL;
+  json::value *jv = parse_utf8_string ("[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]", &err);
+  ASSERT_EQ (NULL, err);
   ASSERT_EQ (JSON_ARRAY, jv->get_kind ());
   json::array *arr = static_cast <json::array *> (jv);
   ASSERT_EQ (10, arr->get_length ());
@@ -768,8 +797,10 @@ test_parse_array ()
 static void
 test_parse_object ()
 {
+  char *err = NULL;
   json::value *jv
-    = parse_utf8_string ("{\"foo\": \"bar\", \"baz\": [42, 43]}");
+    = parse_utf8_string ("{\"foo\": \"bar\", \"baz\": [42, 43]}", &err);
+  ASSERT_EQ (NULL, err);
   ASSERT_TRUE (jv != NULL);
   ASSERT_EQ (JSON_OBJECT, jv->get_kind ());
   json::object *jo = static_cast <json::object *> (jv);
@@ -796,14 +827,53 @@ test_parse_object ()
 }
 
 static void
+test_parse_jsonrpc ()
+{
+  char *err = NULL;
+  const char *request
+    = ("{\"jsonrpc\": \"2.0\", \"method\": \"subtract\","
+       " \"params\": [42, 23], \"id\": 1}");
+  json::value *jv = parse_utf8_string (request, &err);
+  ASSERT_EQ (NULL, err);
+  ASSERT_TRUE (jv != NULL);
+  delete jv;
+}
+
+static void
 test_parse_empty_object ()
 {
-  json::value *jv = parse_utf8_string ("{}");
+  char *err = NULL;
+  json::value *jv = parse_utf8_string ("{}", &err);
+  ASSERT_EQ (NULL, err);
   ASSERT_TRUE (jv != NULL);
   ASSERT_EQ (JSON_OBJECT, jv->get_kind ());
   delete jv;
 }
 
+static void
+test_error_empty_string ()
+{
+  char *err = NULL;
+  json::value *jv = parse_utf8_string ("", &err);
+  ASSERT_STREQ ("error at index 0: unexpected token: EOF", err);
+  ASSERT_TRUE (jv == NULL);
+  free (err);
+}
+
+static void
+test_error_missing_comma ()
+{
+  char *err = NULL;
+  /*                  01234567.  */
+  const char *json = "[0, 1 2]";
+  json::value *jv = parse_utf8_string (json, &err);
+  ASSERT_STREQ ("error at index 6: expected ']'; got number",
+		err);
+  // FIXME: unittest the lexer?
+  ASSERT_TRUE (jv == NULL);
+  free (err);
+}
+
 /* Run all of the selftests within this file.  */
 
 void
@@ -813,7 +883,13 @@ json_c_tests ()
   test_parse_number ();
   test_parse_array ();
   test_parse_object ();
+  test_parse_jsonrpc ();
   test_parse_empty_object ();
+  test_error_empty_string ();
+  test_error_missing_comma ();
+
+  /* FIXME: tests for roundtripping (noting that we don't preserve
+     object key ordering).  */
 }
 
 } // namespace selftest
diff --git a/gcc/json.h b/gcc/json.h
index 0545068..f50e14f 100644
--- a/gcc/json.h
+++ b/gcc/json.h
@@ -148,8 +148,9 @@ class literal : public value
 
 /* Parsing decls.  */
 
-extern value *parse_utf8_string (size_t sz, const char *utf8_buf);
-extern value *parse_utf8_string (const char *utf8);
+extern value *parse_utf8_string (size_t sz, const char *utf8_buf,
+				 char **err_out);
+extern value *parse_utf8_string (const char *utf8, char **err_out);
 
 } // namespace json
 
diff --git a/gcc/lsp-main.c b/gcc/lsp-main.c
index 4c2ab16..fd66683 100644
--- a/gcc/lsp-main.c
+++ b/gcc/lsp-main.c
@@ -28,7 +28,8 @@ along with GCC; see the file COPYING3.  If not see
 class jsonrpc_server : public httpish_server
 {
  public:
-  jsonrpc_server (jsonrpc::server &handler) : m_handler (handler) {}
+  jsonrpc_server (jsonrpc::server &handler)
+  : httpish_server (true), m_handler (handler) {}
 
   void on_header (size_t key_sz, const char *key,
 		  size_t value_sz, const char *value) OVERRIDE FINAL
@@ -99,5 +100,6 @@ serve_lsp (int port)
 {
   test_server ts (true);
   jsonrpc_server s (ts);
+  fprintf (stderr, "serving on port %i\n", port);
   s.serve (port);
 }
diff --git a/gcc/testsuite/gcc.dg/lsp/test.py b/gcc/testsuite/gcc.dg/lsp/test.py
new file mode 100644
index 0000000..9e36bf9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lsp/test.py
@@ -0,0 +1,30 @@
+import jsonrpc # pip install json-rpc
+
+def test():
+    print('test')
+
+import requests
+import json
+
+
+def main():
+    url = "http://localhost:4000/jsonrpc"
+    headers = {'content-type': 'application/json'}
+
+    # Example echo method
+    payload = {
+        "method": "echo",
+        "params": ["echome!"],
+        "jsonrpc": "2.0",
+        "id": 0,
+    }
+    response = requests.post(
+        url, data=json.dumps(payload), headers=headers).json()
+
+    assert response["result"] == "echome!"
+    assert response["jsonrpc"]
+    assert response["id"] == 0
+
+if __name__ == "__main__":
+    main()
+
-- 
1.8.5.3

