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] Adds SwiftFunction annotation #2304

Merged
merged 37 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4337cd6
Add SwiftFunction annotation
ailtonvivaz Jul 3, 2022
d3eb40d
Bump version to 3.2.4
ailtonvivaz Jul 3, 2022
2920bbf
Remove unused imports
ailtonvivaz Jul 3, 2022
ac02d63
Improve methods map
ailtonvivaz Jul 5, 2022
f52ab0c
Remove unnecessary print
ailtonvivaz Jul 8, 2022
b5ec8ae
Force cast match of SwiftFunction
ailtonvivaz Jul 9, 2022
ac2b51b
Update packages/pigeon/lib/pigeon_lib.dart
ailtonvivaz Jul 27, 2022
f0bbf86
Merge branch 'main' into swift-function-annotation
ailtonvivaz Jul 27, 2022
5f70ea2
Improve documentation of function to parse method with SwiftFunction
ailtonvivaz Jul 27, 2022
718c1b3
Merge branch 'main' into swift-function-annotation
ailtonvivaz Aug 24, 2022
b034144
Merge branch 'main' into swift-function-annotation
ailtonvivaz Aug 25, 2022
519ff35
Merge branch 'main' into swift-function-annotation
ailtonvivaz Aug 27, 2022
9f86539
Fix some dartdocs
ailtonvivaz Aug 31, 2022
d754a58
Merge branch 'main' into swift-function-annotation
ailtonvivaz Sep 22, 2022
d8bf6d4
Merge branch 'swift-function-annotation' of github.com:ailtonvivaz/pa…
tarrinneal Jan 17, 2023
d4175a9
gen
tarrinneal Jan 17, 2023
09ebdea
analyze
tarrinneal Jan 17, 2023
6db9d1b
Improve SwiftFunction application
ailtonvivaz Jan 18, 2023
22af130
Merge branch 'main' into swift-function-annotation
ailtonvivaz Jan 18, 2023
399e4ae
Add type annotation
ailtonvivaz Jan 18, 2023
608b5ef
format
tarrinneal Jan 18, 2023
4bc40a3
Run format
ailtonvivaz Jan 18, 2023
a8c90f0
Update macos Swift tests
ailtonvivaz Jan 18, 2023
1bdde71
Bump version to 7.0.0
ailtonvivaz Jan 18, 2023
3da3698
Merge branch 'swift-function-annotation' of github.com:ailtonvivaz/pa…
tarrinneal Jan 18, 2023
b13b7ca
revert version change
tarrinneal Jan 18, 2023
ef9b5d0
Improve some code of SwiftGenerator
ailtonvivaz Jan 18, 2023
62b3163
Merge branch 'swift-function-annotation' of github.com:ailtonvivaz/pa…
ailtonvivaz Jan 18, 2023
a44aa18
Bump version to 6.1.0
ailtonvivaz Jan 18, 2023
5aa2197
Improve echo functions for Swift
ailtonvivaz Jan 18, 2023
9176579
Match order of parameters
ailtonvivaz Jan 18, 2023
372d7b7
Documents _SwiftFunctionComponents.fromMethod and _SwiftFunctionArgument
ailtonvivaz Jan 18, 2023
e734c28
Merge branch 'main' into swift-function-annotation
ailtonvivaz Jan 24, 2023
0039165
Improve doc comments
ailtonvivaz Jan 24, 2023
651f9fa
Fix tests
ailtonvivaz Jan 24, 2023
217e4f7
Merge branch 'main' of github.com:flutter/packages into swift-functio…
tarrinneal Jan 25, 2023
245a327
Fix SwiftFunction documentation
ailtonvivaz Jan 26, 2023
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
3 changes: 3 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 3.2.7
* Adds `@SwiftFunction` annotation for specifying custom swift function signature.

## 3.2.6

* [java] Fixes returning int values from FlutterApi methods that fit in 32 bits.
Expand Down
8 changes: 7 additions & 1 deletion packages/pigeon/lib/ast.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Method extends Node {
this.isAsynchronous = false,
this.offset,
this.objcSelector = '',
this.swiftFunction = '',
this.taskQueueType = TaskQueueType.serial,
});

