[M] Change in dart/sdk[main]: [VM/Service] Fix generation of toJson() calls on dynamic variables
Derek Xu (Gerrit)
Attention is currently required from: Ben Konyi.
Derek Xu would like Ben Konyi to review this change.
[VM/Service] Fix generation of toJson() calls on dynamic variables
TEST=CI, manual testing
Issue: https://github.com/dart-lang/sdk/issues/51188
Issue: https://github.com/dart-lang/sdk/issues/51100
Fixes https://github.com/dart-lang/sdk/issues/51100
Change-Id: I4b7122df2e24534ed919aa0665dcf2a777c5deed
---
M pkg/vm_service/CHANGELOG.md
M pkg/vm_service/lib/src/vm_service.dart
M pkg/vm_service/pubspec.yaml
M pkg/vm_service/test/get_object_rpc_test.dart
M pkg/vm_service/tool/dart/generate_dart.dart
5 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/pkg/vm_service/CHANGELOG.md b/pkg/vm_service/CHANGELOG.md
index fcfe3ae..e40cded 100644
--- a/pkg/vm_service/CHANGELOG.md
+++ b/pkg/vm_service/CHANGELOG.md
@@ -1,10 +1,14 @@
# Changelog
+## 11.0.1
+- Fix bug where code would try to call `.toJson()` on `int`s.
+
## 11.0.0
- Change `HttpProfileRequestRef.id` type from `int` to `String`.
- Change `SocketStatistic.id` type from `int` to `String`.
- Change `ext.dart.io.getHttpProfileRequest` `id` parameter type from `int` to `String`.
- Change `ext.dart.io.socketProfilingEnabled` parameter from 'enable' to 'enabled'.
+
## 10.1.2
- Fix bug where code would try to call `.toJson()` on `String`s.
diff --git a/pkg/vm_service/lib/src/vm_service.dart b/pkg/vm_service/lib/src/vm_service.dart
index 0c09d70..e3281a3 100644
--- a/pkg/vm_service/lib/src/vm_service.dart
+++ b/pkg/vm_service/lib/src/vm_service.dart
@@ -2976,7 +2976,7 @@
json.addAll({
'decl': decl?.toJson(),
'name': name,
- 'value': value?.toJson(),
+ 'value': value is int || value is String ? value : value?.toJson(),
});
return json;
}
@@ -3040,7 +3040,7 @@
json['type'] = type;
json.addAll({
'name': name ?? '',
- 'value': value?.toJson(),
+ 'value': value is int || value is String ? value : value?.toJson(),
'declarationTokenPos': declarationTokenPos ?? -1,
'scopeStartTokenPos': scopeStartTokenPos ?? -1,
'scopeEndTokenPos': scopeEndTokenPos ?? -1,
@@ -3114,7 +3114,8 @@
'breakpointNumber': breakpointNumber ?? -1,
'enabled': enabled ?? false,
'resolved': resolved ?? false,
- 'location': location?.toJson(),
+ 'location':
+ location is int || location is String ? location : location?.toJson(),
});
_setIfNotNull(
json, 'isSyntheticAsyncContinuation', isSyntheticAsyncContinuation);
@@ -3653,7 +3654,7 @@
Map<String, dynamic> toJson() {
final json = <String, dynamic>{};
json.addAll({
- 'value': value?.toJson(),
+ 'value': value is int || value is String ? value : value?.toJson(),
});
return json;
}
@@ -4538,7 +4539,12 @@
'static': isStatic ?? false,
});
_setIfNotNull(json, 'location', location?.toJson());
- _setIfNotNull(json, 'staticValue', staticValue?.toJson());
+ _setIfNotNull(
+ json,
+ 'staticValue',
+ staticValue is int || staticValue is String
+ ? staticValue
+ : staticValue?.toJson());
return json;
}
@@ -4770,7 +4776,7 @@
json['type'] = type;
json.addAll({
'name': name ?? '',
- 'owner': owner?.toJson(),
+ 'owner': owner is int || owner is String ? owner : owner?.toJson(),
'static': isStatic ?? false,
'const': isConst ?? false,
'implicit': implicit ?? false,
@@ -4872,7 +4878,7 @@
json['type'] = type;
json.addAll({
'name': name ?? '',
- 'owner': owner?.toJson(),
+ 'owner': owner is int || owner is String ? owner : owner?.toJson(),
'static': isStatic ?? false,
'const': isConst ?? false,
'implicit': implicit ?? false,
@@ -6106,7 +6112,12 @@
'source': source?.toJson(),
});
_setIfNotNull(json, 'parentListIndex', parentListIndex);
- _setIfNotNull(json, 'parentField', parentField?.toJson());
+ _setIfNotNull(
+ json,
+ 'parentField',
+ parentField is int || parentField is String
+ ? parentField
+ : parentField?.toJson());
return json;
}
@@ -6462,8 +6473,8 @@
Map<String, dynamic> toJson() {
final json = <String, dynamic>{};
json.addAll({
- 'key': key?.toJson(),
- 'value': value?.toJson(),
+ 'key': key is int || key is String ? key : key?.toJson(),
+ 'value': value is int || value is String ? value : value?.toJson(),
});
return json;
}
diff --git a/pkg/vm_service/pubspec.yaml b/pkg/vm_service/pubspec.yaml
index 46612ac..aa1038e 100644
--- a/pkg/vm_service/pubspec.yaml
+++ b/pkg/vm_service/pubspec.yaml
@@ -1,5 +1,5 @@
name: vm_service
-version: 11.0.0
+version: 11.0.1
description: >-
A library to communicate with a service implementing the Dart VM
service protocol.
diff --git a/pkg/vm_service/test/get_object_rpc_test.dart b/pkg/vm_service/test/get_object_rpc_test.dart
index cd064e9..a9c8911 100644
--- a/pkg/vm_service/test/get_object_rpc_test.dart
+++ b/pkg/vm_service/test/get_object_rpc_test.dart
@@ -704,6 +704,10 @@
final fieldsMap = HashMap.fromEntries(
result.fields!.map((f) => MapEntry(f.name, f.value)));
expect(fieldsMap.keys.length, result.length);
+ // [BoundField]s have fields with type [dynamic], and such fields have
+ // broken [toJson()] in the past. So, we make the following call just to
+ // ensure that it doesn't throw.
+ result.fields!.first.toJson();
expect(fieldsMap.containsKey(0), true);
expect(fieldsMap[0].valueAsString, '1');
expect(fieldsMap.containsKey("x"), true);
diff --git a/pkg/vm_service/tool/dart/generate_dart.dart b/pkg/vm_service/tool/dart/generate_dart.dart
index ec06cf6..05571fa 100644
--- a/pkg/vm_service/tool/dart/generate_dart.dart
+++ b/pkg/vm_service/tool/dart/generate_dart.dart
@@ -1664,6 +1664,12 @@
gen.write('.toJson()');
}
gen.write(').toList()');
+ } else if (field.type.isMultipleReturns) {
+ gen.write('''
+${field.generatableName} is int || ${field.generatableName} is String
+ ? ${field.generatableName}
+ : ${field.generatableName}?.toJson()
+''');
} else {
gen.write('${field.generatableName}?.toJson()');
}
To view, visit change 281463. To unsubscribe, or for help writing mail filters, visit settings.
Derek Xu (Gerrit)
Attention is currently required from: Ben Konyi.
Ben Konyi (Gerrit)
Attention is currently required from: Derek Xu.
Patch set 2:Code-Review +1
CBuild (Gerrit)
Attention is currently required from: Derek Xu.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/c3a1382e96751492b00c4ec17e70e98ab996571d
Derek Xu (Gerrit)
Attention is currently required from: Derek Xu.
Patch set 2:Commit-Queue +2
Commit Queue (Gerrit)
Commit Queue submitted this change.
[VM/Service] Fix generation of toJson() calls on dynamic variables
TEST=CI, manual testing
Issue: https://github.com/dart-lang/sdk/issues/51188
Issue: https://github.com/dart-lang/sdk/issues/51100
Fixes https://github.com/dart-lang/sdk/issues/51100
Change-Id: I4b7122df2e24534ed919aa0665dcf2a777c5deed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281463
Commit-Queue: Derek Xu <der...@google.com>
Reviewed-by: Ben Konyi <bko...@google.com>
---
M pkg/vm_service/CHANGELOG.md
M pkg/vm_service/lib/src/vm_service.dart
M pkg/vm_service/pubspec.yaml
M pkg/vm_service/test/get_object_rpc_test.dart
M pkg/vm_service/tool/dart/generate_dart.dart
5 files changed, 53 insertions(+), 11 deletions(-)
To view, visit change 281463. To unsubscribe, or for help writing mail filters, visit settings.
Siva Annamalai (Gerrit)
Attention is currently required from: Derek Xu.
6 comments:
Patchset:
I presume this change is related to seeing unboxed values, if that is the case this change seems to be applied to every place where the type used to be dynamic even if that field is not expected to of that type.
File pkg/vm_service/lib/src/vm_service.dart:
Patch Set #2, Line 3657:
'value': value is int || value is String ? value : value?.toJson(),can value be a double?
Patch Set #2, Line 4545:
staticValue is int || staticValue is StringDoes double need to be accounted for ?
Patch Set #2, Line 4779:
'owner': owner is int || owner is String ? owner : owner?.toJson(),how can the owner of a function be an int ?
Patch Set #2, Line 4881:
'owner': owner is int || owner is String ? owner : owner?.toJson(),Ditto question.
Patch Set #2, Line 6118:
parentField is int || parentField is StringCan parentField be an int ?
To view, visit change 281463. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Derek Xu.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/c25308d0037b39019be2e56384c6b1e3b8c87d61
Derek Xu (Gerrit)
6 comments:
Patchset:
I presume this change is related to seeing unboxed values, if that is the case this change seems to […]
Created https://dart-review.googlesource.com/c/sdk/+/281540 to prevent generating unneeded type checks.
File pkg/vm_service/lib/src/vm_service.dart:
Patch Set #2, Line 3657:
'value': value is int || value is String ? value : value?.toJson(),can value be a double?
No
Patch Set #2, Line 4545:
staticValue is int || staticValue is StringDoes double need to be accounted for ?
No
Patch Set #2, Line 4779:
'owner': owner is int || owner is String ? owner : owner?.toJson(),how can the owner of a function be an int ?
Patch Set #2, Line 4881:
'owner': owner is int || owner is String ? owner : owner?.toJson(),Ditto question.
Patch Set #2, Line 6118:
parentField is int || parentField is StringCan parentField be an int ?
Yes
To view, visit change 281463. To unsubscribe, or for help writing mail filters, visit settings.