Skip to content

Support records #1919

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 32 commits into from
Feb 11, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
cb7251b
Display records and add tests
Jan 26, 2023
cbe8d96
Add _experiment test fixture
Jan 26, 2023
0e485cf
Update instanceRef kind to kRecord
Jan 26, 2023
c11b71d
Make offsets work
Jan 27, 2023
13a34ba
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Jan 30, 2023
71c1fe8
Format
Jan 30, 2023
9c34b75
Require min versions of dds and vm_service that support records
Jan 30, 2023
96824c6
Cleanup and add more records tests
Jan 31, 2023
8fd379a
Cleanup names
Jan 31, 2023
56e23f9
Fix lists and maps getObject tests
Jan 31, 2023
5039cab
Fix records test failures
Jan 31, 2023
0adb558
Temporarily disabled failing record type tests
Feb 1, 2023
a0826c9
cleanup comments and formatting
Feb 1, 2023
aa36615
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Feb 1, 2023
98119ca
Enable rectord type tests on main
Feb 1, 2023
31e23e0
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Feb 1, 2023
733c7c6
Addressed CR comments, added tests for named parameters, fixed bugs
Feb 2, 2023
491ab13
Merge with master
Feb 7, 2023
0e280ae
Fix list count bug
Feb 7, 2023
9c4e8bf
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Feb 9, 2023
2cff45a
Update min SDK constrain to support record types and enable skipped t…
Feb 9, 2023
19d64bc
Merge with master
Feb 9, 2023
b5202c7
Fix merge errors
Feb 9, 2023
dd31083
Fix failure on getObject of list with out of range offset
Feb 9, 2023
0b89601
Cleanup
Feb 10, 2023
689d6ba
Use JS logic to check for dart types
Feb 10, 2023
569321a
Addressed CR comments
Feb 10, 2023
bf6e585
Use integers as BoundField names for records
Feb 10, 2023
ded1222
Create positional fields faster, add comments
Feb 10, 2023
614d789
Cleaned up comments
Feb 10, 2023
a8c4700
Addressed comments
Feb 10, 2023
0a18c18
Address CR comments
Feb 10, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

- Cleanup `getObject` code for lists and maps.
- Now works with offset `0` and `null` count.
- Support records:
- Update SDK constraint to `>=3.0.0-188.0.dev <4.0.0`.
- Update `package:vm_service` constraint to `>=10.1.2 <12.0.0`.
- Update `package:dds` constraint to `^2.7.1`.
- Fill `BoundField.name` for records.
- Display records as a container of fields.

## 17.0.0

Expand Down
2 changes: 1 addition & 1 deletion dwds/debug_extension/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: >-
A chrome extension for Dart debugging.

environment:
sdk: ">=3.0.0-134.0.dev <4.0.0"
sdk: ">=3.0.0-188.0.dev <4.0.0"

dependencies:
async: ^2.3.0
Expand Down
2 changes: 1 addition & 1 deletion dwds/debug_extension_mv3/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: >-
A Chrome extension for Dart debugging.

environment:
sdk: ">=3.0.0-134.0.dev <4.0.0"
sdk: ">=3.0.0-188.0.dev <4.0.0"