Expand All @@ -51,14 +52,19 @@ class Method extends Node {
/// An override for the generated objc selector (ex. "divideNumber:by:").
String objcSelector;

/// An override for the generated swift function signature (ex. "divideNumber(_:by:)").
String swiftFunction;

/// Specifies how handlers are dispatched with respect to threading.
TaskQueueType taskQueueType;

@override
String toString() {
final String objcSelectorStr =
objcSelector.isEmpty ? '' : ' objcSelector:$objcSelector';
return '(Method name:$name returnType:$returnType arguments:$arguments isAsynchronous:$isAsynchronous$objcSelectorStr)';
final String swiftFunctionStr =
swiftFunction.isEmpty ? '' : ' swiftFunction:$swiftFunction';
return '(Method name:$name returnType:$returnType arguments:$arguments isAsynchronous:$isAsynchronous$objcSelectorStr swiftFuncton:$swiftFunctionStr)';
ailtonvivaz marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'dart:mirrors';
import 'ast.dart';

/// The current version of pigeon. This must match the version in pubspec.yaml.
const String pigeonVersion = '3.2.6';
const String pigeonVersion = '3.2.7';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
32 changes: 32 additions & 0 deletions packages/pigeon/lib/pigeon_lib.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,19 @@ class ObjCSelector {
final String value;
}

/// Metadata to annotate methods to control the signature used for Swift output.
/// The number of components in the provided signature must match the number of
ailtonvivaz marked this conversation as resolved.
Show resolved Hide resolved
/// arguments in the annotated method.
/// For example:
/// @SwiftFunction('setValue(_:for:)') double divide(int value, String key);
ailtonvivaz marked this conversation as resolved.
Show resolved Hide resolved
class SwiftFunction {
/// Constructor.
const SwiftFunction(this.value);

/// The string representation of the function signature.
final String value;
}

/// Type of TaskQueue which determines how handlers are dispatched for
/// HostApi's.
enum TaskQueueType {
Expand Down Expand Up @@ -665,6 +678,17 @@ List<Error> _validateAst(Root root, String source) {
));
}
}
if (method.swiftFunction.isNotEmpty) {
final RegExp signatureRegex =
RegExp('\\w+ *\\((\\w+:){${method.arguments.length}}\\)');
if (!signatureRegex.hasMatch(method.swiftFunction)) {
result.add(Error(
message:
'Invalid function signature, expected ${method.arguments.length} arguments.',
lineNumber: _calculateLineNumberNullable(source, method.offset),
));
}
}
if (method.taskQueueType != TaskQueueType.serial &&
api.location != ApiLocation.host) {
result.add(Error(
Expand Down Expand Up @@ -947,6 +971,13 @@ class _RootBuilder extends dart_ast_visitor.RecursiveAstVisitor<Object?> {
.asNullable<dart_ast.SimpleStringLiteral>()
?.value ??
'';
final String swiftFunction = _findMetadata(node.metadata, 'SwiftFunction')
?.arguments
?.arguments
.first
.asNullable<dart_ast.SimpleStringLiteral>()
?.value ??
'';
final dart_ast.ArgumentList? taskQueueArguments =
_findMetadata(node.metadata, 'TaskQueue')?.arguments;
final String? taskQueueTypeName = taskQueueArguments == null
Expand All @@ -973,6 +1004,7 @@ class _RootBuilder extends dart_ast_visitor.RecursiveAstVisitor<Object?> {
arguments: arguments,
isAsynchronous: isAsynchronous,
objcSelector: objcSelector,
swiftFunction: swiftFunction,
offset: node.offset,
taskQueueType: taskQueueType));
} else if (_currentClass != null) {
Expand Down
41 changes: 41 additions & 0 deletions packages/pigeon/lib/swift_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,44 @@ class SwiftOptions {
/// Calculates the name of the codec that will be generated for [api].
String _getCodecName(Api api) => '${api.name}Codec';

/// Returns a new [Method] using the swift function signature from [func],
/// ie the name of function e the strings between the semicolons.
ailtonvivaz marked this conversation as resolved.
Show resolved Hide resolved
ailtonvivaz marked this conversation as resolved.
Show resolved Hide resolved
/// Example:
ailtonvivaz marked this conversation as resolved.
Show resolved Hide resolved
/// _swiftMethod('@SwiftFunction('add(_:with:)') void add(int x, int y)')
/// will return 'void add(int _ x, int with y)'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remap the Dart APIs to a hybrid string that will never be used (and has no meaning in either language), only to re-extract the components later, instead of doing the mapping at the point of constructing the Swift code (as is done in the ObjC generator)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed this approach to minimize the changes. But, if this is not the better way, I could change to be similar with ObjC.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having this intermediate format will likely make it harder to understand what's happening, and there's more potential for strange side effects (e.g., some new code trying to use the methods, and it not being obvious why they don't match the original source), so I'm concerned about the maintenance impact. I would much rather this transformation happen at the usage point even if it's more code in the short term.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree. I'll work on this update.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ailtonvivaz What is the status of this update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this. Is this approach better?

/// which represents 'func add(_ x: Int32, with y: Int32) -> Void' in Swift.
Method _swiftMethod(Method func) {
if (func.swiftFunction.isEmpty) {
return func;
} else {
final String argsCapturator =
repeat(r'(\w+):', func.arguments.length).join();
final RegExp signatureRegex = RegExp(r'(\w+) *\(' + argsCapturator + r'\)');
final RegExpMatch match = signatureRegex.firstMatch(func.swiftFunction)!;

final Iterable<String> customComponents = match
.groups(
List<int>.generate(func.arguments.length, (int index) => index + 2))
.whereType();

return Method(
name: match.group(1)!,
returnType: func.returnType,
arguments:
map2(func.arguments, customComponents, (NamedType t, String u) {
if (u == t.name) {
return t;
}

return NamedType(name: '$u ${t.name}', type: t.type);
}).toList(),
isAsynchronous: func.isAsynchronous,
offset: func.offset,
taskQueueType: func.taskQueueType,
);
}
}

/// Writes the codec classwill be used for encoding messages for the [api].
/// Example:
/// private class FooHostApiCodecReader: FlutterStandardReader {...}
Expand Down Expand Up @@ -602,6 +640,9 @@ void generateSwift(SwiftOptions options, Root root, StringSink sink) {
for (final Api api in root.apis) {
_writeCodec(indent, api, root);
indent.addln('');

api.methods = api.methods.map(_swiftMethod).toList();

writeApi(api, root);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: pigeon
description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
repository: https://github.com/flutter/packages/tree/main/packages/pigeon
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3Apigeon
version: 3.2.6 # This must match the version in lib/generator_tools.dart
version: 3.2.7 # This must match the version in lib/generator_tools.dart

environment:
sdk: ">=2.12.0 <3.0.0"
Expand Down
46 changes: 46 additions & 0 deletions packages/pigeon/test/pigeon_lib_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,52 @@ abstract class Api {
expect(results.root.apis[0].methods[0].objcSelector, equals('foobar'));
});

test('custom swift invalid function signature', () {
const String code = '''
@HostApi()
abstract class Api {
@SwiftFunction('subtractValue(_:by:)')
void subtract(int x, int y);
}
''';
final ParseResults results = _parseSource(code);
expect(results.errors.length, 0);
expect(results.root.apis.length, 1);
expect(results.root.apis[0].methods.length, equals(1));
expect(results.root.apis[0].methods[0].swiftFunction,
equals('subtractValue(_:by:)'));
});

test('custom swift invalid function signature', () {
const String code = '''
@HostApi()
abstract class Api {
@SwiftFunction('subtractValue(_:by:error:)')
void subtract(int x, int y);
}
''';
final ParseResults results = _parseSource(code);
expect(results.errors.length, 1);
expect(results.errors[0].lineNumber, 3);
expect(results.errors[0].message,
contains('Invalid function signature, expected 2 arguments'));
});

test('custom swift function signature no arguments', () {
const String code = '''
@HostApi()
abstract class Api {
@SwiftFunction('foobar()')
void initialize();
}
''';
final ParseResults results = _parseSource(code);
expect(results.errors.length, 0);
expect(results.root.apis.length, 1);
expect(results.root.apis[0].methods.length, equals(1));
expect(results.root.apis[0].methods[0].swiftFunction, equals('foobar()'));
});

test('dart test has copyright', () {
final Root root = Root(apis: <Api>[], classes: <Class>[], enums: <Enum>[]);
const PigeonOptions options = PigeonOptions(
Expand Down
95 changes: 95 additions & 0 deletions packages/pigeon/test/swift_generator_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -994,4 +994,99 @@ void main() {
final String code = sink.toString();
expect(code, contains('var input: String\n'));
});

test('swift function signature', () {
final Root root = Root(
apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(
name: 'set',
arguments: <NamedType>[
NamedType(
type: const TypeDeclaration(
baseName: 'int',
isNullable: false,
),
name: 'value',
offset: null,
),
NamedType(
type: const TypeDeclaration(
baseName: 'String',
isNullable: false,
),
name: 'key',
offset: null,
),
],
swiftFunction: 'setValue(_:for:)',
returnType: const TypeDeclaration.voidDeclaration(),
isAsynchronous: false,
)
])
],
classes: <Class>[],
enums: <Enum>[],
);
final StringBuffer sink = StringBuffer();
const SwiftOptions swiftOptions = SwiftOptions();
generateSwift(swiftOptions, root, sink);
final String code = sink.toString();
expect(code, contains('func setValue(_ value: Int32, for key: String)'));
});

test('swift function signature with same name argument', () {
final Root root = Root(
apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(
name: 'set',
arguments: <NamedType>[
NamedType(
type: const TypeDeclaration(
baseName: 'String',
isNullable: false,
),
name: 'key',
offset: null,
),
],
swiftFunction: 'removeValue(key:)',
returnType: const TypeDeclaration.voidDeclaration(),
isAsynchronous: false,
)
])
],
classes: <Class>[],
enums: <Enum>[],
);
final StringBuffer sink = StringBuffer();
const SwiftOptions swiftOptions = SwiftOptions();
generateSwift(swiftOptions, root, sink);
final String code = sink.toString();
expect(code, contains('func removeValue(key: String)'));
});

test('swift function signature with no arguments', () {
final Root root = Root(
apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(
name: 'clear',
arguments: <NamedType>[],
swiftFunction: 'removeAll()',
returnType: const TypeDeclaration.voidDeclaration(),
isAsynchronous: false,
)
])
],
classes: <Class>[],
enums: <Enum>[],
);
final StringBuffer sink = StringBuffer();
const SwiftOptions swiftOptions = SwiftOptions();
generateSwift(swiftOptions, root, sink);
final String code = sink.toString();
expect(code, contains('func removeAll()'));
});
}