[dds/dap] Tidy up variables formatting and allow "format specifiers" in evaluation expressions

This adds some new classes to support different formatting of values in expression evaluation/watch/variables panes.

Some existing code that passed a flag around to suppress quotes has been converted to this use, and in addition, we support some C#-inspired "format specifiers" that allow you to influence the formatting of returned values by adding a suffix to a watch/evaluation expression:

https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2010/e514eeby(v=vs.100)?redirectedfrom=MSDN

So if you add "foo" and "foo,nq" to your watch window, the latter will format the string without quotes. If you add "a" and "a,h" then the latter will format a in hex.

Format specifiers are carried down, so you can add ",h" to a class or list variable, and each value down the tree will be formatted that way.

Partly fixes Dart-Code/Dart-Code#3940.

Change-Id: I78001bd046ee2586bd828752f9ea08da01e7180c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279961
Commit-Queue: Ben Konyi <[email protected]>
Reviewed-by: Ben Konyi <[email protected]>

Author: DanTup

pkg/dds/CHANGELOG.md

@@ -1,6 +1,7 @@
 # 2.7.3
 - [DAP] Added support for displaying records in responses to `variablesRequest`.
 - A new exception `ExistingDartDevelopmentServiceException` (extending `DartDevelopmentServiceException`) is thrown when trying to connect DDS to a VM Service that already has a DDS instance. This new exception contains a `ddsUri` field that is populated with the URI of the existing DDS instance if provided by the target VM Service.
+- [DAP] Added support for `,d` (decimal), `,h` (hex) and `,nq` (no quotes) format specifiers to be used as suffixes to evaluation requests.
 
 # 2.7.2
 - Update DDS protocol version to 1.4.

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

@@ -24,6 +24,7 @@ import '../protocol_converter.dart';
 import '../protocol_generated.dart';
 import '../protocol_stream.dart';
 import '../utils.dart';
+import '../variables.dart';
 import 'mixins.dart';
 
 /// The mime type to send with source responses to the client.
@@ -915,18 +916,24 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
       throw UnimplementedError('Global evaluation not currently supported');
     }
 
+    // Parse the expression for trailing format specifiers.
+    final expressionData = EvaluationExpression.parse(
+      args.expression
+          .trim()
+          // Remove any trailing semicolon as the VM only evaluates expressions
+          // but a user may have highlighted a whole line/statement to send for
+          // evaluation.
+          .replaceFirst(_trailingSemicolonPattern, ''),
+    );
+    final expression = expressionData.expression;
+    final format = expressionData.format;
+
+    final exceptionReference = thread.exceptionReference;
     // The value in the constant `frameExceptionExpression` is used as a special
     // expression that evaluates to the exception on the current thread. This
     // allows us to construct evaluateNames that evaluate to the fields down the
     // tree to support some of the debugger functionality (for example
     // "Copy Value", which re-evaluates).
-    final expression = args.expression
-        .trim()
-        // Remove any trailing semicolon as the VM only evaluates expressions
-        // but a user may have highlighted a whole line/statement to send for
-        // evaluation.
-        .replaceFirst(_trailingSemicolonPattern, '');
-    final exceptionReference = thread.exceptionReference;
     final isExceptionExpression = expression == threadExceptionExpression ||
         expression.startsWith('$threadExceptionExpression.');
 
@@ -974,10 +981,12 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
         thread,
         result,
         allowCallingToString: evaluateToStringInDebugViews,
+        format: format,
       );
 
-      final variablesReference =
-          _converter.isSimpleKind(result.kind) ? 0 : thread.storeData(result);
+      final variablesReference = _converter.isSimpleKind(result.kind)
+          ? 0
+          : thread.storeData(VariableData(result, format));
 
       // Store the expression that gets this object as we may need it to
       // compute evaluateNames for child objects later.
@@ -1662,12 +1671,19 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
       throw StateError('variablesReference is no longer valid');
     }
     final thread = storedData.thread;