dependencies:
built_value: ^8.3.0
Expand Down
131 changes: 122 additions & 9 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class InstanceHelper extends Domain {

final classRef = metaData?.classRef;
if (metaData == null || classRef == null) return null;
if (metaData.jsName == 'Function') {
if (metaData.isFunction) {
return _closureInstanceFor(remoteObject);
}

Expand All @@ -122,6 +122,9 @@ class InstanceHelper extends Domain {
} else if (metaData.isSystemMap) {
return await _mapInstanceFor(classRef, remoteObject,
offset: offset, count: count, length: metaData.length);
} else if (metaData.isRecord) {
return await _recordInstanceFor(classRef, remoteObject,
offset: offset, count: count, length: metaData.length);
} else {
return await _plainInstanceFor(classRef, remoteObject,
offset: offset, count: count, length: metaData.length);
Expand Down Expand Up @@ -150,6 +153,7 @@ class InstanceHelper extends Domain {
Future<BoundField> _fieldFor(Property property, ClassRef classRef) async {
final instance = await _instanceRefForRemote(property.value);
return BoundField(
name: property.name,
decl: FieldRef(
// TODO(grouma) - Convert JS name to Dart.
name: property.name,
Expand Down Expand Up @@ -322,6 +326,115 @@ class InstanceHelper extends Domain {
.map((element) async => await _instanceRefForRemote(element.value)));
}

/// Return the value of the length attribute from [properties], if present.
/// Return elements of the list from [properties].
///
/// Ignore any non-elements like 'length', 'fixed$length', etc.
static List<Property> _indexedListProperties(List<Property> properties) =>
properties
.where((p) => p.name != null && int.tryParse(p.name!) != null)
.toList();

/// The fields for a Dart Record.
Future<List<BoundField>> _recordFields(RemoteObject map,
{int? offset, int? count}) async {
// We do this in in awkward way because we want the keys and values, 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", []);
namedCount = named == null? 0: named.length;
var values = sdkUtils.dloadRepl(this, "values");
values = sdkUtils.dsendRepl(values, "toList", []);

var positional = Array.from(
{ length: positionalCount },
(value, index) => 1 + index
);

return {
positionalCount: positionalCount,
positional: positional,
namedCount: namedCount,
named: named,
values: values
};
}
''';
final result = await inspector.jsCallFunctionOn(map, expression, []);
final positionalCount =
(await inspector.loadField(result, 'positionalCount'))!.value as int;
final positional = await inspector.loadField(result, 'positional');
final named = await inspector.loadField(result, 'named');
final namedCount =
(await inspector.loadField(result, 'namedCount'))!.value as int;
final values = await inspector.loadField(result, 'values');

final length = positionalCount + namedCount;
if (offset != null && length <= offset) {
return [];
}

final positionalInstance =
await instanceFor(positional, offset: offset, count: count);
final valuesInstance =
await instanceFor(values, offset: offset, count: count);

int? nOffset;
int? nCount;
if (offset != null) {
nOffset = offset < positionalCount ? 0 : offset - positionalCount;
}
if (count != null) {
nCount = count < positionalCount ? 0 : count - positionalCount;
}
final namedInstance =
await instanceFor(named, offset: nOffset, count: nCount);

final nameElements = [
...positionalInstance?.elements?.map((e) => e.valueAsString) ?? [],
...namedInstance?.elements?.map((e) => e.valueAsString) ?? [],
];

final fields = <BoundField>[];
final valueElements = valuesInstance?.elements ?? [];
Map.fromIterables(nameElements, valueElements).forEach((key, value) {
fields.add(BoundField(name: key, value: value));
});
return fields;
}

/// Create a Record instance with class [classRef] from [remoteObject].
Future<Instance?> _recordInstanceFor(
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 _recordFields(remoteObject, offset: offset, count: count);
final rangeCount = _calculateRangeCount(
count: count, elementCount: fields.length, length: length);
return Instance(
identityHashCode: remoteObject.objectId.hashCode,
kind: InstanceKind.kRecord,
id: objectId,
classRef: classRef)
..length = length
..offset = offset
..count = rangeCount
..fields = fields;
}

/// Return the available count of elements in the requested range.
/// Return `null` if the range includes the whole object.
/// [count] is the range length requested by the `getObject` call.
Expand All @@ -336,14 +449,6 @@ class InstanceHelper extends Domain {
return min(count, elementCount);
}

/// Return elements of the list from [properties].
///
/// Ignore any non-elements like 'length', 'fixed$length', etc.
static List<Property> _indexedListProperties(List<Property> properties) =>
properties
.where((p) => p.name != null && int.tryParse(p.name!) != null)
.toList();

/// Filter [allJsProperties] and return a list containing only those
/// that correspond to Dart fields on [remoteObject].
///
Expand Down Expand Up @@ -461,6 +566,14 @@ class InstanceHelper extends Domain {
classRef: metaData.classRef)
..length = metaData.length;
}
if (metaData.isRecord) {
return InstanceRef(
kind: InstanceKind.kRecord,
id: objectId,
identityHashCode: remoteObject.objectId.hashCode,
classRef: metaData.classRef)
..length = metaData.length;
}
return InstanceRef(
kind: InstanceKind.kPlainInstance,
id: objectId,
Expand Down
91 changes: 74 additions & 17 deletions dwds/lib/src/debugging/metadata/class.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,37 @@ import 'package:dwds/src/utilities/domain.dart';
import 'package:vm_service/vm_service.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';

const _dartCoreLibrary = 'dart:core';

/// A hard-coded ClassRef for the Closure class.
final classRefForClosure = classRefFor('dart:core', 'Closure');
final classRefForClosure = classRefFor(_dartCoreLibrary, 'Closure');

/// A hard-coded ClassRef for the String class.
final classRefForString = classRefFor('dart:core', InstanceKind.kString);
final classRefForString = classRefFor(_dartCoreLibrary, InstanceKind.kString);

/// A hard-coded ClassRef for a (non-existent) class called Unknown.
final classRefForUnknown = classRefFor('dart:core', 'Unknown');
final classRefForUnknown = classRefFor(_dartCoreLibrary, 'Unknown');

/// A hard-coded LibraryRef for a a dart:core library.
final libraryRefForCore = LibraryRef(
id: _dartCoreLibrary,
name: _dartCoreLibrary,
uri: _dartCoreLibrary,
);

/// Returns a [LibraryRef] for the provided library ID and class name.
LibraryRef libraryRefFor(String libraryId) => LibraryRef(
id: libraryId,
name: libraryId,
uri: libraryId,
);

/// Returns a [ClassRef] for the provided library ID and class name.
ClassRef classRefFor(String? libraryId, String? name) => ClassRef(
id: 'classes|$libraryId|$name',
name: name,
library: libraryId == null
? null
: LibraryRef(id: libraryId, name: libraryId, uri: libraryId));
ClassRef classRefFor(String libraryId, String? name) => ClassRef(
id: 'classes|$libraryId|$name',
name: name,
library: libraryRefFor(libraryId),
);

/// Meta data for a remote Dart class in Chrome.
class ClassMetaData {
Expand All @@ -43,15 +58,34 @@ class ClassMetaData {
final String? dartName;

/// The library identifier, which is the URI of the library.
final String? libraryId;

factory ClassMetaData(
{Object? jsName, Object? libraryId, Object? dartName, Object? length}) {
return ClassMetaData._(jsName as String?, libraryId as String?,
dartName as String?, int.tryParse('$length'));
final String libraryId;

factory ClassMetaData({
Object? jsName,
Object? libraryId,
Object? dartName,
Object? length,
bool isFunction = false,
bool isRecord = false,
}) {
return ClassMetaData._(
jsName as String?,
libraryId as String? ?? _dartCoreLibrary,
dartName as String?,
int.tryParse('$length'),
isFunction,
isRecord,
);
}

ClassMetaData._(this.jsName, this.libraryId, this.dartName, this.length);
ClassMetaData._(
this.jsName,
this.libraryId,
this.dartName,
this.length,
this.isFunction,
this.isRecord,
);

/// Returns the ID of the class.
///
Expand All @@ -69,14 +103,29 @@ class ClassMetaData {
const sdkUtils = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk').dart;
const classObject = sdkUtils.getReifiedType(arg);
const isFunction = sdkUtils.AbstractFunctionType.is(classObject);
const isRecord = sdkUtils.RecordType.is(classObject);
const result = {};
result['name'] = isFunction ? 'Function' : classObject.name;
var name = isFunction ? 'Function' : classObject.name;

result['name'] = name;
result['libraryId'] = sdkUtils.getLibraryUri(classObject);
result['dartName'] = sdkUtils.typeName(classObject);
result['isFunction'] = isFunction;
result['isRecord'] = isRecord;
result['length'] = arg['length'];

if (isRecord) {
result['name'] = 'Record';
var shape = classObject.shape;
var positionalCount = shape.positionals;
var namedCount = shape.named == null ? 0 : shape.named.length;
result['length'] = positionalCount + namedCount;
}

return result;
}
''';

final result = await inspector.jsCallFunctionOn(
remoteObject, evalExpression, [remoteObject],
returnByValue: true);
Expand All @@ -85,6 +134,8 @@ class ClassMetaData {
jsName: metadata['name'],
libraryId: metadata['libraryId'],
dartName: metadata['dartName'],
isFunction: metadata['isFunction'],
isRecord: metadata['isRecord'],
length: metadata['length'],
);
} on ChromeDebugException {
Expand All @@ -105,4 +156,10 @@ class ClassMetaData {

/// True if this class refers to system Lists, which are treated specially.
bool get isSystemList => jsName == 'JSArray';

/// True if this class refers to a function type.
bool isFunction;

/// True if this class refers to a record type.
bool isRecord;
}
4 changes: 2 additions & 2 deletions dwds/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ description: >-
repository: https://github.com/dart-lang/webdev/tree/master/dwds

environment:
sdk: ">=3.0.0-134.0.dev <4.0.0"
sdk: ">=3.0.0-188.0.dev <4.0.0"

dependencies:
async: ^2.9.0
Expand All @@ -34,7 +34,7 @@ dependencies:
stack_trace: ^1.10.0
sse: ^4.1.2
uuid: ^3.0.6
vm_service: ">=10.1.0 <12.0.0"
vm_service: ">=10.1.2 <12.0.0"
web_socket_channel: ^2.2.0
webkit_inspection_protocol: ^1.0.1

Expand Down
Loading