Skip to content

Commit e9cdaae

Browse files
srujzsCommit Queue
authored andcommitted
Clean up debugger_test to use dart:js_interop/avoid dynamic
- Use JSAny for values returned by the devtoolsFormatter. - Makes deliberate wherever we *have* to make unsafe casts from Dart objects to a JS value - Removes use of package:js (except to test a package:js class for formatting) and dart:js_util - Cleans up implicit dynamic in favor of Object/Object? and void - References dart:_debugger types to make it more clear what types we're using Change-Id: I1b388501a65984bfc92c298dbce9181951040c4b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389588 Commit-Queue: Srujan Gaddam <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent b77b34c commit e9cdaae

File tree

2 files changed

+253
-69
lines changed

2 files changed

+253
-69
lines changed

tests/dartdevc/debugger/debugger_test.dart

Lines changed: 89 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
/// Debugger custom formatter tests.
2-
/// If the tests fail, paste the expected output into the [expectedGolden]
3-
/// string literal in this file and audit the diff to ensure changes are
4-
/// expected.
2+
/// If the tests fail, paste the expected output into the golden file and audit
3+
/// the diff to ensure changes are expected.
54
///
65
/// Currently only DDC supports debugging objects with custom formatters
76
/// but it is reasonable to add support to Dart2JS in the future.
87
@JS()
98
library debugger_test;
109

1110
import 'dart:html';
11+
import 'dart:js_interop';
12+
import 'dart:js_interop_unsafe';
13+
1214
import 'package:expect/async_helper.dart';
1315
import 'package:expect/legacy/minitest.dart'; // ignore: deprecated_member_use_from_same_package
14-
import 'package:js/js.dart';
15-
import 'package:js/js_util.dart' as js_util;
16+
import 'package:js/js.dart' as pkgJs;
1617

1718
import 'dart:_debugger' as _debugger;
1819

@@ -47,78 +48,103 @@ class TestGenericClass<X, Y> {
4748
X x;
4849
}
4950

50-
@JS('Object.getOwnPropertyNames')
51-
external List getOwnPropertyNames(obj);
51+
class FormattedObject {
52+
FormattedObject(this.object, this.config);
5253

53-
@JS('devtoolsFormatters')
54-
external List get _devtoolsFormatters;
55-
List get devtoolsFormatters => _devtoolsFormatters;
54+
JSAny? object;
55+
JSAny? config;
56+
}
5657

5758
@JS('JSON.stringify')
58-
external stringify(value, [Function replacer, int space]);
59+
external String? stringify(JSAny? value, [JSFunction replacer, int space]);
60+
61+
@JS('Object.getOwnPropertyNames')
62+
external JSArray<JSString> getOwnPropertyNames(JSObject obj);
63+
64+
@JS()
65+
external JSArray? get devtoolsFormatters;
66+
67+
@JS('Object.getPrototypeOf')
68+
external Prototype getPrototypeOf(JSAny obj);
69+
70+
extension type FormattedJSObject._(JSObject _) implements JSObject {
71+
external JSAny? get object;
72+
external JSAny? get config;
73+
}
74+
75+
// We use `JSAny` here since we're using this to interop with the prototype of a
76+
// Dart class, which isn't a `JSObject`.
77+
extension type Prototype._(JSAny _) implements JSAny {
78+
external JSAny get constructor;
79+
}
80+
81+
extension type FooBar._(JSObject _) implements JSObject {
82+
external FooBar({String foo});
83+
}
5984

60-
// TODO(jacobr): this is only valid if the DDC library loader is used.
61-
// We need a solution that works with all library loaders.
62-
@JS('dart_library.import')
63-
external importDartLibrary(String path);
85+
@pkgJs.JS()
86+
class PackageJSClass<T> {
87+
external factory PackageJSClass(T x);
88+
}
6489

