From e02cf3f1906dda51e5cdf8ae38b3eb46176a90f1 Mon Sep 17 00:00:00 2001
From: Ben McMorran <bemcmorr@microsoft.com>
Date: Tue, 11 Jul 2023 18:15:30 -0700
Subject: Debugger: Correctly handle clients without supportsVariableType

Fixes: #25057
---
 Source/cmDebuggerVariables.cxx                 | 39 +++++++++++++-------------
 Tests/CMakeLib/testDebugger.h                  | 12 +++++---
 Tests/CMakeLib/testDebuggerVariables.cxx       | 29 +++++++++++++++++++
 Tests/CMakeLib/testDebuggerVariablesHelper.cxx |  1 +
 4 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/Source/cmDebuggerVariables.cxx b/Source/cmDebuggerVariables.cxx
index 40fe41f..8acce34 100644
--- a/Source/cmDebuggerVariables.cxx
+++ b/Source/cmDebuggerVariables.cxx
@@ -78,15 +78,16 @@ dap::array<dap::Variable> cmDebuggerVariables::HandleVariablesRequest()
           entry.Value.empty()) {
         continue;
       }
-      variables.push_back(dap::Variable{ {},
-                                         {},
-                                         {},
-                                         entry.Name,
-                                         {},
-                                         PrivateDataHint,
-                                         entry.Type,
-                                         entry.Value,
-                                         0 });
+      variables.push_back(dap::Variable{
+        {},
+        {},
+        {},
+        entry.Name,
+        {},
+        PrivateDataHint,
+        SupportsVariableType ? entry.Type : dap::optional<dap::string>(),
+        entry.Value,
+        0 });
     }
   }
 
@@ -106,16 +107,16 @@ void cmDebuggerVariables::EnumerateSubVariablesIfAny(
 {
   dap::array<dap::Variable> ret;
   for (auto const& variables : SubVariables) {
-    toBeReturned.emplace_back(
-      dap::Variable{ {},
-                     {},
-                     {},
-                     variables->GetName(),
-                     {},
-                     PrivatePropertyHint,
-                     SupportsVariableType ? "collection" : nullptr,
-                     variables->GetValue(),
-                     variables->GetId() });
+    toBeReturned.emplace_back(dap::Variable{
+      {},
+      {},
+      {},
+      variables->GetName(),
+      {},
+      PrivatePropertyHint,
+      SupportsVariableType ? "collection" : dap::optional<dap::string>(),
+      variables->GetValue(),
+      variables->GetId() });
   }
 }
 
diff --git a/Tests/CMakeLib/testDebugger.h b/Tests/CMakeLib/testDebugger.h
index 8ba21f6..d8d2caa 100644
--- a/Tests/CMakeLib/testDebugger.h
+++ b/Tests/CMakeLib/testDebugger.h
@@ -19,11 +19,15 @@
   do {                                                                        \
     ASSERT_TRUE(x.name == expectedName);                                      \
     ASSERT_TRUE(x.value == expectedValue);                                    \
-    ASSERT_TRUE(x.type.value() == expectedType);                              \
-    ASSERT_TRUE(x.evaluateName.has_value() == false);                         \
-    if (std::string(expectedType) == "collection") {                          \
-      ASSERT_TRUE(x.variablesReference != 0);                                 \
+    if (expectedType == nullptr) {                                            \
+      ASSERT_TRUE(x.type == dap::optional<dap::string>());                    \
+    } else {                                                                  \
+      ASSERT_TRUE(x.type == dap::optional<dap::string>(expectedType));        \
+      if (std::string(expectedType) == "collection") {                        \
+        ASSERT_TRUE(x.variablesReference != 0);                               \
+      }                                                                       \
     }                                                                         \
+    ASSERT_TRUE(x.evaluateName.has_value() == false);                         \
   } while (false)
 
 #define ASSERT_VARIABLE_REFERENCE(x, expectedName, expectedValue,             \
diff --git a/Tests/CMakeLib/testDebuggerVariables.cxx b/Tests/CMakeLib/testDebuggerVariables.cxx
index 6c19baa..0d8d18d 100644
--- a/Tests/CMakeLib/testDebuggerVariables.cxx
+++ b/Tests/CMakeLib/testDebuggerVariables.cxx
@@ -8,6 +8,7 @@
 #include <unordered_set>
 #include <vector>
 
+#include <cm3p/cppdap/optional.h>
 #include <cm3p/cppdap/protocol.h>
 #include <cm3p/cppdap/types.h>
 
@@ -174,6 +175,33 @@ static bool testSortTheResult()
   return true;
 }
 
+static bool testNoSupportsVariableType()
+{
+  auto variablesManager =
+    std::make_shared<cmDebugger::cmDebuggerVariablesManager>();
+
+  auto vars = std::make_shared<cmDebugger::cmDebuggerVariables>(
+    variablesManager, "Variables", false, []() {
+      return std::vector<cmDebugger::cmDebuggerVariableEntry>{ { "test",
+                                                                 "value" } };
+    });
+
+  auto subvars = std::make_shared<cmDebugger::cmDebuggerVariables>(
+    variablesManager, "Children", false);
+
+  vars->AddSubVariables(subvars);
+
+  dap::array<dap::Variable> variables =
+    variablesManager->HandleVariablesRequest(
+      CreateVariablesRequest(vars->GetId()));
+
+  ASSERT_TRUE(variables.size() == 2);
+  ASSERT_VARIABLE(variables[0], "Children", "", nullptr);
+  ASSERT_VARIABLE(variables[1], "test", "value", nullptr);
+
+  return true;
+}
+
 int testDebuggerVariables(int, char*[])
 {
   return runTests(std::vector<std::function<bool()>>{
@@ -181,5 +209,6 @@ int testDebuggerVariables(int, char*[])
     testConstructors,
     testIgnoreEmptyStringEntries,
     testSortTheResult,
+    testNoSupportsVariableType,
   });
 }
diff --git a/Tests/CMakeLib/testDebuggerVariablesHelper.cxx b/Tests/CMakeLib/testDebuggerVariablesHelper.cxx
index e0bbdf0..d61b73b 100644
--- a/Tests/CMakeLib/testDebuggerVariablesHelper.cxx
+++ b/Tests/CMakeLib/testDebuggerVariablesHelper.cxx
@@ -8,6 +8,7 @@
 #include <utility>
 #include <vector>
 
+#include <cm3p/cppdap/optional.h>
 #include <cm3p/cppdap/protocol.h>
 #include <cm3p/cppdap/types.h>
 #include <stddef.h>
-- 
cgit v0.12