-    final data = storedData.data;
-    final vmData = data is vm.Response ? data : null;
+    var data = storedData.data;
+
+    VariableFormat? format;
+    // Unwrap any variable we stored with formatting info.
+    if (data is VariableData) {
+      format = data.format;
+      data = data.data;
+    }
+
     final variables = <Variable>[];
 
-    if (vmData is vm.Frame) {
-      final vars = vmData.vars;
+    if (data is vm.Frame) {
+      final vars = data.vars;
       if (vars != null) {
         Future<Variable> convert(int index, vm.BoundVariable variable) {
           // Store the expression that gets this object as we may need it to
@@ -1680,6 +1696,7 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
             allowCallingToString: evaluateToStringInDebugViews &&
                 index <= maxToStringsPerEvaluation,
             evaluateName: variable.name,
+            format: format,
           );
         }
 
@@ -1701,28 +1718,31 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
               thread,
               key,
               allowCallingToString: evaluateToStringInDebugViews,
+              format: format,
             ),
-            variablesReference:
-                _converter.isSimpleKind(key.kind) ? 0 : thread.storeData(key),
+            variablesReference: _converter.isSimpleKind(key.kind)
+                ? 0
+                : thread.storeData(VariableData(key, format)),
           ),
           Variable(
               name: 'value',
               value: await _converter.convertVmInstanceRefToDisplayString(
                 thread,
                 value,
                 allowCallingToString: evaluateToStringInDebugViews,
+                format: format,
               ),
               variablesReference: _converter.isSimpleKind(value.kind)
                   ? 0
-                  : thread.storeData(value),
+                  : thread.storeData(VariableData(value, format)),
               evaluateName:
                   buildEvaluateName('', parentInstanceRefId: value.id)),
         ]);
       }
