[lldb-dap] Migrating 'completions' to structured types. (llvm#153317)

This migrates the CompletionHandler to structured types and adds a new
CompletionItem and CompletionItemType to the general types.

---------

Co-authored-by: Ebuka Ezike <yerimyah1@gmail.com>

Author: ashgti

lldb/tools/lldb-dap/DAP.cpp

@@ -566,6 +566,9 @@ lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
 }
 
 lldb::SBFrame DAP::GetLLDBFrame(uint64_t frame_id) {
+  if (frame_id == LLDB_DAP_INVALID_FRAME_ID)
+    return lldb::SBFrame();
+
   lldb::SBProcess process = target.GetProcess();
   // Upper 32 bits is the thread index ID
   lldb::SBThread thread =
@@ -575,8 +578,8 @@ lldb::SBFrame DAP::GetLLDBFrame(uint64_t frame_id) {
 }
 
 lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
-  const auto frame_id =
-      GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX);
+  const auto frame_id = GetInteger<uint64_t>(arguments, "frameId")
+                            .value_or(LLDB_DAP_INVALID_FRAME_ID);
   return GetLLDBFrame(frame_id);
 }
 

lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp

@@ -8,167 +8,54 @@
 
 #include "DAP.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBStringList.h"
 
-namespace lldb_dap {
+using namespace llvm;
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
 
-// "CompletionsRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Returns a list of possible completions for a given caret
-//     position and text.\nThe CompletionsRequest may only be called if the
-//     'supportsCompletionsRequest' capability exists and is true.",
-//     "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "completions" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/CompletionsArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments"  ]
-//   }]
-// },
-// "CompletionsArguments": {
-//   "type": "object",
-//   "description": "Arguments for 'completions' request.",
-//   "properties": {
-//     "frameId": {
-//       "type": "integer",
-//       "description": "Returns completions in the scope of this stack frame.
-//       If not specified, the completions are returned for the global scope."
-//     },
-//     "text": {
-//       "type": "string",
-//       "description": "One or more source lines. Typically this is the text a
-//       user has typed into the debug console before he asked for completion."
-//     },
-//     "column": {
-//       "type": "integer",
-//       "description": "The character position for which to determine the
-//       completion proposals."
-//     },
-//     "line": {
-//       "type": "integer",
-//       "description": "An optional line for which to determine the completion
-//       proposals. If missing the first line of the text is assumed."
-//     }
-//   },
-//   "required": [ "text", "column" ]
-// },
-// "CompletionsResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'completions' request.",
-//     "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "targets": {
-//             "type": "array",
-//             "items": {
-//               "$ref": "#/definitions/CompletionItem"
-//             },
-//             "description": "The possible completions for ."
-//           }
-//         },
-//         "required": [ "targets" ]
-//       }
-//     },
-//     "required": [ "body" ]
-//   }]
-// },
-// "CompletionItem": {
-//   "type": "object",
-//   "description": "CompletionItems are the suggestions returned from the
-//   CompletionsRequest.", "properties": {
-//     "label": {
-//       "type": "string",
-//       "description": "The label of this completion item. By default this is
-//       also the text that is inserted when selecting this completion."
-//     },
-//     "text": {
-//       "type": "string",
-//       "description": "If text is not falsy then it is inserted instead of the
-//       label."
-//     },
-//     "sortText": {
-//       "type": "string",
-//       "description": "A string that should be used when comparing this item
-//       with other items. When `falsy` the label is used."
-//     },
-//     "type": {
-//       "$ref": "#/definitions/CompletionItemType",
-//       "description": "The item's type. Typically the client uses this
-//       information to render the item in the UI with an icon."
-//     },
-//     "start": {
-//       "type": "integer",
-//       "description": "This value determines the location (in the
-//       CompletionsRequest's 'text' attribute) where the completion text is
-//       added.\nIf missing the text is added at the location specified by the
-//       CompletionsRequest's 'column' attribute."
-//     },
-//     "length": {
-//       "type": "integer",
-//       "description": "This value determines how many characters are
-//       overwritten by the completion text.\nIf missing the value 0 is assumed
-//       which results in the completion text being inserted."
-//     }
-//   },
-//   "required": [ "label" ]
-// },
-// "CompletionItemType": {
-//   "type": "string",
-//   "description": "Some predefined types for the CompletionItem. Please note
-//   that not all clients have specific icons for all of them.", "enum": [
-//   "method", "function", "constructor", "field", "variable", "class",
-//   "interface", "module", "property", "unit", "value", "enum", "keyword",
-//   "snippet", "text", "color", "file", "reference", "customcolor" ]
-// }
-void CompletionsRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  llvm::json::Object body;
-  const auto *arguments = request.getObject("arguments");
+namespace lldb_dap {
 
+/// Returns a list of possible completions for a given caret position and text.
+///
+/// Clients should only call this request if the corresponding capability
+/// `supportsCompletionsRequest` is true.
+Expected<CompletionsResponseBody>
+CompletionsRequestHandler::Run(const CompletionsArguments &args) const {
   // If we have a frame, try to set the context for variable completions.
-  lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
+  lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
   if (frame.IsValid()) {
     frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
     frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
 
-  std::string text = GetString(arguments, "text").value_or("").str();
-  auto original_column =
-      GetInteger<int64_t>(arguments, "column").value_or(text.size());
-  auto original_line = GetInteger<int64_t>(arguments, "line").value_or(1);
+  std::string text = args.text;
+  auto original_column = args.column;
+  auto original_line = args.line;
   auto offset = original_column - 1;
   if (original_line > 1) {
-    llvm::SmallVector<::llvm::StringRef, 2> lines;
-    llvm::StringRef(text).split(lines, '\n');
+    SmallVector<StringRef, 2> lines;
+    StringRef(text).split(lines, '\n');
     for (int i = 0; i < original_line - 1; i++) {
       offset += lines[i].size();
     }
   }
-  llvm::json::Array targets;
+
+  std::vector<CompletionItem> targets;
 
   bool had_escape_prefix =
-      llvm::StringRef(text).starts_with(dap.configuration.commandEscapePrefix);
+      StringRef(text).starts_with(dap.configuration.commandEscapePrefix);
   ReplMode completion_mode = dap.DetectReplMode(frame, text, true);
 
   // Handle the offset change introduced by stripping out the
   // `command_escape_prefix`.
   if (had_escape_prefix) {
     if (offset <
         static_cast<int64_t>(dap.configuration.commandEscapePrefix.size())) {
-      body.try_emplace("targets", std::move(targets));
-      response.try_emplace("body", std::move(body));
-      dap.SendJSON(llvm::json::Value(std::move(response)));
-      return;
+      return CompletionsResponseBody{std::move(targets)};
     }
     offset -= dap.configuration.commandEscapePrefix.size();
   }
@@ -198,27 +85,25 @@ void CompletionsRequestHandler::operator()(
       std::string match = matches.GetStringAtIndex(i);
       std::string description = descriptions.GetStringAtIndex(i);
 
-      llvm::json::Object item;
-      llvm::StringRef match_ref = match;
-      for (llvm::StringRef commit_point : {".", "->"}) {
+      CompletionItem item;
+      StringRef match_ref = match;
+      for (StringRef commit_point : {".", "->"}) {
         if (match_ref.contains(commit_point)) {
           match_ref = match_ref.rsplit(commit_point).second;
         }
       }
-      EmplaceSafeString(item, "text", match_ref);
+      item.text = match_ref;
 
       if (description.empty())
-        EmplaceSafeString(item, "label", match);
+        item.label = match;
       else
-        EmplaceSafeString(item, "label", match + " -- " + description);
+        item.label = match + " -- " + description;
 
       targets.emplace_back(std::move(item));
     }
   }
 
-  body.try_emplace("targets", std::move(targets));
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return CompletionsResponseBody{std::move(targets)};
 }
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp

@@ -23,7 +23,7 @@ llvm::Expected<protocol::DataBreakpointInfoResponseBody>
 DataBreakpointInfoRequestHandler::Run(
     const protocol::DataBreakpointInfoArguments &args) const {
   protocol::DataBreakpointInfoResponseBody response;
-  lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId.value_or(UINT64_MAX));
+  lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
   lldb::SBValue variable = dap.variables.FindVariable(
       args.variablesReference.value_or(0), args.name);
   std::string addr, size;

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

@@ -243,14 +243,17 @@ class BreakpointLocationsRequestHandler
                                  uint32_t end_line) const;
 };
 
-class CompletionsRequestHandler : public LegacyRequestHandler {
+class CompletionsRequestHandler
+    : public RequestHandler<protocol::CompletionsArguments,
+                            llvm::Expected<protocol::CompletionsResponseBody>> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "completions"; }
   FeatureSet GetSupportedFeatures() const override {
     return {protocol::eAdapterFeatureCompletionsRequest};
   }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Expected<protocol::CompletionsResponseBody>
+  Run(const protocol::CompletionsArguments &args) const override;
 };
 
 class ContinueRequestHandler

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

