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

[Pigeon] Condenses serialization formats #2745

Merged
merged 72 commits into from
Dec 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
524af57
Fixs missed indents on generated comments
tarrinneal Oct 25, 2022
39b10a5
begin improved serialization
tarrinneal Oct 25, 2022
cdaecf9
formatting errors
tarrinneal Oct 25, 2022
a4510d5
Dart encoder updated to use lists, tests to match
tarrinneal Oct 27, 2022
eca71a2
Begin swift serial, change error shape
tarrinneal Nov 2, 2022
d439f90
update tests for swift and dart
tarrinneal Nov 4, 2022
ae34fd4
true > false
tarrinneal Nov 5, 2022
c1f3f22
Return to Object
tarrinneal Nov 5, 2022
f8cd1c1
correctly nest lists in dart and tests
tarrinneal Nov 5, 2022
c415c4c
Kotlin gen and tests updated to lists
tarrinneal Nov 7, 2022
5168014
Java and Tests updated to new serials
tarrinneal Nov 11, 2022
340f967
Merge branch 'main' of github.com:flutter/packages
tarrinneal Nov 12, 2022
b5340e9
Stuart check this pr (start of objc changes)
tarrinneal Nov 16, 2022
38232cd
obj c tests for serial
tarrinneal Nov 17, 2022
980a72a
update unit tests to match new generated outputs
tarrinneal Nov 17, 2022
899732d
finish objc tests
tarrinneal Nov 18, 2022
71b5330
More kt tests
tarrinneal Nov 23, 2022
c755b4a
c++ generator and unit tests
tarrinneal Nov 23, 2022
85b53d4
Merge branch 'main' of github.com:flutter/packages
tarrinneal Nov 23, 2022
e6b2552
analyze, format, changelog
tarrinneal Nov 23, 2022
f785b16
test file updates for ci
tarrinneal Nov 23, 2022
300dae6
format and analyze again
tarrinneal Nov 23, 2022
0c73933
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Nov 23, 2022
b4003b0
a few more test generated files
tarrinneal Nov 24, 2022
eb43268
Partial Windows fixes
stuartmorgan Nov 27, 2022
0b498cc
null field tests c++
tarrinneal Nov 29, 2022
9fe0ef0
merge main
tarrinneal Nov 29, 2022
dd40057
format
tarrinneal Nov 29, 2022
888c7bc
fix merge issue with broken test
tarrinneal Nov 29, 2022
7547df5
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Nov 30, 2022
60aa9fe
remove unneeded wrapping
tarrinneal Nov 30, 2022
8859079
Merge branch 'main'
tarrinneal Nov 30, 2022
bee1047
generated files
tarrinneal Nov 30, 2022
aa722ad
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 1, 2022
1e12317
fix some formatting errors
tarrinneal Dec 1, 2022
3245d77
Merge branch 'main' needs regenerated files still
tarrinneal Dec 1, 2022
b2eb193
format
tarrinneal Dec 1, 2022
6cc4cd0
Merge branch 'main'
tarrinneal Dec 1, 2022
f4e9201
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 3, 2022
b54362b
more gen files
tarrinneal Dec 3, 2022
f9c8237
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 8, 2022
799d56d
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 8, 2022
84a83c0
gen files
tarrinneal Dec 8, 2022
23221fe
generator reviews pt1
tarrinneal Dec 9, 2022
01e2072
test fixes pt1
tarrinneal Dec 9, 2022
d579798
fixed nits and nil issues with objc
tarrinneal Dec 12, 2022
0a34031
better fix for objc null classes
tarrinneal Dec 12, 2022
8b11d20
fix doc comment
tarrinneal Dec 12, 2022
b29a0b7
unit test updates
tarrinneal Dec 12, 2022
a01e819
format
tarrinneal Dec 12, 2022
bf0af6c
some c++ fixes
tarrinneal Dec 12, 2022
7cbb9b8
typo
tarrinneal Dec 12, 2022
dfc12e4
format
tarrinneal Dec 12, 2022
f36ebd1
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 12, 2022
9bfe05e
Some nits and such
tarrinneal Dec 13, 2022
2bf4593
comment
tarrinneal Dec 13, 2022
1af4f27
remove deleted files
tarrinneal Dec 13, 2022
9b4bbcc
c++ nits
tarrinneal Dec 13, 2022
6c782fa
objc nits
tarrinneal Dec 13, 2022
4b77db9
all but one c++ bug
tarrinneal Dec 13, 2022
8a6fd34
Merge branch 'serialization' of https://github.com/tarrinneal/package…
tarrinneal Dec 13, 2022
f081333
init all fields
tarrinneal Dec 13, 2022
9b7c24a
start of documenting data shape
tarrinneal Dec 13, 2022
8c3f1ed
nits and error handling
tarrinneal Dec 14, 2022
aa36dc8
more nits and such
tarrinneal Dec 15, 2022
9174253
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 15, 2022
221d864
bug?
tarrinneal Dec 15, 2022
09e0eef
references
tarrinneal Dec 15, 2022
8b706d1
const
tarrinneal Dec 15, 2022
4679325
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 16, 2022
40feac3
new null for larger alltypes
tarrinneal Dec 16, 2022
80a0346
Merge branch 'main' of github.com:flutter/packages into serialization
tarrinneal Dec 18, 2022
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
Prev Previous commit
Next Next commit
nits and error handling
  • Loading branch information
