Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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: 5 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 3.0.2

* Fixes non-nullable classes and enums as fields.
* Fixes nullable collections as return types.

## 3.0.0

* **BREAKING CHANGE**: Removes the `--dart_null_safety` flag. Generated Dart
Expand Down
34 changes: 25 additions & 9 deletions packages/pigeon/lib/dart_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ final BinaryMessenger? _binaryMessenger;
final String returnType = _makeGenericTypeArguments(func.returnType);
final String castCall = _makeGenericCastCall(func.returnType);
const String accessor = 'replyMap[\'${Keys.result}\']';
final String unwrapper = func.returnType.isNullable ? '' : '!';
final String unwrapper =
func.returnType.isNullable ? (castCall.isEmpty ? '' : '?') : '!';
final String returnStatement = func.returnType.isVoid
? 'return;'
: 'return ($accessor as $returnType?)$unwrapper$castCall;';
Expand Down Expand Up @@ -459,13 +460,14 @@ void generateDart(DartOptions opt, Root root, StringSink sink) {
);
for (final NamedType field in klass.fields) {
indent.write('pigeonMap[\'${field.name}\'] = ');
final String conditional = field.type.isNullable ? '?' : '';
if (customClassNames.contains(field.type.baseName)) {
indent.addln(
'${field.name}?.encode();',
'${field.name}$conditional.encode();',
);
} else if (customEnumNames.contains(field.type.baseName)) {
indent.addln(
'${field.name}?.index;',
'${field.name}$conditional.index;',
);
} else {
indent.addln('${field.name};');
Expand All @@ -478,15 +480,29 @@ void generateDart(DartOptions opt, Root root, StringSink sink) {
void writeDecode() {
void writeValueDecode(NamedType field) {
if (customClassNames.contains(field.type.baseName)) {
indent.format('''
final String nonNullValue =
"${field.type.baseName}.decode(pigeonMap['${field.name}']!)";
indent.format(
field.type.isNullable
? '''
pigeonMap['${field.name}'] != null
\t\t? ${field.type.baseName}.decode(pigeonMap['${field.name}']!)
\t\t: null''', leadingSpace: false, trailingNewline: false);
\t\t? $nonNullValue
\t\t: null'''
: nonNullValue,
leadingSpace: false,
trailingNewline: false);
} else if (customEnumNames.contains(field.type.baseName)) {
indent.format('''
final String nonNullValue =
"${field.type.baseName}.values[pigeonMap['${field.name}']! as int]";
indent.format(
field.type.isNullable
? '''
pigeonMap['${field.name}'] != null
\t\t? ${field.type.baseName}.values[pigeonMap['${field.name}']! as int]
\t\t: null''', leadingSpace: false, trailingNewline: false);
\t\t? $nonNullValue
\t\t: null'''
: nonNullValue,
leadingSpace: false,
trailingNewline: false);
} else if (field.type.typeArguments.isNotEmpty) {
final String genericType = _makeGenericTypeArguments(field.type);
final String castCall = _makeGenericCastCall(field.type);
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 @@ -8,7 +8,7 @@ import 'dart:mirrors';
import 'ast.dart';

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

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
12 changes: 11 additions & 1 deletion packages/pigeon/pigeons/non_null_fields.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,21 @@ class SearchRequest {
String query;
}

class ExtraData {
ExtraData({required this.detailA, required this.detailB});
String detailA;
String detailB;
}

enum ReplyType { success, error }

class SearchReply {
SearchReply(this.result, this.error, this.indices);
SearchReply(this.result, this.error, this.indices, this.extraData, this.type);
String result;
String error;
List<int?> indices;
ExtraData extraData;
ReplyType type;
}

@HostApi()
Expand Down
20 changes: 20 additions & 0 deletions packages/pigeon/pigeons/nullable_returns.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,23 @@ abstract class NullableArgHostApi {
abstract class NullableArgFlutterApi {
int doit(int? x);
}

@HostApi()
abstract class NonNullCollectionHostApi {
List<String?>? doit();
}
Copy link
Member

Choose a reason for hiding this comment

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

Adding this here is making sure that the generated code compiles and the dart unit tests below make sure it is generating code as you expect it. But we aren't executing the generated code in the platform tests. Did you want to take a stab at that? That could be added here: https://github.com/flutter/packages/blob/master/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/test/null_safe_test.dart

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests. I noticed that there were also no platform tests of the existing non-collection null return handling, so I added that too.

This spawned a few other changes:

  • Renamed the null-return-type API classes in the Pigeon file, because they made no sense. Originally I had just copied the non-collection versions and hadn't paid close attention to the names, but when using them in the tests they were actively misleading, and it seemed pretty clear once I looked at them again that they were copypasta'd from another pigeons/ file that was testing something completely different.
  • Updated build_runner since I couldn't run the current version (to generate new mocks).

I'm now including the updated generated files, since otherwise the mocks and mock annotations that are now in this PR would refer to things that weren't in the versions of the files checked into the tree, which would be really weird.


@FlutterApi()
abstract class NonNullCollectionFlutterApi {
List<String?>? doit();
}

@HostApi()
abstract class NullableCollectionArgHostApi {
List<String?> doit(List<String?>? x);
}

@FlutterApi()
abstract class NullableCollectionArgFlutterApi {
List<String?> doit(List<String?>? x);
}
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.0.0 # This must match the version in lib/generator_tools.dart
version: 3.0.2 # This must match the version in lib/generator_tools.dart

environment:
sdk: ">=2.12.0 <3.0.0"
Expand Down
117 changes: 117 additions & 0 deletions packages/pigeon/test/dart_generator_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,50 @@ void main() {
);
});

test('nested non-nullable class', () {
final Root root = Root(apis: <Api>[], classes: <Class>[
Class(
name: 'Input',
fields: <NamedType>[
NamedType(
type: const TypeDeclaration(
baseName: 'String',
isNullable: false,
),
name: 'input',
offset: null)
],
),
Class(
name: 'Nested',
fields: <NamedType>[
NamedType(
type: const TypeDeclaration(
baseName: 'Input',
isNullable: false,
),
name: 'nested',
offset: null)
],
)
], enums: <Enum>[]);
final StringBuffer sink = StringBuffer();
generateDart(const DartOptions(), root, sink);
final String code = sink.toString();
expect(
code,
contains(
'pigeonMap[\'nested\'] = nested.encode()',
),
);
expect(
code.replaceAll('\n', ' ').replaceAll(' ', ''),
contains(
'nested: Input.decode(pigeonMap[\'nested\']!)',
),
);
});

test('flutterapi', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
Expand Down Expand Up @@ -401,6 +445,51 @@ void main() {
expect(code, contains('EnumClass doSomething(EnumClass arg0);'));
});

test('flutter non-nullable enum argument with enum class', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
Method(
name: 'doSomething',
arguments: <NamedType>[
NamedType(
type: const TypeDeclaration(
baseName: 'EnumClass',
isNullable: false,
),
name: '',
offset: null)
],
returnType:
const TypeDeclaration(baseName: 'EnumClass', isNullable: false),
isAsynchronous: false,
)
])
], classes: <Class>[
Class(name: 'EnumClass', fields: <NamedType>[
NamedType(
type: const TypeDeclaration(
baseName: 'Enum',
isNullable: false,
),
name: 'enum1',
offset: null)
]),
], enums: <Enum>[
Enum(
name: 'Enum',
members: <String>[
'one',
'two',
],
)
]);
final StringBuffer sink = StringBuffer();
generateDart(const DartOptions(), root, sink);
final String code = sink.toString();
expect(code, contains('pigeonMap[\'enum1\'] = enum1.index;'));
expect(code, contains('enum1: Enum.values[pigeonMap[\'enum1\']! as int]'));
});

test('host void argument', () {
final Root root = Root(apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Expand Down Expand Up @@ -893,6 +982,34 @@ void main() {
expect(code, contains('return (replyMap[\'result\'] as int?);'));
});

test('return nullable collection host', () {
final Root root = Root(
apis: <Api>[
Api(name: 'Api', location: ApiLocation.host, methods: <Method>[
Method(
name: 'doit',
returnType: const TypeDeclaration(
baseName: 'List',
isNullable: true,
typeArguments: <TypeDeclaration>[
TypeDeclaration(baseName: 'int', isNullable: true)
]),
arguments: <NamedType>[])
])
],
classes: <Class>[],
enums: <Enum>[],
);
final StringBuffer sink = StringBuffer();
generateDart(const DartOptions(), root, sink);
final String code = sink.toString();
expect(code, contains('Future<List<int?>?> doit()'));
expect(
code,
contains(
'return (replyMap[\'result\'] as List<Object?>?)?.cast<int?>();'));
});

test('return nullable async host', () {
final Root root = Root(
apis: <Api>[
Expand Down