Skip to content
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

Fix failures on displaying getters #2343

Merged
merged 10 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Send untruncated `dart:developer` logs to debugging clients. - [#2333](https://github.com/dart-lang/webdev/pull/2333)
- Enabling tests that run with the DDC module system and exposing `utilities/ddc_names.dart` - [#2295](https://github.com/dart-lang/webdev/pull/2295)
- Fix errors on displaying getters. - [#2343](https://github.com/dart-lang/webdev/pull/2343)
annagrin marked this conversation as resolved.
Show resolved Hide resolved

## 23.1.1

Expand Down
78 changes: 37 additions & 41 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,29 +238,49 @@ class InstanceHelper extends Domain {
final objectId = remoteObject.objectId;
if (objectId == null) return null;

final fields = await _getInstanceFields(
metaData,
remoteObject,
offset: offset,
count: count,
);

final result = Instance(
kind: InstanceKind.kPlainInstance,
id: objectId,
identityHashCode: remoteObject.objectId.hashCode,
classRef: metaData.classRef,
fields: fields,
);
return result;
}

Future<List<BoundField>> _getInstanceFields(
ClassMetaData metaData,
RemoteObject remoteObject, {
int? offset,
int? count,
}) async {
final objectId = remoteObject.objectId;
if (objectId == null) throw StateError('Object id is null for instance');

final properties = await inspector.getProperties(
objectId,
offset: offset,
count: count,
length: metaData.length,
);

final dartProperties = await _dartFieldsFor(properties, remoteObject);
var boundFields = await Future.wait(
final boundFields = await Future.wait(
dartProperties
.map<Future<BoundField>>((p) => _fieldFor(p, metaData.classRef)),
);
boundFields = boundFields

return boundFields
.where((bv) => inspector.isDisplayableObject(bv.value))
.toList()
..sort(_compareBoundFields);
final result = Instance(
kind: InstanceKind.kPlainInstance,
id: objectId,
identityHashCode: remoteObject.objectId.hashCode,
classRef: metaData.classRef,
fields: boundFields,
);
return result;
}

int _compareBoundFields(BoundField a, BoundField b) {
Expand Down Expand Up @@ -700,7 +720,13 @@ class InstanceHelper extends Domain {
final objectId = remoteObject.objectId;
if (objectId == null) return null;

final fields = await _typeFields(metaData.classRef, remoteObject);
final fields = await _getInstanceFields(
metaData,
remoteObject,
offset: offset,
count: count,
);

return Instance(
identityHashCode: objectId.hashCode,
kind: InstanceKind.kType,
Expand All @@ -714,36 +740,6 @@ class InstanceHelper extends Domain {
);
}

/// 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>> _typeFields(
ClassRef classRef,
RemoteObject type,
) async {
// Present the type as an instance of `core.Type` class and
// hide the internal implementation.
final expression = _jsRuntimeFunctionCall('getTypeFields(this)');

final result = await inspector.jsCallFunctionOn(type, expression, []);
final hashCodeObject = await inspector.loadField(result, 'hashCode');
final runtimeTypeObject = await inspector.loadField(result, 'runtimeType');

final properties = [
Property({'name': 'hashCode', 'value': hashCodeObject}),
Property({'name': 'runtimeType', 'value': runtimeTypeObject}),
];

final boundFields = await Future.wait(
properties.map<Future<BoundField>>((p) => _fieldFor(p, classRef)),
);
return boundFields;
}

/// 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 Down
1 change: 1 addition & 0 deletions dwds/lib/src/debugging/metadata/provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class MetadataProvider {
'dart:_js_primitives',
'dart:_metadata',
'dart:_native_typed_data',
'dart:_rti',
'dart:async',
'dart:collection',
'dart:convert',
Expand Down
41 changes: 39 additions & 2 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,35 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
await isCompilerInitialized;
_checkIsolate('evaluate', isolateId);

final library = await inspector.getLibrary(targetId);
late Obj object;
try {
object = await inspector.getObject(targetId);
} catch (_) {
return ErrorRef(
kind: 'error',
message: 'Evaluate is called on an unsupported target:'
'$targetId',
id: createId(),
);
}

final library =
object is Library ? object : inspector.isolate.rootLib;

if (object is Instance) {
// Evaluate is called on a target - convert this to a dart
// expression and scope by adding a target variable to the
// expression and the scope, for example:
//
// Library: 'package:hello_world/main.dart'
// Expression: 'hashCode' => 'x.hashCode'
// Scope: {} => { 'x' : targetId }

final target = _newVariableForScope(scope);
expression = '$target.$expression';
scope = (scope ?? {})..addAll({target: targetId});
}

return await _getEvaluationResult(
isolateId,
() => evaluator.evaluateExpression(
Expand All @@ -631,7 +659,7 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
);
}
throw RPCError(
'evaluateInFrame',
'evaluate',
RPCErrorKind.kInvalidRequest.code,
'Expression evaluation is not supported for this configuration.',
);
Expand All @@ -640,6 +668,15 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
);
}

String _newVariableForScope(Map<String, String>? scope) {
// Find a new variable not in scope.
var candidate = 'x';
while (scope?.containsKey(candidate) ?? false) {
candidate += '\$1';
}
return candidate;
}

@override
Future<Response> evaluateInFrame(
String isolateId,
Expand Down
59 changes: 43 additions & 16 deletions dwds/test/evaluate_common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ void testAll({
await getInstanceRef(event.topFrame!.index!, 'stream');
final instance = await getInstance(instanceRef);

expect(instance, matchInstance('_AsBroadcastStream<int>'));
expect(instance, matchInstanceClassName('_AsBroadcastStream<int>'));
});
});

Expand Down Expand Up @@ -668,24 +668,24 @@ void testAll({
tearDown(() async {});

evaluate(
libraryId,
targetId,
expr, {
scope,
}) async =>
await context.service.evaluate(
isolateId,
libraryId,
targetId,
expr,
scope: scope,
);

getInstanceRef(
libraryId,
targetId,
expr, {
scope,
}) async {
final result = await evaluate(
libraryId,
targetId,
expr,
scope: scope,
);
Expand All @@ -699,6 +699,28 @@ void testAll({
return isolate.rootLib!.id!;
}

test(
'RecordType getters',
() async {
final libraryId = getRootLibraryId();

final type = await getInstanceRef(libraryId, '(0,1).runtimeType');
final result = await getInstanceRef(type.id, 'hashCode');

expect(result, matchInstanceRefKind('Double'));
},
skip: 'https://github.com/dart-lang/sdk/issues/54609',
annagrin marked this conversation as resolved.
Show resolved Hide resolved
);

test('Object getters', () async {
final libraryId = getRootLibraryId();

final type = await getInstanceRef(libraryId, 'Object()');
final result = await getInstanceRef(type.id, 'hashCode');

expect(result, matchInstanceRefKind('Double'));
});

test('with scope', () async {
final libraryId = getRootLibraryId();

Expand Down Expand Up @@ -762,15 +784,13 @@ void testAll({
final evaluation2 = evaluate(libraryId, 'MainClass(1,1).toString()');

final results = await Future.wait([evaluation1, evaluation2]);

expect(
results[0],
matchErrorRef(contains('No batch result object ID')),
);
expect(
results[1],
matchErrorRef(contains('No batch result object ID')),
matchErrorRef(
contains('Evaluate is called on an unsupported target'),
),
);
expect(results[1], matchInstanceRef('1, 1'));
});

test('with scope override', () async {
Expand Down Expand Up @@ -902,13 +922,20 @@ Future<String> _setBreakpointInInjectedClient(WipDebugger debugger) async {
return result.json['result']['breakpointId'];
}

Matcher matchInstanceRef(dynamic value) => isA<InstanceRef>().having(
(instance) => instance.valueAsString,
'valueAsString',
value,
Matcher matchInstanceRefKind(String kind) =>
isA<InstanceRef>().having((instance) => instance.kind, 'kind', kind);

Matcher matchInstanceRef(dynamic value) => isA<InstanceRef>()
.having((instance) => instance.valueAsString, 'valueAsString', value);

Matcher matchInstanceClassName(dynamic className) => isA<Instance>().having(
(instance) => instance.classRef!.name,
'class name',
className,
);

Matcher matchInstance(dynamic className) => isA<Instance>().having(
Matcher matchInstanceRefClassName(dynamic className) =>
isA<InstanceRef>().having(
(instance) => instance.classRef!.name,
'class name',
className,
Expand Down
63 changes: 48 additions & 15 deletions dwds/test/instances/common/instance_inspection_common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,32 @@ void runTests({
final instanceId = instanceRef.id!;
expect(await getObject(instanceId), matchListInstance(type: 'int'));

expect(await getFields(instanceRef), [0, 1, 2]);
expect(await getFields(instanceRef, offset: 1, count: 0), []);
expect(await getFields(instanceRef, offset: 0), [0, 1, 2]);
expect(await getFields(instanceRef, offset: 0, count: 1), [0]);
expect(await getFields(instanceRef, offset: 1), [1, 2]);
expect(await getFields(instanceRef, offset: 1, count: 1), [1]);
expect(await getFields(instanceRef, offset: 1, count: 3), [1, 2]);
expect(await getFields(instanceRef, offset: 3, count: 3), []);
expect(
await getFields(instanceRef),
{0: 0.0, 1: 1.0, 2: 2.0},
);
expect(await getFields(instanceRef, offset: 1, count: 0), {});
expect(
await getFields(instanceRef, offset: 0),
{0: 0.0, 1: 1.0, 2: 2.0},
);
expect(
await getFields(instanceRef, offset: 0, count: 1),
{0: 0.0},
);
expect(
await getFields(instanceRef, offset: 1),
{0: 1.0, 1: 2.0},
);
expect(
await getFields(instanceRef, offset: 1, count: 1),
{0: 1.0},
);
expect(
await getFields(instanceRef, offset: 1, count: 3),
{0: 1.0, 1: 2.0},
);
expect(await getFields(instanceRef, offset: 3, count: 3), {});
});
});

Expand Down Expand Up @@ -284,13 +302,28 @@ void runTests({
matchSetInstance(type: '_HashSet<int>'),
);

expect(await getFields(instanceRef), [1, 4, 5, 7]);
expect(await getFields(instanceRef, offset: 0), [1, 4, 5, 7]);
expect(await getFields(instanceRef, offset: 1, count: 2), [4, 5]);
expect(await getFields(instanceRef, offset: 2), [5, 7]);
expect(await getFields(instanceRef, offset: 2, count: 10), [5, 7]);
expect(await getFields(instanceRef, offset: 1, count: 0), []);
expect(await getFields(instanceRef, offset: 10, count: 2), []);
expect(
await getFields(instanceRef),
{0: 1.0, 1: 4.0, 2: 5.0, 3: 7.0},
);
expect(
await getFields(instanceRef, offset: 0),
{0: 1.0, 1: 4.0, 2: 5.0, 3: 7.0},
);
expect(
await getFields(instanceRef, offset: 1, count: 2),
{0: 4.0, 1: 5.0},
);
expect(
await getFields(instanceRef, offset: 2),
{0: 5.0, 1: 7.0},
);
expect(
await getFields(instanceRef, offset: 2, count: 10),
{0: 5.0, 1: 7.0},
);
expect(await getFields(instanceRef, offset: 1, count: 0), {});
expect(await getFields(instanceRef, offset: 10, count: 2), {});
});
});

Expand Down
2 changes: 1 addition & 1 deletion dwds/test/instances/common/patterns_inspection_common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void runTests({
final frame = event.topFrame!;
final frameIndex = frame.index!;
final instanceRef = await getInstanceRef(frameIndex, 'obj');
expect(await getFields(instanceRef), [0, 1]);
expect(await getFields(instanceRef), {0: 0.0, 1: 1.0});

expect(await getFrameVariables(frame), {
'obj': matchListInstance(type: 'int'),
Expand Down
Loading