-    } else if (vmData is vm.ObjRef) {
+    } else if (data is vm.ObjRef) {
       final object = await _isolateManager.getObject(
         storedData.thread.isolate,
-        vmData,
+        data,
         offset: childStart,
         count: childCount,
       );
@@ -1737,10 +1757,11 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
         variables.addAll(await _converter.convertVmInstanceToVariablesList(
           thread,
           object,
-          evaluateName: buildEvaluateName('', parentInstanceRefId: vmData.id),
+          evaluateName: buildEvaluateName('', parentInstanceRefId: data.id),
           allowCallingToString: evaluateToStringInDebugViews,
           startItem: childStart,
           numItems: childCount,
+          format: format,
         ));
       } else {
         variables.add(Variable(
@@ -1976,9 +1997,9 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
     }
   }
 
-  /// Helper to convert to InstanceRef to a complete untruncated String,
-  /// handling [vm.InstanceKind.kNull] which is the type for the unused fields
-  /// of a log event.
+  /// Helper to convert to InstanceRef to a complete untruncated unquoted
+  /// String, handling [vm.InstanceKind.kNull] which is the type for the unused
+  /// fields of a log event.
   Future<String?> getFullString(ThreadInfo thread, vm.InstanceRef? ref) async {
     if (ref == null || ref.kind == vm.InstanceKind.kNull) {
       return null;
@@ -1992,7 +2013,7 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
       // setting.
       allowCallingToString: true,
       allowTruncatedValue: false,
-      includeQuotesAroundString: false,
+      format: VariableFormat.noQuotes(),
     )
         // Fetching strings from the server may throw if they have been
         // collected since (for example if a Hot Restart occurs while

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

@@ -10,9 +10,11 @@ import 'package:collection/collection.dart';
 import 'package:path/path.dart' as path;
 import 'package:vm_service/vm_service.dart' as vm;
 
+import '../../dap.dart';
 import 'adapters/dart.dart';
 import 'isolate_manager.dart';
 import 'protocol_generated.dart' as dap;
+import 'variables.dart';
 
 /// A helper that handlers converting to/from DAP and VM Service types and to
 /// user-friendly display strings.
@@ -57,43 +59,42 @@ class ProtocolConverter {
     vm.InstanceRef ref, {
     required bool allowCallingToString,
     bool allowTruncatedValue = true,
-    bool includeQuotesAroundString = true,
+    VariableFormat? format,
   }) async {
     final isTruncated = ref.valueAsStringIsTruncated ?? false;
+    final valueAsString = ref.valueAsString;
+    final formatter = format ?? const VariableFormat();
     if (ref.kind == vm.InstanceKind.kString && isTruncated) {
       // Call toString() if allowed (and we don't already have a value),
       // otherwise (or if it returns null) fall back to the truncated value
       // with "…" suffix.
       var stringValue = allowCallingToString &&
-              (ref.valueAsString == null || !allowTruncatedValue)
-          ? await _callToString(
-              thread,
-              ref,
+              (valueAsString == null || !allowTruncatedValue)
+          ? await _callToString(thread, ref,
               // Quotes are handled below, so they can be wrapped around the
               // ellipsis.
-              includeQuotesAroundString: false,
-            )
+              format: VariableFormat.noQuotes())
           : null;
-      stringValue ??= '${ref.valueAsString}…';
+      stringValue ??= '$valueAsString…';
 
-      return includeQuotesAroundString ? '"$stringValue"' : stringValue;
+      return formatter.formatString(stringValue);
     } else if (ref.kind == vm.InstanceKind.kString) {
       // Untruncated strings.
-      return includeQuotesAroundString
-          ? '"${ref.valueAsString}"'
-          : ref.valueAsString.toString();
-    } else if (ref.valueAsString != null) {
-      return isTruncated
-          ? '${ref.valueAsString}…'
-          : ref.valueAsString.toString();
+      return formatter.formatString(valueAsString ?? "");
+    } else if (valueAsString != null) {
+      if (isTruncated) {
+        return '$valueAsString…';
+      } else if (ref.kind == vm.InstanceKind.kInt) {
+        return formatter.formatInt(int.tryParse(valueAsString));
+      } else {
+        return valueAsString.toString();
+      }
     } else if (ref.kind == 'PlainInstance') {
       var stringValue = ref.classRef?.name ?? '<unknown instance>';
       if (allowCallingToString) {
-        final toStringValue = await _callToString(
-          thread,
-          ref,
-          includeQuotesAroundString: false,
-        );
+        final toStringValue = await _callToString(thread, ref,
+            // Suppress quotes because this is going inside a longer string.
+            format: VariableFormat.noQuotes());
         // Include the toString() result only if it's not the default (which
         // duplicates the type name we're already showing).
         if (toStringValue != "Instance of '${ref.classRef?.name}'") {
@@ -125,6 +126,7 @@ class ProtocolConverter {
     required bool allowCallingToString,
     int? startItem = 0,
     int? numItems,
+    VariableFormat? format,
   }) async {
     final elements = instance.elements;
     final associations = instance.associations;
@@ -139,6 +141,7 @@ class ProtocolConverter {
           name: null,
           evaluateName: evaluateName,
           allowCallingToString: allowCallingToString,
+          format: format,
         )
       ];
     } else if (elements != null) {
@@ -153,6 +156,7 @@ class ProtocolConverter {
                   evaluateName, '[${start + index}]'),
               allowCallingToString:
                   allowCallingToString && index <= maxToStringsPerEvaluation,
+              format: format,
             ),
           ));
     } else if (associations != null) {
@@ -168,11 +172,18 @@ class ProtocolConverter {
         final callToString =
             allowCallingToString && index <= maxToStringsPerEvaluation;
 
-        final keyDisplay = await convertVmResponseToDisplayString(thread, key,
-            allowCallingToString: callToString);
+        final keyDisplay = await convertVmResponseToDisplayString(
+          thread,
+          key,
+          allowCallingToString: callToString,
+          format: format,
+        );
         final valueDisplay = await convertVmResponseToDisplayString(
-            thread, value,
-            allowCallingToString: callToString);
+          thread,
+          value,
+          allowCallingToString: callToString,
+          format: format,
+        );
 
         // We only provide an evaluateName for the value, and only if the
         // key is a simple value.
@@ -186,7 +197,7 @@ class ProtocolConverter {
         return dap.Variable(
           name: '${start + index}',
           value: '$keyDisplay -> $valueDisplay',
-          variablesReference: thread.storeData(mapEntry),
+          variablesReference: thread.storeData(VariableData(mapEntry, format)),
         );
       }));
     } else if (_isList(instance) &&
@@ -216,13 +227,17 @@ class ProtocolConverter {
           } else {
             name ??= field.name;
           }
-          return convertVmResponseToVariable(thread, field.value,
-              name: name ?? '<unnamed field>',
-              evaluateName: name != null
-                  ? _adapter.combineEvaluateName(evaluateName, '.$name')
-                  : null,
-              allowCallingToString:
-                  allowCallingToString && index <= maxToStringsPerEvaluation);
+          return convertVmResponseToVariable(
+            thread,
+            field.value,
+            name: name ?? '<unnamed field>',
+            evaluateName: name != null
+                ? _adapter.combineEvaluateName(evaluateName, '.$name')
+                : null,
+            allowCallingToString:
+                allowCallingToString && index <= maxToStringsPerEvaluation,
+            format: format,
+          );
         },
       ));
 