@@ -329,6 +329,17 @@ json::Value toJSON(const ContinueResponseBody &CRB) {
   return std::move(Body);
 }
 
+bool fromJSON(const json::Value &Params, CompletionsArguments &CA,
+              json::Path P) {
+  json::ObjectMapper O(Params, P);
+  return O && O.map("text", CA.text) && O.map("column", CA.column) &&
+         O.mapOptional("frameId", CA.frameId) && O.mapOptional("line", CA.line);
+}
+
+json::Value toJSON(const CompletionsResponseBody &CRB) {
+  return json::Object{{"targets", CRB.targets}};
+}
+
 bool fromJSON(const json::Value &Params, SetVariableArguments &SVA,
               json::Path P) {
   json::ObjectMapper O(Params, P);

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

@@ -310,6 +310,8 @@ bool fromJSON(const llvm::json::Value &, LaunchRequestArguments &,
 using LaunchResponse = VoidResponse;
 
 #define LLDB_DAP_INVALID_PORT -1
+/// An invalid 'frameId' default value.
+#define LLDB_DAP_INVALID_FRAME_ID UINT64_MAX
 
 /// lldb-dap specific attach arguments.
 struct AttachRequestArguments {
@@ -376,6 +378,35 @@ struct ContinueResponseBody {
 };
 llvm::json::Value toJSON(const ContinueResponseBody &);
 
+/// Arguments for `completions` request.
+struct CompletionsArguments {
+  /// Returns completions in the scope of this stack frame. If not specified,
+  /// the completions are returned for the global scope.
+  uint64_t frameId = LLDB_DAP_INVALID_FRAME_ID;
+
+  /// One or more source lines. Typically this is the text users have typed into
+  /// the debug console before they asked for completion.
+  std::string text;
+
+  /// The position within `text` for which to determine the completion
+  /// proposals. It is measured in UTF-16 code units and the client capability
+  /// `columnsStartAt1` determines whether it is 0- or 1-based.
+  int64_t column = 0;
+
+  /// A line for which to determine the completion proposals. If missing the
+  /// first line of the text is assumed.
+  int64_t line = 0;
+};
+bool fromJSON(const llvm::json::Value &, CompletionsArguments &,
+              llvm::json::Path);
+
+/// Response to `completions` request.
+struct CompletionsResponseBody {
+  /// The possible completions for a given caret position and text.
+  std::vector<CompletionItem> targets;
+};
+llvm::json::Value toJSON(const CompletionsResponseBody &);
+
 /// Arguments for `configurationDone` request.
 using ConfigurationDoneArguments = EmptyArguments;
 
@@ -455,7 +486,7 @@ struct ScopesArguments {
   /// Retrieve the scopes for the stack frame identified by `frameId`. The
   /// `frameId` must have been obtained in the current suspended state. See
   /// 'Lifetime of Object References' in the Overview section for details.
-  uint64_t frameId = LLDB_INVALID_FRAME_ID;
+  uint64_t frameId = LLDB_DAP_INVALID_FRAME_ID;
 };
 bool fromJSON(const llvm::json::Value &, ScopesArguments &, llvm::json::Path);
 
@@ -541,7 +572,7 @@ using StepInResponse = VoidResponse;
 /// Arguments for `stepInTargets` request.
 struct StepInTargetsArguments {
   /// The stack frame for which to retrieve the possible step-in targets.
-  uint64_t frameId = LLDB_INVALID_FRAME_ID;
+  uint64_t frameId = LLDB_DAP_INVALID_FRAME_ID;
 };
 bool fromJSON(const llvm::json::Value &, StepInTargetsArguments &,
               llvm::json::Path);
@@ -690,7 +721,7 @@ struct DataBreakpointInfoArguments {
   /// When `name` is an expression, evaluate it in the scope of this stack
   /// frame. If not specified, the expression is evaluated in the global scope.
   /// When `asAddress` is true, the `frameId` is ignored.
-  std::optional<uint64_t> frameId;
+  uint64_t frameId = LLDB_DAP_INVALID_FRAME_ID;
 
   /// If specified, a debug adapter should return information for the range of
   /// memory extending `bytes` number of bytes from the address or variable