Skip to content

Pigeon : Support for asynchronous methods/Functions #201

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

Merged
gaaclarke merged 11 commits into from
Sep 29, 2020
Merged

Pigeon : Support for asynchronous methods/Functions #201

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5ade5a5
Basic implementation of async support
xsahil03x Aug 13, 2020
6c18031
Fix minor bugs
xsahil03x Aug 13, 2020
76c4433
Minor improvements
xsahil03x Aug 13, 2020
4dedf30
Ran formatter
xsahil03x Aug 28, 2020
e78437e
Fix tests
xsahil03x Sep 17, 2020
15296a5
[Java] Add basic async support for HostApi.
xsahil03x Sep 17, 2020
f06d842
[HostApi] add async generator tests
xsahil03x Sep 18, 2020
595bc5d
[Dart generator] Fix local_tests
xsahil03x Sep 19, 2020
8f20a39
Added a compilation test.
gaaclarke Sep 29, 2020
666bf38
added some spaces in the java code and updated a docstring
gaaclarke Sep 29, 2020
2ea5e55
updated test for new formatting
gaaclarke Sep 29, 2020
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
5 changes: 4 additions & 1 deletion packages/pigeon/lib/ast.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Node {}
/// Represents a method on an [Api].
class Method extends Node {
/// Parametric constructor for [Method].
Method({this.name, this.returnType, this.argType});
Method({this.name, this.returnType, this.argType, this.isAsynchronous});

/// The name of the method.
String name;
Expand All @@ -27,6 +27,9 @@ class Method extends Node {

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

/// Whether the method is asynchronous or not

@gaaclarke gaaclarke Sep 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/// Whether the receiver of this method is expected to return synchronously or not.

@gaaclarke gaaclarke Sep 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Done.

bool isAsynchronous;
}

/// Represents a collection of [Method]s that are hosted ona given [location].
Expand Down
12 changes: 10 additions & 2 deletions packages/pigeon/lib/dart_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,12 @@ void _writeFlutterApi(Indent indent, Api api,
indent.write('abstract class ${api.name} ');
indent.scoped('{', '}', () {
for (Method func in api.methods) {
final bool isAsync = func.isAsynchronous;
final String returnType =
isAsync ? 'Future<${func.returnType}>' : '${func.returnType}';
final String argSignature =
func.argType == 'void' ? '' : '${func.argType} arg';
indent.writeln('${func.returnType} ${func.name}($argSignature);');
indent.writeln('$returnType ${func.name}($argSignature);');
}
indent.write('static void setup(${api.name} api) ');
indent.scoped('{', '}', () {
Expand All @@ -92,6 +95,7 @@ void _writeFlutterApi(Indent indent, Api api,
indent.scoped('{', '});', () {
final String argType = func.argType;
final String returnType = func.returnType;
final bool isAsync = func.isAsynchronous;
String call;
if (argType == 'void') {
call = 'api.${func.name}()';
Expand All @@ -108,7 +112,11 @@ void _writeFlutterApi(Indent indent, Api api,
indent.writeln('return <dynamic, dynamic>{};');
}
} else {
indent.writeln('final $returnType output = $call;');
if (isAsync) {
indent.writeln('final $returnType output = await $call;');
} else {
indent.writeln('final $returnType output = $call;');
}
const String returnExpresion = 'output._toMap()';
final String returnStatement = isMockHandler
? 'return <dynamic, dynamic>{\'${Keys.result}\': $returnExpresion};'
Expand Down
52 changes: 41 additions & 11 deletions packages/pigeon/lib/java_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,30 @@ class JavaOptions {

void _writeHostApi(Indent indent, Api api) {
assert(api.location == ApiLocation.host);

if (api.methods.any((Method it) => it.isAsynchronous)) {
indent.write('public interface Result<T> ');
indent.scoped('{', '}', () {
indent.writeln('void success(T result);');
});
indent.addln('');
}

indent.writeln(
'/** Generated interface from Pigeon that represents a handler of messages from Flutter.*/');
indent.write('public interface ${api.name} ');
indent.scoped('{', '}', () {
for (Method method in api.methods) {
final String argSignature =
method.argType == 'void' ? '' : '${method.argType} arg';
indent.writeln('${method.returnType} ${method.name}($argSignature);');
final String returnType =
method.isAsynchronous ? 'void' : method.returnType;
final List<String> argSignature = <String>[];
if (method.argType != 'void') {
argSignature.add('${method.argType} arg');
}
if (method.isAsynchronous) {
argSignature.add('Result<${method.returnType}> result');
}
indent.writeln('$returnType ${method.name}(${argSignature.join(',')});');

@gaaclarke gaaclarke Sep 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: argSignature.join(', ')

@gaaclarke gaaclarke Sep 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Done.

}
indent.addln('');
indent.writeln(
Expand Down Expand Up @@ -65,17 +81,26 @@ void _writeHostApi(Indent indent, Api api) {
'HashMap<String, HashMap> wrapped = new HashMap<>();');
indent.write('try ');
indent.scoped('{', '}', () {
String methodArgument;
if (argType == 'void') {
methodArgument = '';
} else {
final List<String> methodArgument = <String>[];
if (argType != 'void') {
indent.writeln('@SuppressWarnings("ConstantConditions")');
indent.writeln(
'$argType input = $argType.fromMap((HashMap)message);');
methodArgument = 'input';
methodArgument.add('input');
}
final String call = 'api.${method.name}($methodArgument)';
if (method.returnType == 'void') {
if (method.isAsynchronous) {
methodArgument.add(
'result -> {'
'wrapped.put("${Keys.result}", result.toMap());'
'reply.reply(wrapped);'
'}',
);
}
final String call =
'api.${method.name}(${methodArgument.join(',')})';
if (method.isAsynchronous) {
indent.writeln('$call;');
} else if (method.returnType == 'void') {
indent.writeln('$call;');
indent.writeln('wrapped.put("${Keys.result}", null);');
} else {
Expand All @@ -88,8 +113,13 @@ void _writeHostApi(Indent indent, Api api) {
indent.scoped('{', '}', () {
indent.writeln(
'wrapped.put("${Keys.error}", wrapError(exception));');
if (method.isAsynchronous) {
indent.writeln('reply.reply(wrapped);');
}
});
indent.writeln('reply.reply(wrapped);');
if (!method.isAsynchronous) {
indent.writeln('reply.reply(wrapped);');
}
});
});
indent.scoped(null, '}', () {
Expand Down
15 changes: 14 additions & 1 deletion packages/pigeon/lib/pigeon_lib.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ const List<String> _validTypes = <String>[
'Map',
];

class _Asynchronous {
const _Asynchronous();
}

/// Metadata to annotate a Api method as asynchronous
const _Asynchronous async = _Asynchronous();

/// Metadata to annotate a Pigeon API implemented by the host-platform.
///
/// The abstract class with this annotation groups a collection of Dart↔host
Expand Down Expand Up @@ -224,14 +231,20 @@ class Pigeon {
final List<Method> functions = <Method>[];
for (DeclarationMirror declaration in apiMirror.declarations.values) {
if (declaration is MethodMirror && !declaration.isConstructor) {
final bool isAsynchronous =

@gaaclarke gaaclarke Sep 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you switch this so that it is reading in the async keyword instead of using a using @async metadata?

@xsahil03x xsahil03x Sep 17, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@gaaclarke by adding an async keyword to the method

@HostApi
abstract class Api {
  Future<Foo> doit() async;
}

will look more like a broken code as it won't compile. Maybe we can try reading Future<> instead ?

@HostApi
abstract class Api {
  Future<Foo> doit();
}

@gaaclarke gaaclarke Sep 17, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you think that looks like broken code? It looks correct to me.

@xsahil03x xsahil03x Sep 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because this doesn't compile. It needs a function body. (Due to async keyword)

@HostApi()
abstract class Api {
  Future<Foo> doit() async;
}

declaration.metadata.any((InstanceMirror it) {
return MirrorSystem.getName(it.type.simpleName) ==
'${async.runtimeType}';

@gaaclarke gaaclarke Sep 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there no way to do equality checks on the type directly instead of resorting to string checks?

});
functions.add(Method()
..name = MirrorSystem.getName(declaration.simpleName)
..argType = declaration.parameters.isEmpty
? 'void'
: MirrorSystem.getName(
declaration.parameters[0].type.simpleName)
..returnType =
MirrorSystem.getName(declaration.returnType.simpleName));
MirrorSystem.getName(declaration.returnType.simpleName)
..isAsynchronous = isAsynchronous);
}
}
final HostApi hostApi = _getHostApi(apiMirror);
Expand Down
133 changes: 123 additions & 10 deletions packages/pigeon/test/dart_generator_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:pigeon/ast.dart';
import 'package:pigeon/dart_generator.dart';
import 'package:pigeon/generator_tools.dart';
import 'package:test/test.dart';
import 'package:pigeon/dart_generator.dart';
import 'package:pigeon/ast.dart';

void main() {
test('gen one class', () {
Expand All @@ -29,7 +29,12 @@ void main() {
test('gen one host api', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(name: 'doSomething', argType: 'Input', returnType: 'Output')
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'Output',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand Down Expand Up @@ -69,7 +74,12 @@ void main() {
test('flutterapi', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
Method(name: 'doSomething', argType: 'Input', returnType: 'Output')
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'Output',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand All @@ -89,7 +99,12 @@ void main() {
test('host void', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(name: 'doSomething', argType: 'Input', returnType: 'void')
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'void',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand All @@ -106,7 +121,12 @@ void main() {
test('flutter void return', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
Method(name: 'doSomething', argType: 'Input', returnType: 'void')
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'void',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand All @@ -124,7 +144,12 @@ void main() {
test('flutter void argument', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
Method(name: 'doSomething', argType: 'void', returnType: 'Output')
Method(
name: 'doSomething',
argType: 'void',
returnType: 'Output',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand All @@ -141,7 +166,12 @@ void main() {
test('host void argument', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(name: 'doSomething', argType: 'void', returnType: 'Output')
Method(
name: 'doSomething',
argType: 'void',
returnType: 'Output',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand All @@ -161,8 +191,18 @@ void main() {
location: ApiLocation.host,
dartHostTestHandler: 'ApiMock',
methods: <Method>[
Method(name: 'doSomething', argType: 'Input', returnType: 'Output'),
Method(name: 'voidReturner', argType: 'Input', returnType: 'void')
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'Output',
isAsynchronous: false,
),
Method(
name: 'voidReturner',
argType: 'Input',
returnType: 'void',
isAsynchronous: false,
)
])
], classes: <Class>[
Class(
Expand Down Expand Up @@ -197,4 +237,77 @@ void main() {
final String code = sink.toString();
expect(code, contains('// @dart = 2.8'));
});

test('gen one async Flutter Api', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'Output',
isAsynchronous: true,
)
])
], classes: <Class>[
Class(
name: 'Input',
fields: <Field>[Field(name: 'input', dataType: 'String')]),
Class(
name: 'Output',
fields: <Field>[Field(name: 'output', dataType: 'String')])
]);
final StringBuffer sink = StringBuffer();
generateDart(root, sink);
final String code = sink.toString();
expect(code, contains('abstract class Api'));
expect(code, contains('Future<Output> doSomething(Input arg);'));
expect(
code, contains('final Output output = await api.doSomething(input);'));
});

test('gen one async Host Api', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(
name: 'doSomething',
argType: 'Input',
returnType: 'Output',
isAsynchronous: true,
)
])
], classes: <Class>[
Class(
name: 'Input',
fields: <Field>[Field(name: 'input', dataType: 'String')]),
Class(
name: 'Output',
fields: <Field>[Field(name: 'output', dataType: 'String')])
]);
final StringBuffer sink = StringBuffer();
generateDart(root, sink);
final String code = sink.toString();
expect(code, contains('class Api'));
expect(code, matches('Output.*doSomething.*Input'));
});

test('async host void argument', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(
name: 'doSomething',
argType: 'void',
returnType: 'Output',
isAsynchronous: true,
)
])
], classes: <Class>[
Class(
name: 'Output',
fields: <Field>[Field(name: 'output', dataType: 'String')]),
]);
final StringBuffer sink = StringBuffer();
generateDart(root, sink);
final String code = sink.toString();
expect(code, matches('channel\.send[(]null[)]'));
});
}
Loading