65-
@JS('ExampleJSClass')
66-
class ExampleJSClass<T> {
67-
external factory ExampleJSClass(T x);
68-
external T get x;
90+
T unsafeCast<T extends JSAny?>(Object? object) {
91+
// This is improper interop code. However, this test mixes Dart and JS values.
92+
// Since this is only ever run on DDC, this is okay, but we should be
93+
// deliberate about where we're mixing Dart and JS values.
94+
return object as T;
6995
}
7096

71-
// Replacer normalizes file names that could vary depending on the test runner.
97+
// Replacer normalizes file names that could vary depending on the test runner
7298
// styles.
73-
replacer(String key, value) {
99+
JSAny? replacer(String key, JSAny? externalValue) {
74100
// The values for keys with name 'object' may be arbitrary Dart nested
75101
// Objects so are not safe to stringify.
76-
if (key == 'object') return '<OBJECT>';
77-
if (value is String) {
78-
if (value.contains('dart_sdk.js')) return '<DART_SDK>';
79-
if (new RegExp(r'[.](js|dart|html)').hasMatch(value)) return '<FILE>';
102+
if (key == 'object') return '<OBJECT>'.toJS;
103+
if (externalValue.isA<JSString>()) {
104+
final value = (externalValue as JSString).toDart;
105+
if (value.contains('dart_sdk.js')) return '<DART_SDK>'.toJS;
106+
if (new RegExp(r'[.](js|dart|html)').hasMatch(value)) {
107+
return '<FILE>'.toJS;
108+
}
80109
// Normalize the name of the `Event` type as it appears in this test.
81110
// The new type system preserves the original name from the Dart source.
82111
// TODO(48585): Remove when no longer running with the old type system.
83-
value = value.replaceAll(r'Event$', 'Event');
112+
return value.replaceAll(r'Event$', 'Event').toJS;
84113
}
85-
return value;
114+
return externalValue;
86115
}
87116

88-
String? format(value) {
117+
String? format(JSAny? value) {
89118
// Avoid double-escaping strings.
90-
if (value is String) return value;
91-
if (value is Function) value = allowInterop(value);
92-
return stringify(value, allowInterop(replacer), 4);
93-
}
94-
95-
class FormattedObject {
96-
FormattedObject(this.object, this.config);
97-
98-
Object? object;
99-
Object? config;
119+
if (value.isA<JSString>()) return (value as JSString).toDart;
120+
return stringify(value, replacer.toJS, 4);
100121
}
101122

102123
/// Extract all object tags from a json ml expression to enable
103124
/// calling the custom formatter on the extracted object tag.
104-
List<FormattedObject> extractNestedFormattedObjects(json) {
125+
List<FormattedObject> extractNestedFormattedObjects(JSAny json) {
105126
var ret = <FormattedObject>[];
106-
if (json is String || json is bool || json is num) return ret;
107-
if (json is List) {
108-
for (var e in json) {
109-
ret.addAll(extractNestedFormattedObjects(e));
127+
if (json.isA<JSString>() || json.isA<JSBoolean>() || json.isA<JSNumber>()) {
128+
return ret;
129+
}
130+
if (json.isA<JSArray>()) {
131+
for (var i = 0; i < (json as JSArray<JSAny>).length; i++) {
132+
ret.addAll(extractNestedFormattedObjects(json[i]));
110133
}
111134
return ret;
112135
}
113136

114-
for (var name in getOwnPropertyNames(json)) {
137+
// Must be a JS object. See JsonMLElement in dart:_debugger.
138+
final jsObject = json as FormattedJSObject;
139+
final propertyNames = getOwnPropertyNames(jsObject);
140+
for (var i = 0; i < propertyNames.length; i++) {
141+
final name = propertyNames[i].toDart;
115142
if (name == 'object') {
116143
// Found a nested formatted object.
117-
ret.add(new FormattedObject(js_util.getProperty(json, 'object'),
118-
js_util.getProperty(json, 'config')));
144+
ret.add(new FormattedObject(jsObject.object, jsObject.config));
119145
return ret;
120146
}
121-
ret.addAll(extractNestedFormattedObjects(js_util.getProperty(json, name)));
147+
ret.addAll(extractNestedFormattedObjects(jsObject[name]!));
122148
}
123149
return ret;
124150
}
@@ -146,12 +172,14 @@ main() async {
146172
document.body!.append(new ScriptElement()
147173
..type = 'text/javascript'
148174
..innerHtml = r"""
149-
window.ExampleJSClass = function ExampleJSClass(x) {
175+
window.PackageJSClass = function PackageJSClass(x) {
150176
this.x = x;
151177
};
152178
""");
153179

154-
var _devtoolsFormatter = devtoolsFormatters.first;
180+
var _devtoolsFormatter = (devtoolsFormatters![0]
181+
as ExternalDartReference<_debugger.JsonMLFormatter>)
182+
.toDartObject;
155183

156184
var actual = new StringBuffer();
157185

@@ -162,22 +190,22 @@ window.ExampleJSClass = function ExampleJSClass(x) {
162190
// of expectations.
163191
// The verify golden match test cases does the final comparison of golden
164192
// to expected output.
165-
addGolden(String name, value) {
193+
void addGolden(String name, JSAny value) {
166194
var text = format(value);
167195
actual.write('Test: $name\n'
168196
'Value:\n'
169197
'$text\n'
170198
'-----------------------------------\n');
171199
}
172200

173-
addFormatterGoldens(String name, object, [config]) {
201+
void addFormatterGoldens(String name, Object? object, [Object? config]) {
174202
addGolden(
175203
'$name formatting header', _devtoolsFormatter.header(object, config));
176204
addGolden('$name formatting body', _devtoolsFormatter.body(object, config));
177205
}
178206

179207
// Include goldens for the nested [[class]] definition field.
180-
addNestedFormatterGoldens(String name, obj) {
208+
void addNestedFormatterGoldens(String name, Object obj) {
181209
addGolden('$name instance header', _devtoolsFormatter.header(obj, null));
182210
var body = _devtoolsFormatter.body(obj, null);
183211
addGolden('$name instance body', body);
@@ -190,9 +218,11 @@ window.ExampleJSClass = function ExampleJSClass(x) {
190218
}
191219

192220
// Include goldens for the nested [[class]] definition field.
193-
addAllNestedFormatterGoldens(String name, obj) {
221+
void addAllNestedFormatterGoldens(String name, Object obj) {
194222
addGolden('$name header', _devtoolsFormatter.header(obj, null));
195-
var body = _devtoolsFormatter.body(obj, null);
223+
// The cast to `JSAny` is safe as `header` and `body` should always return
224+
// JS values.
225+
var body = _devtoolsFormatter.body(obj, null) as JSAny;
196226
addGolden('$name body', body);
197227

198228
var nestedObjects = extractNestedFormattedObjects(body);
@@ -256,14 +286,13 @@ window.ExampleJSClass = function ExampleJSClass(x) {
256286
addFormatterGoldens('Function with function arguments', addEventListener);
257287

258288
// Closure
259-
addGolden('dart:html method', window.addEventListener);
289+
addGolden('dart:html method', unsafeCast(window.addEventListener));
260290

261291
// Get a reference to the JS constructor for a Dart class.
262292
// This tracks a regression bug where overly verbose and confusing output
263293
// was shown for this case.
264294
var testClass = new TestClass(17);
265-
var dartConstructor = js_util.getProperty(
266-
js_util.objectGetPrototypeOf(testClass)!, 'constructor');
295+
var dartConstructor = getPrototypeOf(unsafeCast(testClass)).constructor;
267296
addFormatterGoldens('Raw reference to dart constructor', dartConstructor);
268297
});
269298

@@ -281,8 +310,7 @@ window.ExampleJSClass = function ExampleJSClass(x) {
281310
});
282311

283312
group('JS interop object formatting', () {
284-
var object = js_util.newObject();
285-
js_util.setProperty(object, 'foo', 'bar');
313+
var object = FooBar(foo: 'bar');
286314
// Make sure we don't apply the Dart custom formatter to JS interop objects.
287315
expect(_devtoolsFormatter.header(object, null), isNull);
288316
});
@@ -322,8 +350,8 @@ window.ExampleJSClass = function ExampleJSClass(x) {
322350
'TestGenericClass', new TestGenericClass<int, List>(42));
323351
addNestedFormatterGoldens(
324352
'TestGenericClassJSInterop',
325-
new TestGenericClass<ExampleJSClass<String>, int>(
326-
new ExampleJSClass("Hello")));
353+
new TestGenericClass<PackageJSClass<JSString>, int>(
354+
new PackageJSClass("Hello".toJS)));
327355
});
328356

329357
test('verify golden match', () {

0 commit comments

Comments
 (0)