[lldb-dap] Imporve error reporting if a command's arguments fail to parse correctly.

Previously the error only contained the failed to parse JSON message, which has no additional context.

This improves the error messages and improves the consistency of handling properties in protocol structures. Updating the fields to use 'ObjectMapper.map' instead of 'ObjectMapper.mapOptional' caught that adapterID was misspelled as well.

Author: ashgti

lldb/tools/lldb-dap/Handler/RequestHandler.h

@@ -124,6 +124,7 @@ class RequestHandler : public BaseRequestHandler {
     if (request.arguments && !fromJSON(request.arguments, arguments, root)) {
       std::string parse_failure;
       llvm::raw_string_ostream OS(parse_failure);
+      OS << "invalid arguments for request '" << request.command << "': ";
       root.printErrorContext(request.arguments, OS);
 
       protocol::ErrorMessage error_message;

lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp

@@ -178,7 +178,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) {
     return false;
   }
 
-  return O.map("success", R.success) && O.mapOptional("message", R.message) &&
+  return O.map("success", R.success) && O.map("message", R.message) &&
          mapRaw(Params, "body", R.body, P);
 }
 

lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp

@@ -20,16 +20,16 @@ namespace lldb_dap::protocol {
 bool fromJSON(const llvm::json::Value &Params, CancelArguments &CA,
               llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("requestId", CA.requestId) &&
-         O.mapOptional("progressId", CA.progressId);
+  return O && O.map("requestId", CA.requestId) &&
+         O.map("progressId", CA.progressId);
 }
 
 bool fromJSON(const json::Value &Params, DisconnectArguments &DA,
               json::Path P) {
   json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("restart", DA.restart) &&
-         O.mapOptional("terminateDebuggee", DA.terminateDebuggee) &&
-         O.mapOptional("suspendDebuggee", DA.suspendDebuggee);
+  return O && O.map("restart", DA.restart) &&
+         O.map("terminateDebuggee", DA.terminateDebuggee) &&
+         O.map("suspendDebuggee", DA.suspendDebuggee);
 }
 
 bool fromJSON(const llvm::json::Value &Params, PathFormat &PF,
@@ -75,23 +75,33 @@ bool fromJSON(const llvm::json::Value &Params, InitializeRequestArguments &IRA,
 
   const json::Object *O = Params.getAsObject();
 
-  for (auto &kv : ClientFeatureByKey)
-    if (std::optional<bool> v = O->getBoolean(kv.first()); v && *v)
+  for (auto &kv : ClientFeatureByKey) {
+    const json::Value *value_ref = O->get(kv.first());
+    if (!value_ref)
+      continue;
+
+    const std::optional<bool> value = value_ref->getAsBoolean();
+    if (!value) {
+      P.field(kv.first()).report("expected bool");
+      return false;
+    }
+
+    if (*value)
       IRA.supportedFeatures.insert(kv.second);
+  }
 
-  return OM.mapOptional("adatperID", IRA.adatperID) &&
-         OM.mapOptional("clientID", IRA.clientID) &&
-         OM.mapOptional("clientName", IRA.clientName) &&
-         OM.mapOptional("locale", IRA.locale) &&
-         OM.mapOptional("linesStartAt1", IRA.linesStartAt1) &&
-         OM.mapOptional("columnsStartAt1", IRA.columnsStartAt1) &&
-         OM.mapOptional("pathFormat", IRA.pathFormat) &&
-         OM.mapOptional("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
+  return OM.map("adapterID", IRA.adapterID) &&
+         OM.map("clientID", IRA.clientID) &&
+         OM.map("clientName", IRA.clientName) && OM.map("locale", IRA.locale) &&
+         OM.map("linesStartAt1", IRA.linesStartAt1) &&
+         OM.map("columnsStartAt1", IRA.columnsStartAt1) &&
+         OM.map("pathFormat", IRA.pathFormat) &&
+         OM.map("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
 }
 
 bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
   json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("source", SA.source) &&
+  return O && O.map("source", SA.source) &&
          O.map("sourceReference", SA.sourceReference);
 }
 

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

@@ -100,7 +100,7 @@ enum PathFormat : unsigned { ePatFormatPath, ePathFormatURI };
 /// Arguments for `initialize` request.
 struct InitializeRequestArguments {
   /// The ID of the debug adapter.
-  std::string adatperID;
+  std::string adapterID;
 
   /// The ID of the client using this adapter.
   std::optional<std::string> clientID;
@@ -113,7 +113,7 @@ struct InitializeRequestArguments {
 
   /// Determines in what format paths are specified. The default is `path`,
   /// which is the native format.
-  std::optional<PathFormat> pathFormat = ePatFormatPath;
+  PathFormat pathFormat = ePatFormatPath;
 
   /// If true all line numbers are 1-based (default).
   std::optional<bool> linesStartAt1;

lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp

@@ -38,9 +38,9 @@ bool fromJSON(const json::Value &Params, PresentationHint &PH, json::Path P) {
 
 bool fromJSON(const json::Value &Params, Source &S, json::Path P) {
   json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("name", S.name) && O.mapOptional("path", S.path) &&
-         O.mapOptional("presentationHint", S.presentationHint) &&
-         O.mapOptional("sourceReference", S.sourceReference);
+  return O && O.map("name", S.name) && O.map("path", S.path) &&
+         O.map("presentationHint", S.presentationHint) &&
+         O.map("sourceReference", S.sourceReference);
 }
 
 json::Value toJSON(const ExceptionBreakpointsFilter &EBF) {