Skip to content

[pigeon] implemented generics for fields #416

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.

Sign up for GitHub

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

Closed
gaaclarke wants to merge 14 commits into from
Closed

[pigeon] implemented generics for fields #416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
43da42e
[pigeon] implemented generics
gaaclarke Jul 21, 2021
7d4f275
implemented generics for java
gaaclarke Jul 21, 2021
bd04fa8
implemented generics for objc
gaaclarke Jul 22, 2021
3e75a35
started parsing return value generics
gaaclarke Jul 22, 2021
5cebd68
started parsing return value generics
gaaclarke Jul 22, 2021
8e9de1d
started parsing argument generics
gaaclarke Jul 22, 2021
ceafca5
fixed maps on dart
gaaclarke Jul 22, 2021
d4e133a
formatter and started throwing an error for primitive generics
gaaclarke Jul 23, 2021
f4ab00d
fixed faulty rebase
gaaclarke Jul 28, 2021
e0819cf
stuarts feedback
gaaclarke Jul 29, 2021
6ea9b5b
stuarts feedback for typedeclaration
gaaclarke Jul 30, 2021
69744ac
Revert "stuarts feedback for typedeclaration"
gaaclarke Aug 3, 2021
9860126
introduced the TypedEntity and TypeDeclaration
gaaclarke Aug 3, 2021
845897d
Revert "introduced the TypedEntity and TypeDeclaration"
gaaclarke Aug 3, 2021
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 packages/pigeon/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* [front-end] Added more errors for incorrect usage of Pigeon (previously they were just ignored).
* Moved Pigeon to using a custom codec which allows collection types to contain custom classes.
* Started allowing primitive data types as arguments and return types.
* Started supporting generics' type arguments for fields in classes.

## 0.3.0

Expand Down
40 changes: 28 additions & 12 deletions packages/pigeon/lib/ast.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,18 @@ class Method extends Node {
required this.name,
required this.returnType,
required this.argType,
this.isArgNullable = false,
this.isAsynchronous = false,
this.isReturnNullable = false,
this.offset,
});

/// The name of the method.
String name;

/// The data-type of the return value.
String returnType;

/// True if the method can return a null value.
bool isReturnNullable;
Field returnType;

/// The data-type of the argument.
String argType;

/// True if the argument has a null tag `?`.
bool isArgNullable;
Field argType;

@stuartmorgan-g stuartmorgan-g Aug 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment and name for both of these indicate that they are a type, so they should be TypeDeclarations rather than Fields.


/// Whether the receiver of this method is expected to return synchronously or not.
bool isAsynchronous;
Expand Down Expand Up @@ -82,6 +74,30 @@ class Api extends Node {
}
}

/// A parameter to a generic entity. For example, "String" to "List<String>".
class TypeArgument {

@stuartmorgan-g stuartmorgan-g Aug 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a TypedEntity?

/// Constructor for [TypeArgument].
TypeArgument({
required this.dataType,
required this.isNullable,
this.typeArguments,

@stuartmorgan-g stuartmorgan-g Aug 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three arguments are a TypeDeclaration, so that's what the constructor argument and the field should be. The whole point of adding TypeDeclaration was to avoid this duplication everywhere.

});

/// A string representation of the base datatype.
final String dataType;

/// The type arguments to this [TypeArgument].
final List<TypeArgument>? typeArguments;

/// True if the type is nullable.
final bool isNullable;

@override
String toString() {
return '(TypeArgument dataType:$dataType isNullable:$isNullable typeArguments:$typeArguments)';
Comment thread
stuartmorgan-g marked this conversation as resolved.
}
}