@@ -252,6 +267,7 @@ class ProtocolConverter {
                   _adapter.combineEvaluateName(evaluateName, '.$getterName'),
               allowCallingToString:
                   allowCallingToString && index <= maxToStringsPerEvaluation,
+              format: format,
             );
           } catch (e) {
             return dap.Variable(
@@ -325,14 +341,14 @@ class ProtocolConverter {
     ThreadInfo thread,
     vm.Response response, {
     required bool allowCallingToString,
-    bool includeQuotesAroundString = true,
+    VariableFormat? format,
   }) async {
     if (response is vm.InstanceRef) {
       return convertVmInstanceRefToDisplayString(
         thread,
         response,
         allowCallingToString: allowCallingToString,
-        includeQuotesAroundString: includeQuotesAroundString,
+        format: format,
       );
     } else if (response is vm.Sentinel) {
       return '<sentinel>';
@@ -354,12 +370,14 @@ class ProtocolConverter {
     required String? name,
     required String? evaluateName,
     required bool allowCallingToString,
+    VariableFormat? format,
   }) async {
     if (response is vm.InstanceRef) {
       // For non-simple variables, store them and produce a new reference that
       // can be used to access their fields/items/associations.
-      final variablesReference =
-          isSimpleKind(response.kind) ? 0 : thread.storeData(response);
+      final variablesReference = isSimpleKind(response.kind)
+          ? 0
+          : thread.storeData(VariableData(response, format));
 
       return dap.Variable(
         name: name ?? response.kind.toString(),
@@ -368,6 +386,7 @@ class ProtocolConverter {
           thread,
           response,
           allowCallingToString: allowCallingToString,
+          format: format,
         ),
         indexedVariables: _isList(response) ? response.length : null,
         variablesReference: variablesReference,
@@ -545,7 +564,7 @@ class ProtocolConverter {
   Future<String?> _callToString(
     ThreadInfo thread,
     vm.InstanceRef ref, {
-    bool includeQuotesAroundString = true,
+    VariableFormat? format,
   }) async {
     final service = _adapter.vmService;
     if (service == null) {
@@ -571,7 +590,7 @@ class ProtocolConverter {
       thread,
       result,
       allowCallingToString: false, // Don't allow recursing.
-      includeQuotesAroundString: includeQuotesAroundString,
+      format: format,
     );
   }