tarrinneal committed Dec 14, 2022
commit 8c3f1ed418bc3f3463228111105187236937df25
12 changes: 4 additions & 8 deletions packages/pigeon/lib/cpp_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,8 @@ void _writeDataClassImplementation(Indent indent, Class klass, Root root) {
// Deserialization.
indent.write('${klass.name}::${klass.name}(flutter::EncodableList list) ');
tarrinneal marked this conversation as resolved.
Show resolved Hide resolved
indent.scoped('{', '}', () {
getFieldsInSerializationOrder(klass)
.toList()
.asMap()
.forEach((int index, final NamedType field) {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
final String instanceVariableName = _makeInstanceVariableName(field);
final String pointerFieldName =
'${_pointerPrefix}_${_makeVariableName(field)}';
Expand Down Expand Up @@ -1061,12 +1059,10 @@ void generateCppHeader(
indent, anEnum.documentationComments, _docCommentSpec);
indent.write('enum class ${anEnum.name} ');
indent.scoped('{', '};', () {
int index = 0;
for (final String member in anEnum.members) {
enumerate(anEnum.members, (int index, final String member) {
indent.writeln(
'$member = $index${index == anEnum.members.length - 1 ? '' : ','}');
index++;
}
});
});
}

Expand Down
15 changes: 6 additions & 9 deletions packages/pigeon/lib/dart_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ void _writeCodec(Indent indent, String codecName, Api api, Root root) {
indent.writeln('@override');
indent.write('void writeValue(WriteBuffer buffer, Object? value) ');
indent.scoped('{', '}', () {
codecClasses
.toList()
.asMap()
.forEach((int index, final EnumeratedClass customClass) {
enumerate(codecClasses, (int index, final EnumeratedClass customClass) {
final String ifValue = 'if (value is ${customClass.name}) ';
if (index == 0) {
tarrinneal marked this conversation as resolved.
Show resolved Hide resolved
indent.write(ifValue);
Expand Down Expand Up @@ -169,7 +166,8 @@ String _getMethodArgumentsSignature(
///
/// Messages will be sent and received in a list.
///
/// If the message recieved was succesful it will be contained at the 0'th index.
/// If the message recieved was succesful,
/// the result will be contained at the 0'th index.
///
/// If the message was a failure, the list will contain 3 items:
/// a code, a message, and details in that order.
Expand Down Expand Up @@ -594,13 +592,12 @@ $resultAt != null
indent.writeln('result as List<Object?>;');
indent.write('return ${klass.name}');
indent.scoped('(', ');', () {
int index = 0;
for (final NamedType field in getFieldsInSerializationOrder(klass)) {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
indent.write('${field.name}: ');
writeValueDecode(field, index);
indent.addln(',');
index++;
}
});
});
});
}
Expand Down
24 changes: 7 additions & 17 deletions packages/pigeon/lib/java_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,7 @@ Result<$returnType> $resultName = new Result<$returnType>() {
if (method.isAsynchronous) {
indent.writeln('reply.reply(wrappedError);');
} else {
indent.writeln('wrapped.add(wrappedError.get(0));');
indent.writeln('wrapped.add(wrappedError.get(1));');
indent.writeln('wrapped.add(wrappedError.get(2));');
indent.writeln('wrapped = wrappedError;');
}
});
if (!method.isAsynchronous) {
Expand Down Expand Up @@ -571,17 +569,11 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {

indent.write('public enum ${anEnum.name} ');
indent.scoped('{', '}', () {
int index = 0;
for (final String member in anEnum.members) {
enumerate(anEnum.members, (int index, final String member) {
indent.writeln(
'${camelToSnake(member)}($index)${index == anEnum.members.length - 1 ? ';' : ','}');
index++;
}
});
indent.writeln('');
// We use explicit indexing here as use of the ordinal() method is
// discouraged. The toMap and fromMap API matches class API to allow
// the same code to work with enums and classes, but this
// can also be done directly in the host and flutter APIs.
indent.writeln('private final int index;');
indent.write('private ${anEnum.name}(final int index) ');
indent.scoped('{', '}', () {
Expand Down Expand Up @@ -623,7 +615,7 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
indent.write('@NonNull ArrayList<Object> toList() ');
indent.scoped('{', '}', () {
indent.writeln(
'ArrayList<Object> toListResult = new ArrayList<Object>();');
'ArrayList<Object> toListResult = new ArrayList<Object>(${klass.fields.length});');
for (final NamedType field in getFieldsInSerializationOrder(klass)) {
final HostDatatype hostDatatype = getFieldHostDatatype(
field,
Expand Down Expand Up @@ -653,10 +645,8 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
indent.scoped('{', '}', () {
const String result = 'pigeonResult';
indent.writeln('${klass.name} $result = new ${klass.name}();');
getFieldsInSerializationOrder(klass)
.toList()
.asMap()
.forEach((int index, final NamedType field) {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
final String fieldVariable = field.name;
final String setter = _makeSetter(field);
indent.writeln('Object $fieldVariable = list.get($index);');
Expand Down Expand Up @@ -751,7 +741,7 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
void writeWrapError() {
indent.format('''
@NonNull private static ArrayList<Object> wrapError(@NonNull Throwable exception) {
\tArrayList<Object> errorList = new ArrayList<>();
\tArrayList<Object> errorList = new ArrayList<>(3);
\terrorList.add(exception.toString());
\terrorList.add(exception.getClass().getSimpleName());
\terrorList.add("Cause: " + exception.getCause() + ", Stacktrace: " + Log.getStackTraceString(exception));
Expand Down
70 changes: 32 additions & 38 deletions packages/pigeon/lib/kotlin_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ void _writeHostApi(Indent indent, Api api, Root root) {
indent.write('channel.setMessageHandler ');
indent.scoped('{ $messageVarName, reply ->', '}', () {
indent.writeln('val wrapped = mutableListOf<Any?>()');
tarrinneal marked this conversation as resolved.
Show resolved Hide resolved

indent.writeln('var error = listOf<Any?>()');
indent.write('try ');
indent.scoped('{', '}', () {
final List<String> methodArgument = <String>[];
Expand Down Expand Up @@ -246,13 +246,20 @@ void _writeHostApi(Indent indent, Api api, Root root) {
}, addTrailingNewline: false);
indent.add(' catch (exception: Error) ');
indent.scoped('{', '}', () {
indent.writeln('wrapped.add(wrapError(exception))');
indent.writeln('error = wrapError(exception)');
if (method.isAsynchronous) {
indent.writeln('reply.reply(wrapped)');
tarrinneal marked this conversation as resolved.
Show resolved Hide resolved
}
});
if (!method.isAsynchronous) {
indent.writeln('reply.reply(wrapped)');
indent.write('if (error.size == 0) ');
indent.scoped('{', '}', () {
indent.writeln('reply.reply(wrapped)');
}, addTrailingNewline: false);
indent.add(' else ');
indent.scoped('{', '}', () {
indent.writeln('reply.reply(error)');
});
}
});
}, addTrailingNewline: false);
Expand Down Expand Up @@ -472,20 +479,14 @@ void generateKotlin(KotlinOptions options, Root root, StringSink sink) {
indent, anEnum.documentationComments, _docCommentSpec);
indent.write('enum class ${anEnum.name}(val raw: Int) ');
indent.scoped('{', '}', () {
// We use explicit indexing here as use of the ordinal() method is
// discouraged. The toList and fromList API matches class API to allow
// the same code to work with enums and classes, but this
// can also be done directly in the host and flutter APIs.
int index = 0;
for (final String member in anEnum.members) {
enumerate(anEnum.members, (int index, final String member) {
indent.write('${member.toUpperCase()}($index)');
if (index != anEnum.members.length - 1) {
indent.addln(',');
} else {
indent.addln(';');
}
index++;
}
});

indent.writeln('');
indent.write('companion object ');
Expand All @@ -509,31 +510,26 @@ void generateKotlin(KotlinOptions options, Root root, StringSink sink) {
}

void writeToList() {
indent.write('fun toList(): MutableList<Any?> ');
indent.write('fun toList(): List<Any?> ');
indent.scoped('{', '}', () {
indent.writeln('val list = mutableListOf<Any?>()');

for (final NamedType field in getFieldsInSerializationOrder(klass)) {
final HostDatatype hostDatatype = getHostDatatype(field);
String toWriteValue = '';
final String fieldName = field.name;
if (!hostDatatype.isBuiltin &&
rootClassNameSet.contains(field.type.baseName)) {
toWriteValue = '$fieldName?.toList()';
} else if (!hostDatatype.isBuiltin &&
rootEnumNameSet.contains(field.type.baseName)) {
toWriteValue = '$fieldName?.raw';
} else {
toWriteValue = fieldName;
}

if (field.type.isNullable) {
indent.writeln('list.add($toWriteValue)');
} else {
indent.writeln('list.add($toWriteValue)');
indent.write('return listOf<Any?>');
indent.scoped('(', ')', () {
for (final NamedType field in getFieldsInSerializationOrder(klass)) {
final HostDatatype hostDatatype = getHostDatatype(field);
String toWriteValue = '';
final String fieldName = field.name;
if (!hostDatatype.isBuiltin &&
rootClassNameSet.contains(field.type.baseName)) {
toWriteValue = '$fieldName?.toList()';
} else if (!hostDatatype.isBuiltin &&
rootEnumNameSet.contains(field.type.baseName)) {
toWriteValue = '$fieldName?.raw';
} else {
toWriteValue = fieldName;
}
indent.writeln('$toWriteValue,');
}
}
indent.writeln('return list');
});
});
}

Expand All @@ -546,10 +542,8 @@ void generateKotlin(KotlinOptions options, Root root, StringSink sink) {
indent.write('fun fromList(list: List<Any?>): $className ');

indent.scoped('{', '}', () {
getFieldsInSerializationOrder(klass)
.toList()
.asMap()
.forEach((int index, final NamedType field) {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
final HostDatatype hostDatatype = getHostDatatype(field);

// The StandardMessageCodec can give us [Integer, Long] for
Expand Down
12 changes: 4 additions & 8 deletions packages/pigeon/lib/objc_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -561,13 +561,11 @@ void generateObjcHeader(ObjcOptions options, Root root, StringSink sink) {

indent.write('typedef NS_ENUM(NSUInteger, $enumName) ');
indent.scoped('{', '};', () {
int index = 0;
for (final String member in anEnum.members) {
enumerate(anEnum.members, (int index, final String member) {
// Capitalized first letter to ensure Swift compatibility
indent.writeln(
'$enumName${member[0].toUpperCase()}${member.substring(1)} = $index,');
index++;
}
});
});
}

Expand Down Expand Up @@ -950,10 +948,8 @@ static id GetNullableObjectAtIndex(NSArray* array, NSInteger key) {
indent.scoped('{', '}', () {
const String resultName = 'pigeonResult';
indent.writeln('$className *$resultName = [[$className alloc] init];');
getFieldsInSerializationOrder(klass)
.toList()
.asMap()
.forEach((int index, final NamedType field) {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
if (enumNames.contains(field.type.baseName)) {
indent.writeln(
'$resultName.${field.name} = [${_listGetter(classNames, 'list', field, index, options.prefix)} integerValue];');
Expand Down
21 changes: 5 additions & 16 deletions packages/pigeon/lib/swift_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,9 @@ import FlutterMacOS

indent.write('enum ${anEnum.name}: Int ');
indent.scoped('{', '}', () {
// We use explicit indexing here as use of the ordinal() method is
// discouraged. The toList and fromList API matches class API to allow
// the same code to work with enums and classes, but this
// can also be done directly in the host and flutter APIs.
int index = 0;
for (final String member in anEnum.members) {
enumerate(anEnum.members, (int index, final String member) {
indent.writeln('case ${_camelCase(member)} = $index');
index++;
}
});
});
}

Expand Down Expand Up @@ -513,10 +507,7 @@ import FlutterMacOS
toWriteValue = field.name;
}

final String comma =
getFieldsInSerializationOrder(klass).last == field ? '' : ',';

indent.writeln('$toWriteValue$comma');
indent.writeln('$toWriteValue,');
}
});
});
Expand All @@ -527,10 +518,8 @@ import FlutterMacOS
indent.write('static func fromList(_ list: [Any?]) -> $className? ');

indent.scoped('{', '}', () {
getFieldsInSerializationOrder(klass)
.toList()
.asMap()
.forEach((int index, final NamedType field) {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
final HostDatatype hostDatatype = getHostDatatype(field);

final String listValue = 'list[$index]';
Expand Down
15 changes: 10 additions & 5 deletions packages/pigeon/test/kotlin_generator_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void main() {
expect(code, contains('data class Foobar ('));
expect(code, contains('val field1: Long? = null'));
expect(code, contains('fun fromList(list: List<Any?>): Foobar'));
expect(code, contains('fun toList(): MutableList<Any?>'));
expect(code, contains('fun toList(): List<Any?>'));
});

test('gen one enum', () {
Expand Down Expand Up @@ -130,14 +130,19 @@ void main() {
if (api != null) {
channel.setMessageHandler { message, reply ->
val wrapped = mutableListOf<Any?>()
var error = listOf<Any?>()
try {
val args = message as List<Any?>
val inputArg = args[0] as Input
wrapped.add(api.doSomething(inputArg))
} catch (exception: Error) {
wrapped.add(wrapError(exception))
error = wrapError(exception)
}
if (error.size == 0) {
reply.reply(wrapped)
} else {
reply.reply(error)
}
reply.reply(wrapped)
}
} else {
channel.setMessageHandler(null)
Expand Down Expand Up @@ -368,7 +373,7 @@ void main() {
final String code = sink.toString();
expect(code, contains('fun doSomething(): Output'));
expect(code, contains('wrapped.add(api.doSomething())'));
expect(code, contains('wrapped.add(wrapError(exception))'));
expect(code, contains('error = wrapError(exception)'));
expect(code, contains('reply(wrapped)'));
});

Expand Down Expand Up @@ -482,7 +487,7 @@ void main() {
expect(
code, contains('val nested: Nested? = (list[0] as? List<Any?>)?.let'));
expect(code, contains('Nested.fromList(it)'));
expect(code, contains('fun toList(): MutableList<Any?>'));
expect(code, contains('fun toList(): List<Any?>'));
});

test('gen one async Host Api', () {
Expand Down