Skip to content

Cleanup record types display #2070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Apr 22, 2023
Merged
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
127 changes: 113 additions & 14 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 29 additions & 2 deletions dwds/lib/src/debugging/metadata/class.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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');

Expand Down Expand Up @@ -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,
Expand All @@ -122,6 +134,7 @@ class ClassMetaData {
int.tryParse('$length'),
isFunction,
isRecord,
isRecordType,
isNativeError,
);
}
Expand All @@ -134,6 +147,7 @@ class ClassMetaData {
this.length,
this.isFunction,
this.isRecord,
this.isRecordType,
this.isNativeError,
);

Expand All @@ -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;
Expand All @@ -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'];

Expand All @@ -173,6 +190,10 @@ class ClassMetaData {
result['length'] = positionalCount + namedCount;
}

if (isRecordType) {
result['name'] = 'RecordType';
result['length'] = arg.types.length;
}
return result;
}
''';
Expand All @@ -190,6 +211,7 @@ class ClassMetaData {
dartName: metadata['dartName'],
isFunction: metadata['isFunction'],
isRecord: metadata['isRecord'],
isRecordType: metadata['isRecordType'],
isNativeError: metadata['isNativeError'],
length: metadata['length'],
);
Expand All @@ -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
Expand All @@ -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;
Expand Down
Loading