Skip to content

Commit

Permalink
Fix failures on displaying getters (#2343)
Browse files Browse the repository at this point in the history
* Fix test faliures

* Update changelog

* Fixe analyzer errors

* Display type instanses as common instances

* Added getters checks to instance tests

* Fixed analyzer errors

* Add dart:_rti to the list of sdk libraries

* Fix failing tests

* Fixed failing tests
  • Loading branch information
Anna Gringauze authored Jan 22, 2024
1 parent a5ad753 commit e382db3
Show file tree
Hide file tree
Showing 10 changed files with 582 additions and 231 deletions.
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)

## 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',
);

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

0 comments on commit e382db3

Please sign in to comment.