[dds] Add support for showing Maps in variablesRequests for DAP

Change-Id: I92379f4f9d300d2cdbd209bb77259acdcad7a089
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209914
Reviewed-by: Ben Konyi <bkonyi@google.com>

pkg/dds/lib/src/dap/adapters/dart.dart

@@ -1095,8 +1095,37 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
         // Sort the variables by name.
         variables.sortBy((v) => v.name);
       }
-    } else if (vmData is vm.MapAssociation) {
-      // TODO(dantup): Maps
+    } else if (data is vm.MapAssociation) {
+      final key = data.key;
+      final value = data.value;
+      if (key is vm.InstanceRef && value is vm.InstanceRef) {
+        // For a MapAssociation, we create a dummy set of variables for "key" and
+        // "value" so that each may be expanded if they are complex values.
+        variables.addAll([
+          Variable(
+            name: 'key',
+            value: await _converter.convertVmInstanceRefToDisplayString(
+              thread,
+              key,
+              allowCallingToString: evaluateToStringInDebugViews,
+            ),
+            variablesReference:
+                _converter.isSimpleKind(key.kind) ? 0 : thread.storeData(key),
+          ),
+          Variable(
+              name: 'value',
+              value: await _converter.convertVmInstanceRefToDisplayString(
+                thread,
+                value,
+                allowCallingToString: evaluateToStringInDebugViews,
+              ),
+              variablesReference: _converter.isSimpleKind(value.kind)
+                  ? 0
+                  : thread.storeData(value),
+              evaluateName:
+                  buildEvaluateName('', parentInstanceRefId: value.id)),
+        ]);
+      }
     } else if (vmData is vm.ObjRef) {
       final object =
           await _isolateManager.getObject(storedData.thread.isolate, vmData);

pkg/dds/lib/src/dap/protocol_converter.dart

@@ -168,14 +168,26 @@ class ProtocolConverter {
       return Future.wait(associations
           .sublist(start, numItems != null ? start + numItems : null)
           .mapIndexed((index, mapEntry) async {
+        final key = mapEntry.key;
+        final value = mapEntry.value;
         final callToString =
             allowCallingToString && index <= maxToStringsPerEvaluation;
-        final keyDisplay = await convertVmResponseToDisplayString(
-            thread, mapEntry.key,
+
+        final keyDisplay = await convertVmResponseToDisplayString(thread, key,
             allowCallingToString: callToString);
         final valueDisplay = await convertVmResponseToDisplayString(
-            thread, mapEntry.value,
+            thread, value,
             allowCallingToString: callToString);
+
+        // We only provide an evaluateName for the value, and only if the
+        // key is a simple value.
+        if (key is vm.InstanceRef &&
+            value is vm.InstanceRef &&
+            evaluateName != null &&
+            isSimpleKind(key.kind)) {
+          _adapter.storeEvaluateName(value, '$evaluateName[$keyDisplay]');
+        }
+
         return dap.Variable(
           name: '${start + index}',
           value: '$keyDisplay -> $valueDisplay',

pkg/dds/test/dap/integration/debug_eval_test.dart

@@ -47,7 +47,7 @@ void main(List<String> args) {
       );
 
       // Check we got a variablesReference that maps on to the fields.
-      expect(result.variablesReference, greaterThan(0));
+      expect(result.variablesReference, isPositive);
       await client.expectVariables(
         result.variablesReference,
         '''
@@ -111,7 +111,7 @@ void main(List<String> args) {
         threadExceptionExpression,
         '_Exception',
       );
-      expect(result.variablesReference, greaterThan(0));
+      expect(result.variablesReference, isPositive);
     });
 
     test(

pkg/dds/test/dap/integration/debug_test.dart

@@ -159,7 +159,7 @@ void main(List<String> args) async {
 
       // SDK sources should have a sourceReference and no path.
       expect(topFrame.source!.path, isNull);
-      expect(topFrame.source!.sourceReference, greaterThan(0));
+      expect(topFrame.source!.sourceReference, isPositive);
 
       // Source code should contain the implementation/signature of print().
       final source = await client.getValidSource(topFrame.source!);

pkg/dds/test/dap/integration/debug_variables_test.dart

@@ -225,13 +225,136 @@ void main(List<String> args) {
       );
     });
 
-    test('renders a simple map', () {
-      // TODO(dantup): Implement this (inc evaluateNames)
-    }, skip: true);
+    test('renders a simple map with keys/values', () async {
+      final client = dap.client;
+      final testFile = await dap.createTestFile(r'''
+void main(List<String> args) {
+  final myVariable = {
+    'zero': 0,
+    'one': 1,
+    'two': 2
+  };
+  print('Hello!'); // BREAKPOINT
+}
+    ''');
+      final breakpointLine = lineWith(testFile, '// BREAKPOINT');
 
-    test('renders a simple map subset', () {
-      // TODO(dantup): Implement this (inc evaluateNames)
-    }, skip: true);
+      final stop = await client.hitBreakpoint(testFile, breakpointLine);
+      final variables = await client.expectLocalVariable(
+        stop.threadId!,
+        expectedName: 'myVariable',
+        expectedDisplayString: 'Map (3 items)',
+        // For maps, we render a level of MapAssociates first, which show
+        // their index numbers. Expanding them has a Key and a Value "field"
+        // which correspond to the items.
+        expectedVariables: '''
+            0: "zero" -> 0
+            1: "one" -> 1
+            2: "two" -> 2
+        ''',
+      );
+
+      // Check one of the MapAssociation variables has the correct Key/Value
+      // inside.
+      expect(variables.variables, hasLength(3));
+      final variableOne = variables.variables[1];
+      expect(variableOne.variablesReference, isPositive);
+      await client.expectVariables(
+        variableOne.variablesReference,
+        '''
+            key: "one"
+            value: 1, eval: myVariable["one"]
+        ''',
+      );
+    });
+
+    test('renders a simple map subset', () async {
+      final client = dap.client;
+      final testFile = await dap.createTestFile(r'''
+void main(List<String> args) {
+  final myVariable = {
+    'zero': 0,
+    'one': 1,
+    'two': 2
+  };
+  print('Hello!'); // BREAKPOINT
+}
+    ''');
+      final breakpointLine = lineWith(testFile, '// BREAKPOINT');
+
+      final stop = await client.hitBreakpoint(testFile, breakpointLine);
+      await client.expectLocalVariable(
+        stop.threadId!,
+        expectedName: 'myVariable',
+        expectedDisplayString: 'Map (3 items)',
+        // For maps, we render a level of MapAssociates first, which show
+        // their index numbers. Expanding them has a Key and a Value "field"
+        // which correspond to the items.
+        expectedVariables: '''
+            1: "one" -> 1
+        ''',
+        start: 1,
+        count: 1,
+      );
+    });
+
+    test('renders a complex map with keys/values', () async {
+      final client = dap.client;
+      final testFile = await dap.createTestFile(r'''
+void main(List<String> args) {
+  final myVariable = {
+    DateTime(2000, 1, 1): Exception("my error")
+  };
+  print('Hello!'); // BREAKPOINT
+}
+    ''');
+      final breakpointLine = lineWith(testFile, '// BREAKPOINT');
+
+      final stop = await client.hitBreakpoint(testFile, breakpointLine);
+      final mapVariables = await client.expectLocalVariable(
+        stop.threadId!,
+        expectedName: 'myVariable',
+        expectedDisplayString: 'Map (1 item)',
+        expectedVariables: '''
+            0: DateTime -> _Exception
+        ''',
+      );
+
+      // Check one of the MapAssociation variables has the correct Key/Value
+      // inside.
+      expect(mapVariables.variables, hasLength(1));
+      final mapVariable = mapVariables.variables[0];
+      expect(mapVariable.variablesReference, isPositive);
+      final variables = await client.expectVariables(
+        mapVariable.variablesReference,
+        // We don't expect an evaluteName because the key is not a simple type.
+        '''
+            key: DateTime
+            value: _Exception
+        ''',
+      );
+
+      // Check the Key can be drilled into.
+      expect(variables.variables, hasLength(2));
+      final keyVariable = variables.variables[0];
+      expect(keyVariable.variablesReference, isPositive);
+      await client.expectVariables(
+        keyVariable.variablesReference,
+        '''
+            isUtc: false
+        ''',
+      );
+
+      // Check the Value can be drilled into.
+      final valueVariable = variables.variables[1];
+      expect(valueVariable.variablesReference, isPositive);
+      await client.expectVariables(
+        valueVariable.variablesReference,
+        '''
+            message: "my error"
+        ''',
+      );
+    });
     // These tests can be slow due to starting up the external server process.
   }, timeout: Timeout.none);
 }

pkg/dds/test/dap/integration/test_client.dart

@@ -605,7 +605,7 @@ extension DapTestClientExtension on DapTestClient {
 
   /// A helper that finds a named variable in the Variables scope for the top
   /// frame and asserts its child variables (fields/getters/etc) match.
-  Future<void> expectLocalVariable(
+  Future<VariablesResponseBody> expectLocalVariable(
     int threadId, {
     required String expectedName,
     required String expectedDisplayString,
@@ -632,7 +632,7 @@ extension DapTestClientExtension on DapTestClient {
     expect(expectedVariable.value, equals(expectedDisplayString));
 
     // Check the child fields.
-    await expectVariables(
+    return expectVariables(
       expectedVariable.variablesReference,
       expectedVariables,
       start: start,