Cleanup record types display (#2070)

* Fix getObject failure on record class * Merge with master * Update PR reference * Cleanup record type display * Cleanup * Update changelog * Update version and build * Remove printing * Validate record and record type refs in tests * Address CR comments * Address CR comments * Address CR comments
dart-lang · Apr 22, 2023 · 8b42c95 · 8b42c95
1 parent 1cfb3bd
commit 8b42c95
Show file tree

Hide file tree

Showing 7 changed files with 946 additions and 400 deletions.
diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 - Do not show async frame errors on evaluation. - [#2073](https://github.com/dart-lang/webdev/pull/2073)
 - Refactor code for presenting record instances. - [#2074](https://github.com/dart-lang/webdev/pull/2074)
+- Display record types concisely. - [#2070](https://github.com/dart-lang/webdev/pull/2070)
 
 ## 19.0.0
 

diff --git a/dwds/lib/src/debugging/instance.dart b/dwds/lib/src/debugging/instance.dart
@@ -155,6 +155,14 @@ class InstanceHelper extends Domain {
         count: count,
         length: metaData.length,
       );
+    } else if (metaData.isRecordType) {
+      return await _recordTypeInstanceFor(
+        classRef,
+        remoteObject,
+        offset: offset,
+        count: count,
+        length: metaData.length,
+      );
     } else if (metaData.isSet) {
       return await _setInstanceFor(
         classRef,
@@ -416,16 +424,16 @@ class InstanceHelper extends Domain {
     int? count,
     int? length,
   }) async {
-    // Filter out all non-indexed properties
-    final elements = _indexedListProperties(
-      await debugger.getProperties(
-        list.objectId!,
-        offset: offset,
-        count: count,
-        length: length,
-      ),
+    final properties = await debugger.getProperties(
+      list.objectId!,
+      offset: offset,
+      count: count,
+      length: length,
     );
 
+    // Filter out all non-indexed properties
+    final elements = _indexedListProperties(properties);
+
     final rangeCount = _calculateRangeCount(
       count: count,
       elementCount: elements.length,
@@ -546,19 +554,19 @@ class InstanceHelper extends Domain {
         await instanceFor(valuesObject, offset: offset, count: count);
     final valueElements = valuesInstance?.elements ?? [];
 
-    if (fieldNameElements.length != valueElements.length) {
-      _logger.warning('Record fields and values are not the same length.');
-      return [];
-    }
-
     return _elementsToBoundFields(fieldNameElements, valueElements);
   }
 
   /// Create a list of `BoundField`s from field [names] and [values].
-  static List<BoundField> _elementsToBoundFields(
+  List<BoundField> _elementsToBoundFields(
     List<dynamic> names,
     List<dynamic> values,
   ) {
+    if (names.length != values.length) {
+      _logger.warning('Bound field names and values are not the same length.');
+      return [];
+    }
+
     final boundFields = <BoundField>[];
     Map.fromIterables(names, values).forEach((name, value) {
       boundFields.add(BoundField(name: name, value: value));
@@ -608,6 +616,89 @@ class InstanceHelper extends Domain {
       ..fields = fields;
   }
 
+  /// Create a RecordType instance with class [classRef] from [remoteObject].
+  ///
+  /// Returns an instance containing [count] fields, if available,
+  /// starting from the [offset].
+  ///
+  /// If [offset] is `null`, assumes 0 offset.
+  /// If [count] is `null`, return all fields starting from the offset.
+  /// [length] is the expected length of the whole object, read from
+  /// the [ClassMetaData].
+  Future<Instance?> _recordTypeInstanceFor(
+    ClassRef classRef,
+    RemoteObject remoteObject, {
+    int? offset,
+    int? count,
+    int? length,
+  }) async {
+    final objectId = remoteObject.objectId;
+    if (objectId == null) return null;
+    // Records are complicated, do an eval to get names and values.
+    final fields =
+        await _recordTypeFields(remoteObject, offset: offset, count: count);
+    final rangeCount = _calculateRangeCount(
+      count: count,
+      elementCount: fields.length,
+      length: length,
+    );
+    return Instance(
+      identityHashCode: remoteObject.objectId.hashCode,
+      kind: InstanceKind.kRecordType,
+      id: objectId,
+      classRef: classRef,
+    )
+      ..length = length
+      ..offset = offset
+      ..count = rangeCount
+      ..fields = fields;
+  }
+
+  /// The field types for a Dart RecordType.
+  ///
+  /// Returns a range of [count] field types, if available, starting from
+  /// the [offset].
+  ///
+  /// If [offset] is `null`, assumes 0 offset.
+  /// If [count] is `null`, return all field types starting from the offset.
+  Future<List<BoundField>> _recordTypeFields(
+    RemoteObject record, {
+    int? offset,
+    int? count,
+  }) async {
+    // We do this in in awkward way because we want the names and types, but we
+    // can't return things by value or some Dart objects will come back as
+    // values that we need to be RemoteObject, e.g. a List of int.
+    final expression = '''
+      function() {
+        var sdkUtils = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk').dart;
+        var shape = sdkUtils.dloadRepl(this, "shape");
+        var positionalCount = sdkUtils.dloadRepl(shape, "positionals");
+        var named = sdkUtils.dloadRepl(shape, "named");
+        named = named == null? null: sdkUtils.dsendRepl(named, "toList", []);
+        var types = sdkUtils.dloadRepl(this, "types");
+        types = types.map(t => sdkUtils.wrapType(t));
+        types = sdkUtils.dsendRepl(types, "toList", []);
+
+        return {
+          positionalCount: positionalCount,
+          named: named,
+          types: types
+        };
+      }
+    ''';
+    final result = await inspector.jsCallFunctionOn(record, expression, []);
+    final fieldNameElements =
+        await _recordShapeFields(result, offset: offset, count: count);
+
+    final typesObject = await inspector.loadField(result, 'types');
+    final typesInstance =
+        await instanceFor(typesObject, offset: offset, count: count);
+    final typeElements = typesInstance?.elements ?? [];
+
+    return _elementsToBoundFields(fieldNameElements, typeElements);
+  }
+
   Future<Instance?> _setInstanceFor(
     ClassRef classRef,
     RemoteObject remoteObject, {
@@ -801,6 +892,14 @@ class InstanceHelper extends Domain {
             classRef: metaData.classRef,
           )..length = metaData.length;
         }
+        if (metaData.isRecordType) {
+          return InstanceRef(
+            kind: InstanceKind.kRecordType,
+            id: objectId,
+            identityHashCode: remoteObject.objectId.hashCode,
+            classRef: metaData.classRef,
+          )..length = metaData.length;
+        }
         if (metaData.isSet) {
           return InstanceRef(
             kind: InstanceKind.kSet,

diff --git a/dwds/lib/src/debugging/metadata/class.dart b/dwds/lib/src/debugging/metadata/class.dart
@@ -10,6 +10,7 @@ import 'package:vm_service/vm_service.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
 const _dartCoreLibrary = 'dart:core';
+const _dartRuntimeLibrary = 'dart:_runtime';
 const _dartInterceptorsLibrary = 'dart:_interceptors';
 
 /// A hard-coded ClassRef for the Closure class.
@@ -21,6 +22,10 @@ final classRefForString = classRefFor(_dartCoreLibrary, InstanceKind.kString);
 /// A hard-coded ClassRef for the Record class.
 final classRefForRecord = classRefFor(_dartCoreLibrary, InstanceKind.kRecord);
 
+/// A hard-coded ClassRef for the RecordType class.
+final classRefForRecordType =
+    classRefFor(_dartRuntimeLibrary, InstanceKind.kRecordType);
+
 /// A hard-coded ClassRef for a (non-existent) class called Unknown.
 final classRefForUnknown = classRefFor(_dartCoreLibrary, 'Unknown');
 
@@ -105,14 +110,21 @@ class ClassMetaData {
     Object? length,
     bool isFunction = false,
     bool isRecord = false,
+    bool isRecordType = false,
     bool isNativeError = false,
   }) {
     final jName = jsName as String?;
     final dName = dartName as String?;
     final library = libraryId as String? ?? _dartCoreLibrary;
     final id = '$library:$jName';
 
-    final classRef = isRecord ? classRefForRecord : classRefFor(library, dName);
+    var classRef = classRefFor(library, dName);
+    if (isRecord) {
+      classRef = classRefForRecord;
+    }
+    if (isRecordType) {
+      classRef = classRefForRecordType;
+    }
 
     return ClassMetaData._(
       id,
@@ -122,6 +134,7 @@ class ClassMetaData {
       int.tryParse('$length'),
       isFunction,
       isRecord,
+      isRecordType,
       isNativeError,
     );
   }
@@ -134,6 +147,7 @@ class ClassMetaData {
     this.length,
     this.isFunction,
     this.isRecord,
+    this.isRecordType,
     this.isNativeError,
   );
 
@@ -149,10 +163,12 @@ class ClassMetaData {
       function(arg) {
         const sdk = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk');
         const dart = sdk.dart;
+        const core = sdk.core;
         const interceptors = sdk._interceptors;
         const classObject = dart.getReifiedType(arg);
         const isFunction = classObject instanceof dart.AbstractFunctionType;
         const isRecord = classObject instanceof dart.RecordType;
+        const isRecordType = dart.is(arg, dart.RecordType);
         const isNativeError = dart.is(arg, interceptors.NativeError);
         const result = {};
         var name = isFunction ? 'Function' : classObject.name;
@@ -162,6 +178,7 @@ class ClassMetaData {
         result['dartName'] = dart.typeName(classObject);
         result['isFunction'] = isFunction;
         result['isRecord'] = isRecord;
+        result['isRecordType'] = isRecordType;
         result['isNativeError'] = isNativeError;
         result['length'] = arg['length'];
 
@@ -173,6 +190,10 @@ class ClassMetaData {
           result['length'] = positionalCount + namedCount;
         }
 
+        if (isRecordType) {
+          result['name'] = 'RecordType';
+          result['length'] = arg.types.length;
+        }
         return result;
       }
     ''';
@@ -190,6 +211,7 @@ class ClassMetaData {
         dartName: metadata['dartName'],
         isFunction: metadata['isFunction'],
         isRecord: metadata['isRecord'],
+        isRecordType: metadata['isRecordType'],
         isNativeError: metadata['isNativeError'],
         length: metadata['length'],
       );
@@ -203,6 +225,8 @@ class ClassMetaData {
     }
   }
 
+  /// TODO(annagrin): convert fields and getters below to kinds.
+
   /// True if this class refers to system maps, which are treated specially.
   ///
   /// Classes that implement Map or inherit from MapBase are still treated as
@@ -219,9 +243,12 @@ class ClassMetaData {
   /// True if this class refers to a function type.
   bool isFunction;
 
-  /// True if this class refers to a record type.
+  /// True if this class refers to a Record type.
   bool isRecord;
 
+  /// True if this class refers to a RecordType type.
+  bool isRecordType;
+
   /// True is this class refers to a native JS type.
   /// i.e. inherits from NativeError.
   bool isNativeError;