[lldb-dap] Make environment option an object

Author: da-viper

lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py

@@ -229,7 +229,7 @@ def test_environment(self):
         Tests launch of a simple program with environment variables
         """
         program = self.getBuildArtifact("a.out")
-        env = ["NO_VALUE", "WITH_VALUE=BAR", "EMPTY_VALUE=", "SPACE=Hello World"]
+        env = {"NO_VALUE": "", "WITH_VALUE":"BAR", "EMPTY_VALUE": "", "SPACE": "Hello World"}
         self.build_and_launch(program, env=env)
         self.continue_to_exit()
 
@@ -242,7 +242,7 @@ def test_environment(self):
             lines.pop(0)
         # Make sure each environment variable in "env" is actually set in the
         # program environment that was printed to STDOUT
-        for var in env:
+        for var in env.keys():
             found = False
             for program_var in lines:
                 if var in program_var:

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -136,6 +136,31 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
   return strs;
 }
 
+std::unordered_map<std::string, std::string>
+GetStringMap(const llvm::json::Object &obj, llvm::StringRef key) {
+  std::unordered_map<std::string, std::string> strs;
+  const auto *const json_object = obj.getObject(key);
+  if (!json_object)
+    return strs;
+
+  for (const auto &[key, value] : *json_object) {
+    switch (value.kind()) {
+    case llvm::json::Value::String:
+      strs.emplace(key.str(), value.getAsString()->str());
+      break;
+    case llvm::json::Value::Number:
+    case llvm::json::Value::Boolean:
+      strs.emplace(key.str(), llvm::to_string(value));
+      break;
+    case llvm::json::Value::Null:
+    case llvm::json::Value::Object:
+    case llvm::json::Value::Array:
+      break;
+    }
+  }
+  return strs;
+}
+
 static bool IsClassStructOrUnionType(lldb::SBType t) {
   return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct |
                               lldb::eTypeClassArray)) != 0;
@@ -1370,13 +1395,16 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
   if (!cwd.empty())
     run_in_terminal_args.try_emplace("cwd", cwd);
 
-  // We need to convert the input list of environments variables into a
-  // dictionary
-  std::vector<std::string> envs = GetStrings(launch_request_arguments, "env");
+  std::unordered_map<std::string, std::string> envMap =
+      GetStringMap(*launch_request_arguments, "env");
   llvm::json::Object environment;
-  for (const std::string &env : envs) {
-    size_t index = env.find('=');
-    environment.try_emplace(env.substr(0, index), env.substr(index + 1));
+  for (const auto &[key, value] : envMap) {
+    if (key.empty())
+      g_dap.SendOutput(OutputType::Stderr,
+                       "empty environment variable for value: \"" + value +
+                           '\"');
+    else
+      environment.try_emplace(key, value);
   }
   run_in_terminal_args.try_emplace("env",
                                    llvm::json::Value(std::move(environment)));

lldb/tools/lldb-dap/JSONUtils.h

@@ -16,6 +16,7 @@
 #include "llvm/Support/JSON.h"
 #include <cstdint>
 #include <optional>
+#include <unordered_map>
 
 namespace lldb_dap {
 
@@ -152,6 +153,27 @@ bool ObjectContainsKey(const llvm::json::Object &obj, llvm::StringRef key);
 std::vector<std::string> GetStrings(const llvm::json::Object *obj,
                                     llvm::StringRef key);
 
+/// Extract an object of key value strings for the specified key from an object.
+///
+/// String values in the object will be extracted without any quotes
+/// around them. Numbers and Booleans will be converted into
+/// strings. Any NULL, array or objects values in the array will be
+/// ignored.
+///
+/// \param[in] obj
+///     A JSON object that we will attempt to extract the array from
+///
+/// \param[in] key
+///     The key to use when extracting the value
+///
+/// \return
+///     An object of key value strings for the specified \a key, or
+///     \a fail_value if there is no key that matches or if the
+///     value is not an object or key and values in the object are not
+///     strings, numbers or booleans.
+std::unordered_map<std::string, std::string>
+GetStringMap(const llvm::json::Object &obj, llvm::StringRef key);
+
 /// Fill a response object given the request object.
 ///
 /// The \a response object will get its "type" set to "response",

lldb/tools/lldb-dap/README.md

@@ -77,7 +77,10 @@ adds `FOO=1` and `bar` to the environment:
   "name": "Debug",
   "program": "/tmp/a.out",
   "args": [ "one", "two", "three" ],
-  "env": [ "FOO=1", "BAR" ],
+  "env": {
+    "FOO": "1"
+    "BAR": ""
+  }
 }
 ```
 

lldb/tools/lldb-dap/lldb-dap.cpp

@@ -1831,7 +1831,13 @@ lldb::SBError LaunchProcess(const llvm::json::Object &request) {
     launch_info.SetArguments(MakeArgv(args).data(), true);
 
   // Pass any environment variables along that the user specified.
-  auto envs = GetStrings(arguments, "env");
+  auto envMap = GetStringMap(*arguments, "env");
+  std::vector<std::string> envs;
+  envs.reserve(envMap.size());
+  for (const auto &[key, value] : envMap) {
+    envs.emplace_back(key + '=' + value);
+  }
+
   if (!envs.empty())
     launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
 

lldb/tools/lldb-dap/package.json

@@ -160,9 +160,14 @@
                 "default": "${workspaceRoot}"
               },
               "env": {
-                "type": "array",
-                "description": "Additional environment variables to set when launching the program. This is an array of strings that contains the variable name followed by an optional '=' character and the environment variable's value.",
-                "default": []
+                "type": "object",
+                "description": "Additional environment variables to set when launching the program. E.g. `{ \"FOO\": \"1\" }`",
+                "patternProperties": {
+                  ".*": {
+                    "type": "string"
+                  }
+                },
+                "default": {}
               },
               "stopOnEntry": {
                 "type": "boolean",