/// Represents a field on a [Class].
class Field extends Node {
/// Parametric constructor for [Field].
Expand All @@ -106,11 +122,11 @@ class Field extends Node {
bool isNullable;

/// Type parameters used for generics.
List<Field>? typeArguments;
List<TypeArgument>? typeArguments;

@override
String toString() {
return '(Field name:$name dataType:$dataType)';
return '(Field name:$name dataType:$dataType typeArguments:$typeArguments)';

@stuartmorgan-g stuartmorgan-g Aug 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you still have the bug here that will be fixed by replacing the fields here with TypeDeclaration.

}
}

Expand Down
57 changes: 39 additions & 18 deletions packages/pigeon/lib/dart_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ final BinaryMessenger$nullTag _binaryMessenger;
}
String argSignature = '';
String sendArgument = 'null';
if (func.argType != 'void') {
argSignature = '${func.argType} arg';
if (func.argType.dataType != 'void') {

@stuartmorgan-g stuartmorgan-g Aug 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious: why was avoiding LoD violations so important for TypeDeclaration that it needed a new interface wrapping its accessors, while this—which looks conceptually the same to me, and which you do throughout this PR—is not an issue for you?

argSignature = '${func.argType.dataType} arg';
sendArgument = 'arg';
}
indent.write(
'Future<${func.returnType}> ${func.name}($argSignature) async ',
'Future<${func.returnType.dataType}> ${func.name}($argSignature) async ',
);
indent.scoped('{', '}', () {
final String channelName = makeChannelName(api, func);
Expand All @@ -137,9 +137,9 @@ final BinaryMessenger$nullTag _binaryMessenger;
'\'$channelName\', codec, binaryMessenger: _binaryMessenger);',
);
});
final String returnStatement = func.returnType == 'void'
final String returnStatement = func.returnType.dataType == 'void'
? '// noop'
: 'return (replyMap[\'${Keys.result}\'] as ${func.returnType}$nullTag)$unwrapOperator;';
: 'return (replyMap[\'${Keys.result}\'] as ${func.returnType.dataType}$nullTag)$unwrapOperator;';
indent.format('''
final Map<Object$nullTag, Object$nullTag>$nullTag replyMap =\n\t\tawait channel.send($sendArgument) as Map<Object$nullTag, Object$nullTag>$nullTag;
if (replyMap == null) {
Expand Down Expand Up @@ -181,10 +181,11 @@ void _writeFlutterApi(
indent.addln('');
for (final Method func in api.methods) {
final bool isAsync = func.isAsynchronous;
final String returnType =
isAsync ? 'Future<${func.returnType}>' : func.returnType;
final String returnType = isAsync
? 'Future<${func.returnType.dataType}>'
: func.returnType.dataType;
final String argSignature =
func.argType == 'void' ? '' : '${func.argType} arg';
func.argType.dataType == 'void' ? '' : '${func.argType.dataType} arg';
Comment thread
stuartmorgan-g marked this conversation as resolved.
indent.writeln('$returnType ${func.name}($argSignature);');
}
indent.write('static void setup(${api.name}$nullTag api) ');
Expand Down Expand Up @@ -215,12 +216,12 @@ void _writeFlutterApi(
'channel.$messageHandlerSetter((Object$nullTag message) async ',
);
indent.scoped('{', '});', () {
final String argType = func.argType;
final String returnType = func.returnType;
final String argType = func.argType.dataType;
final String returnType = func.returnType.dataType;
final bool isAsync = func.isAsynchronous;
final String emptyReturnStatement = isMockHandler
? 'return <Object$nullTag, Object$nullTag>{};'
: func.returnType == 'void'
: func.returnType.dataType == 'void'
? 'return;'
: 'return null;';
String call;
Expand Down Expand Up @@ -263,14 +264,30 @@ void _writeFlutterApi(
});
}

String _addGenericTypes(String dataType, String nullTag) {
switch (dataType) {
/// Converts a [List] of [TypeArgument]s to a comma separated [String] to be
/// used in Dart code.
String _flattenTypeArguments(List<TypeArgument> args, String nullTag) {
Comment thread
stuartmorgan-g marked this conversation as resolved.
return args
.map((TypeArgument arg) => arg.typeArguments == null
? '${arg.dataType}$nullTag'
: '${arg.dataType}<${_flattenTypeArguments(arg.typeArguments!, nullTag)}>$nullTag')
.reduce((String value, String element) => '$value, $element');
}

/// Creates the type declaration for use in Dart code from a [Field] making sure
/// that type arguments are used for primitive generic types.
String _addGenericTypes(Field field, String nullTag) {
Comment thread
stuartmorgan-g marked this conversation as resolved.
switch (field.dataType) {
case 'List':
return 'List<Object$nullTag>$nullTag';
return (field.typeArguments == null)
? 'List<Object$nullTag>$nullTag'
: 'List<${_flattenTypeArguments(field.typeArguments!, nullTag)}>$nullTag';

@stuartmorgan-g stuartmorgan-g Aug 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below, consider a local for field.typeArguments so that you don't need the !. (You could even hoist it outside the switch and share the line; it's not always needed, but it's also just a trivial accessor so calling it when not needed isn't going to hurt anything.)

case 'Map':
return 'Map<Object$nullTag, Object$nullTag>$nullTag';
return (field.typeArguments == null)
? 'Map<Object$nullTag, Object$nullTag>$nullTag'
: 'Map<${_flattenTypeArguments(field.typeArguments!, nullTag)}>$nullTag';
default:
return '$dataType$nullTag';
return '${field.dataType}$nullTag';
}
}

Expand Down Expand Up @@ -315,7 +332,7 @@ void generateDart(DartOptions opt, Root root, StringSink sink) {
indent.write('class ${klass.name} ');
indent.scoped('{', '}', () {
for (final Field field in klass.fields) {
final String datatype = _addGenericTypes(field.dataType, nullTag);
final String datatype = _addGenericTypes(field, nullTag);
indent.writeln('$datatype ${field.name};');
}
if (klass.fields.isNotEmpty) {
Expand Down Expand Up @@ -365,9 +382,13 @@ pigeonMap['${field.name}'] != null
pigeonMap['${field.name}'] != null
\t\t? ${field.dataType}.values[pigeonMap['${field.name}']$unwrapOperator as int]
\t\t: null''', leadingSpace: false, trailingNewline: false);
} else if (field.dataType == 'Map' && field.typeArguments != null) {
indent.add(
'(pigeonMap[\'${field.name}\'] as Map<Object$nullTag, Object$nullTag>$nullTag)$nullTag.cast<${_flattenTypeArguments(field.typeArguments!, nullTag)}>()',
);
} else {
indent.add(
'pigeonMap[\'${field.name}\'] as ${_addGenericTypes(field.dataType, nullTag)}',
'pigeonMap[\'${field.name}\'] as ${_addGenericTypes(field, nullTag)}',
);
}
indent.addln(index == klass.fields.length - 1 ? ';' : '');
Expand Down
8 changes: 4 additions & 4 deletions packages/pigeon/lib/generator_tools.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ class HostDatatype {
/// `builtinResolver` will return the host datatype for the Dart datatype for
/// builtin types. `customResolver` can modify the datatype of custom types.
HostDatatype getHostDatatype(Field field, List<Class> classes, List<Enum> enums,
String? Function(String) builtinResolver,
String? Function(Field) builtinResolver,
{String Function(String)? customResolver}) {
final String? datatype = builtinResolver(field.dataType);
final String? datatype = builtinResolver(field);
if (datatype == null) {
if (classes.map((Class x) => x.name).contains(field.dataType)) {
final String customName = customResolver != null
Expand Down Expand Up @@ -285,8 +285,8 @@ const int _minimumCodecFieldKey = 128;
Iterable<EnumeratedClass> getCodecClasses(Api api) sync* {
final Set<String> names = <String>{};
for (final Method method in api.methods) {
names.add(method.returnType);
names.add(method.argType);
names.add(method.returnType.dataType);
names.add(method.argType.dataType);
}
final List<String> sortedNames = names
.where((String element) =>
Expand Down
79 changes: 50 additions & 29 deletions packages/pigeon/lib/java_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,6 @@
import 'ast.dart';
import 'generator_tools.dart';

const Map<String, String> _javaTypeForDartTypeMap = <String, String>{
'bool': 'Boolean',
'int': 'Long',
'String': 'String',
'double': 'Double',
'Uint8List': 'byte[]',
'Int32List': 'int[]',
'Int64List': 'long[]',
'Float64List': 'double[]',
'List': 'List<Object>',
'Map': 'Map<Object, Object>',
};

/// Options that control how Java code will be generated.
class JavaOptions {
/// Creates a [JavaOptions] object
Expand Down Expand Up @@ -137,17 +124,19 @@ void _writeHostApi(Indent indent, Api api) {
indent.write('public interface ${api.name} ');
indent.scoped('{', '}', () {
for (final Method method in api.methods) {
final String argType = _javaTypeForDartTypePassthrough(method.argType);
final String argType =
_javaTypeForDartTypePassthrough(method.argType.dataType);
final String returnType = method.isAsynchronous
? 'void'
: _javaTypeForDartTypePassthrough(method.returnType);
: _javaTypeForDartTypePassthrough(method.returnType.dataType);
final List<String> argSignature = <String>[];
if (method.argType != 'void') {
if (method.argType.dataType != 'void') {
argSignature.add('$argType arg');
}
if (method.isAsynchronous) {
final String returnType =
method.returnType == 'void' ? 'Void' : method.returnType;
final String returnType = method.returnType.dataType == 'void'
? 'Void'
: method.returnType.dataType;
argSignature.add('Result<$returnType> result');
}
indent.writeln('$returnType ${method.name}(${argSignature.join(', ')});');
Expand Down Expand Up @@ -181,9 +170,9 @@ static MessageCodec<Object> getCodec() {
indent.write('channel.setMessageHandler((message, reply) -> ');
indent.scoped('{', '});', () {
final String argType =
_javaTypeForDartTypePassthrough(method.argType);
_javaTypeForDartTypePassthrough(method.argType.dataType);
final String returnType =
_javaTypeForDartTypePassthrough(method.returnType);
_javaTypeForDartTypePassthrough(method.returnType.dataType);
indent.writeln('Map<String, Object> wrapped = new HashMap<>();');
indent.write('try ');
indent.scoped('{', '}', () {
Expand All @@ -200,7 +189,7 @@ static MessageCodec<Object> getCodec() {
}
if (method.isAsynchronous) {
final String resultValue =
method.returnType == 'void' ? 'null' : 'result';
method.returnType.dataType == 'void' ? 'null' : 'result';
methodArgument.add(
'result -> { '
'wrapped.put("${Keys.result}", $resultValue); '
Expand All @@ -212,7 +201,7 @@ static MessageCodec<Object> getCodec() {
'api.${method.name}(${methodArgument.join(', ')})';
if (method.isAsynchronous) {
indent.writeln('$call;');
} else if (method.returnType == 'void') {
} else if (method.returnType.dataType == 'void') {
indent.writeln('$call;');
indent.writeln('wrapped.put("${Keys.result}", null);');
} else {
Expand Down Expand Up @@ -265,12 +254,13 @@ static MessageCodec<Object> getCodec() {
''');
for (final Method func in api.methods) {
final String channelName = makeChannelName(api, func);
final String returnType = func.returnType == 'void'
final String returnType = func.returnType.dataType == 'void'
? 'Void'
: _javaTypeForDartTypePassthrough(func.returnType);
final String argType = _javaTypeForDartTypePassthrough(func.argType);
: _javaTypeForDartTypePassthrough(func.returnType.dataType);
final String argType =
_javaTypeForDartTypePassthrough(func.argType.dataType);
String sendArgument;
if (func.argType == 'void') {
if (func.argType.dataType == 'void') {
indent.write('public void ${func.name}(Reply<$returnType> callback) ');
sendArgument = 'null';
} else {
Expand All @@ -288,7 +278,7 @@ static MessageCodec<Object> getCodec() {
indent.dec();
indent.write('channel.send($sendArgument, channelReply -> ');
indent.scoped('{', '});', () {
if (func.returnType == 'void') {
if (func.returnType.dataType == 'void') {
indent.writeln('callback.reply(null);');
} else {
indent.writeln('@SuppressWarnings("ConstantConditions")');
Expand All @@ -313,8 +303,39 @@ String _makeSetter(Field field) {
return 'set$uppercased';
}

String? _javaTypeForDartType(String datatype) {
return _javaTypeForDartTypeMap[datatype];
/// Converts a [List] of [TypeArgument]s to a comma separated [String] to be
/// used in Java code.
String _flattenTypeArguments(List<TypeArgument> args) {
Comment thread
stuartmorgan-g marked this conversation as resolved.
return args
.map((TypeArgument e) => e.typeArguments == null
? _javaTypeForDartTypePassthrough(e.dataType)
: '${_javaTypeForDartTypePassthrough(e.dataType)}<${_flattenTypeArguments(e.typeArguments!)}>')
.reduce((String value, String element) => '$value, $element');
}

String? _javaTypeForDartType(Field field) {
const Map<String, String> javaTypeForDartTypeMap = <String, String>{
'bool': 'Boolean',
'int': 'Long',
'String': 'String',
'double': 'Double',
'Uint8List': 'byte[]',
'Int32List': 'int[]',
'Int64List': 'long[]',
'Float64List': 'double[]',
'Map': 'Map<Object, Object>',
};
if (javaTypeForDartTypeMap.containsKey(field.dataType)) {
return javaTypeForDartTypeMap[field.dataType];
} else if (field.dataType == 'List') {
Comment thread
stuartmorgan-g marked this conversation as resolved.
if (field.typeArguments == null) {
return 'List<Object>';
} else {
return 'List<${_flattenTypeArguments(field.typeArguments!)}>';
}
} else {
return null;
}
}

String _castObject(
Expand Down
Loading