[pigeon] fixed nested classes type arguments can be omitted from the output (#471)
diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md
index 02c3121..9181db8 100644
--- a/packages/pigeon/CHANGELOG.md
+++ b/packages/pigeon/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.0.7
+
+* [front-end] Fixed bug where nested classes' type arguments aren't included in
+ the output (generated class and codec).
+
## 1.0.6
* Updated example README for set up steps.
@@ -22,6 +27,7 @@
explicitly.
## 1.0.1
+
* [front-end] Fixed bug where classes only referenced as type arguments for
generics weren't being generated.
diff --git a/packages/pigeon/lib/dart_generator.dart b/packages/pigeon/lib/dart_generator.dart
index 7a839b3..79711e2 100644
--- a/packages/pigeon/lib/dart_generator.dart
+++ b/packages/pigeon/lib/dart_generator.dart
@@ -52,15 +52,15 @@
String _getCodecName(Api api) => '_${api.name}Codec';
-void _writeCodec(Indent indent, String codecName, Api api) {
+void _writeCodec(Indent indent, String codecName, Api api, Root root) {
indent.write('class $codecName extends StandardMessageCodec ');
indent.scoped('{', '}', () {
indent.writeln('const $codecName();');
- if (getCodecClasses(api).isNotEmpty) {
+ if (getCodecClasses(api, root).isNotEmpty) {
indent.writeln('@override');
indent.write('void writeValue(WriteBuffer buffer, Object? value) ');
indent.scoped('{', '}', () {
- for (final EnumeratedClass customClass in getCodecClasses(api)) {
+ for (final EnumeratedClass customClass in getCodecClasses(api, root)) {
indent.write('if (value is ${customClass.name}) ');
indent.scoped('{', '} else ', () {
indent.writeln('buffer.putUint8(${customClass.enumeration});');
@@ -76,7 +76,8 @@
indent.scoped('{', '}', () {
indent.write('switch (type) ');
indent.scoped('{', '}', () {
- for (final EnumeratedClass customClass in getCodecClasses(api)) {
+ for (final EnumeratedClass customClass
+ in getCodecClasses(api, root)) {
indent.write('case ${customClass.enumeration}: ');
indent.writeScoped('', '', () {
indent.writeln(
@@ -129,10 +130,10 @@
}).join(', ');
}
-void _writeHostApi(DartOptions opt, Indent indent, Api api) {
+void _writeHostApi(DartOptions opt, Indent indent, Api api, Root root) {
assert(api.location == ApiLocation.host);
final String codecName = _getCodecName(api);
- _writeCodec(indent, codecName, api);
+ _writeCodec(indent, codecName, api, root);
indent.addln('');
final String nullTag = opt.isNullSafe ? '?' : '';
final String unwrapOperator = opt.isNullSafe ? '!' : '';
@@ -209,13 +210,14 @@
void _writeFlutterApi(
DartOptions opt,
Indent indent,
- Api api, {
+ Api api,
+ Root root, {
String Function(Method)? channelNameFunc,
bool isMockHandler = false,
}) {
assert(api.location == ApiLocation.flutter);
final String codecName = _getCodecName(api);
- _writeCodec(indent, codecName, api);
+ _writeCodec(indent, codecName, api, root);
final String nullTag = opt.isNullSafe ? '?' : '';
final String unwrapOperator = opt.isNullSafe ? '!' : '';
indent.write('abstract class ${api.name} ');
@@ -467,9 +469,9 @@
for (final Api api in root.apis) {
indent.writeln('');
if (api.location == ApiLocation.host) {
- _writeHostApi(opt, indent, api);
+ _writeHostApi(opt, indent, api, root);
} else if (api.location == ApiLocation.flutter) {
- _writeFlutterApi(opt, indent, api);
+ _writeFlutterApi(opt, indent, api, root);
}
}
}
@@ -517,6 +519,7 @@
opt,
indent,
mockApi,
+ root,
channelNameFunc: (Method func) => makeChannelName(api, func),
isMockHandler: true,
);
diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart
index 92e6bda..cb74607 100644
--- a/packages/pigeon/lib/generator_tools.dart
+++ b/packages/pigeon/lib/generator_tools.dart
@@ -8,7 +8,7 @@
import 'ast.dart';
/// The current version of pigeon. This must match the version in pubspec.yaml.
-const String pigeonVersion = '1.0.6';
+const String pigeonVersion = '1.0.7';
/// Read all the content from [stdin] to a String.
String readStdin() {
@@ -154,10 +154,11 @@
final bool isBuiltin;
}
-/// Calculates the [HostDatatype] for the provided [NamedType]. It will check the
-/// field against the `classes` to check if it is a builtin type.
-/// `builtinResolver` will return the host datatype for the Dart datatype for
-/// builtin types. `customResolver` can modify the datatype of custom types.
+/// Calculates the [HostDatatype] for the provided [NamedType]. It will check
+/// the field against [classes], the list of custom classes, to check if it is a
+/// builtin type. [builtinResolver] will return the host datatype for the Dart
+/// datatype for builtin types. [customResolver] can modify the datatype of
+/// custom types.
HostDatatype getHostDatatype(NamedType field, List<Class> classes,
List<Enum> enums, String? Function(NamedType) builtinResolver,
{String Function(String)? customResolver}) {
@@ -280,26 +281,86 @@
/// avoid collisions with the StandardMessageCodec.
const int _minimumCodecFieldKey = 128;
-Iterable<String> _getReferencedTypes(TypeDeclaration type) sync* {
+Iterable<TypeDeclaration> _getTypeArguments(TypeDeclaration type) sync* {
for (final TypeDeclaration typeArg in type.typeArguments) {
- yield* _getReferencedTypes(typeArg);
+ yield* _getTypeArguments(typeArg);
}
- yield type.baseName;
+ yield type;
+}
+
+bool _isUnseenCustomType(
+ TypeDeclaration type, Set<String> referencedTypeNames) {
+ return !referencedTypeNames.contains(type.baseName) &&
+ !validTypes.contains(type.baseName);
+}
+
+class _Bag<Key, Value> {
+ Map<Key, List<Value>> map = <Key, List<Value>>{};
+ void add(Key key, Value? value) {
+ if (!map.containsKey(key)) {
+ map[key] = value == null ? <Value>[] : <Value>[value];
+ } else {
+ if (value != null) {
+ map[key]!.add(value);
+ }
+ }
+ }
+
+ void addMany(Iterable<Key> keys, Value? value) {
+ for (final Key key in keys) {
+ add(key, value);
+ }
+ }
+}
+
+/// Recurses into a list of [Api]s and produces a list of all referenced types
+/// and an associated [List] of the offsets where they are found.
+Map<TypeDeclaration, List<int>> getReferencedTypes(
+ List<Api> apis, List<Class> classes) {
+ final _Bag<TypeDeclaration, int> references = _Bag<TypeDeclaration, int>();
+ for (final Api api in apis) {
+ for (final Method method in api.methods) {
+ for (final NamedType field in method.arguments) {
+ references.addMany(_getTypeArguments(field.type), field.offset);
+ }
+ references.addMany(_getTypeArguments(method.returnType), method.offset);
+ }
+ }
+
+ final Set<String> referencedTypeNames =
+ references.map.keys.map((TypeDeclaration e) => e.baseName).toSet();
+ final List<String> classesToCheck = List<String>.from(referencedTypeNames);
+ while (classesToCheck.isNotEmpty) {
+ final String next = classesToCheck.removeLast();
+ final Class aClass = classes.firstWhere((Class x) => x.name == next,
+ orElse: () => Class(name: '', fields: <NamedType>[]));
+ for (final NamedType field in aClass.fields) {
+ if (_isUnseenCustomType(field.type, referencedTypeNames)) {
+ references.add(field.type, field.offset);
+ classesToCheck.add(field.type.baseName);
+ }
+ for (final TypeDeclaration typeArg in field.type.typeArguments) {
+ if (_isUnseenCustomType(typeArg, referencedTypeNames)) {
+ references.add(typeArg, field.offset);
+ classesToCheck.add(typeArg.baseName);
+ }
+ }
+ }
+ }
+ return references.map;
}
/// Given an [Api], return the enumerated classes that must exist in the codec
/// where the enumeration should be the key used in the buffer.
-Iterable<EnumeratedClass> getCodecClasses(Api api) sync* {
- final Set<String> names = <String>{};
- for (final Method method in api.methods) {
- names.addAll(_getReferencedTypes(method.returnType));
- for (final NamedType argument in method.arguments) {
- names.addAll(_getReferencedTypes(argument.type));
- }
- }
- final List<String> sortedNames = names
+Iterable<EnumeratedClass> getCodecClasses(Api api, Root root) sync* {
+ final Set<String> enumNames = root.enums.map((Enum e) => e.name).toSet();
+ final List<String> sortedNames = getReferencedTypes(<Api>[api], root.classes)
+ .keys
+ .map((TypeDeclaration e) => e.baseName)
.where((String element) =>
- element != 'void' && !validTypes.contains(element))
+ element != 'void' &&
+ !validTypes.contains(element) &&
+ !enumNames.contains(element))
.toList();
sortedNames.sort();
int enumeration = _minimumCodecFieldKey;
diff --git a/packages/pigeon/lib/java_generator.dart b/packages/pigeon/lib/java_generator.dart
index f239989..2bf2001 100644
--- a/packages/pigeon/lib/java_generator.dart
+++ b/packages/pigeon/lib/java_generator.dart
@@ -55,21 +55,22 @@
String _getCodecName(Api api) => '${api.name}Codec';
-void _writeCodec(Indent indent, Api api) {
+void _writeCodec(Indent indent, Api api, Root root) {
final String codecName = _getCodecName(api);
indent.write('private static class $codecName extends StandardMessageCodec ');
indent.scoped('{', '}', () {
indent
.writeln('public static final $codecName INSTANCE = new $codecName();');
indent.writeln('private $codecName() {}');
- if (getCodecClasses(api).isNotEmpty) {
+ if (getCodecClasses(api, root).isNotEmpty) {
indent.writeln('@Override');
indent.write(
'protected Object readValueOfType(byte type, ByteBuffer buffer) ');
indent.scoped('{', '}', () {
indent.write('switch (type) ');
indent.scoped('{', '}', () {
- for (final EnumeratedClass customClass in getCodecClasses(api)) {
+ for (final EnumeratedClass customClass
+ in getCodecClasses(api, root)) {
indent.write('case (byte)${customClass.enumeration}: ');
indent.writeScoped('', '', () {
indent.writeln(
@@ -86,7 +87,7 @@
indent.write(
'protected void writeValue(ByteArrayOutputStream stream, Object value) ');
indent.writeScoped('{', '}', () {
- for (final EnumeratedClass customClass in getCodecClasses(api)) {
+ for (final EnumeratedClass customClass in getCodecClasses(api, root)) {
indent.write('if (value instanceof ${customClass.name}) ');
indent.scoped('{', '} else ', () {
indent.writeln('stream.write(${customClass.enumeration});');
@@ -517,7 +518,7 @@
}
for (final Api api in root.apis) {
- _writeCodec(indent, api);
+ _writeCodec(indent, api, root);
indent.addln('');
if (api.location == ApiLocation.host) {
_writeHostApi(indent, api);
diff --git a/packages/pigeon/lib/objc_generator.dart b/packages/pigeon/lib/objc_generator.dart
index c32b640..1d21c5e 100644
--- a/packages/pigeon/lib/objc_generator.dart
+++ b/packages/pigeon/lib/objc_generator.dart
@@ -161,19 +161,20 @@
String _getCodecGetterName(String? prefix, String className) =>
'${_className(prefix, className)}GetCodec';
-void _writeCodec(Indent indent, String name, ObjcOptions options, Api api) {
+void _writeCodec(
+ Indent indent, String name, ObjcOptions options, Api api, Root root) {
final String readerWriterName = '${name}ReaderWriter';
final String readerName = '${name}Reader';
final String writerName = '${name}Writer';
indent.writeln('@interface $readerName : FlutterStandardReader');
indent.writeln('@end');
indent.writeln('@implementation $readerName');
- if (getCodecClasses(api).isNotEmpty) {
+ if (getCodecClasses(api, root).isNotEmpty) {
indent.writeln('- (nullable id)readValueOfType:(UInt8)type ');
indent.scoped('{', '}', () {
indent.write('switch (type) ');
indent.scoped('{', '}', () {
- for (final EnumeratedClass customClass in getCodecClasses(api)) {
+ for (final EnumeratedClass customClass in getCodecClasses(api, root)) {
indent.write('case ${customClass.enumeration}: ');
indent.writeScoped('', '', () {
indent.writeln(
@@ -192,10 +193,10 @@
indent.writeln('@interface $writerName : FlutterStandardWriter');
indent.writeln('@end');
indent.writeln('@implementation $writerName');
- if (getCodecClasses(api).isNotEmpty) {
+ if (getCodecClasses(api, root).isNotEmpty) {
indent.writeln('- (void)writeValue:(id)value ');
indent.scoped('{', '}', () {
- for (final EnumeratedClass customClass in getCodecClasses(api)) {
+ for (final EnumeratedClass customClass in getCodecClasses(api, root)) {
indent.write(
'if ([value isKindOfClass:[${_className(options.prefix, customClass.name)} class]]) ');
indent.scoped('{', '} else ', () {
@@ -722,7 +723,7 @@
for (final Api api in root.apis) {
final String codecName = _getCodecName(options.prefix, api.name);
- _writeCodec(indent, codecName, options, api);
+ _writeCodec(indent, codecName, options, api, root);
indent.addln('');
if (api.location == ApiLocation.host) {
_writeHostApiSource(indent, options, api);
diff --git a/packages/pigeon/lib/pigeon_lib.dart b/packages/pigeon/lib/pigeon_lib.dart
index c286157..f34bb51 100644
--- a/packages/pigeon/lib/pigeon_lib.dart
+++ b/packages/pigeon/lib/pigeon_lib.dart
@@ -504,13 +504,6 @@
}
}
-Iterable<String> _getReferencedTypes(TypeDeclaration type) sync* {
- for (final TypeDeclaration typeArg in type.typeArguments) {
- yield* _getReferencedTypes(typeArg);
- }
- yield type.baseName;
-}
-
class _RootBuilder extends dart_ast_visitor.RecursiveAstVisitor<Object?> {
_RootBuilder(this.source);
@@ -542,38 +535,17 @@
_storeCurrentApi();
_storeCurrentClass();
- final Set<String> referencedTypes = <String>{};
- for (final Api api in _apis) {
- for (final Method method in api.methods) {
- for (final NamedType field in method.arguments) {
- referencedTypes.addAll(_getReferencedTypes(field.type));
- }
- referencedTypes.addAll(_getReferencedTypes(method.returnType));
- }
- }
-
- final List<String> classesToCheck = List<String>.from(referencedTypes);
- while (classesToCheck.isNotEmpty) {
- final String next = classesToCheck.last;
- classesToCheck.removeLast();
- final Class aClass = _classes.firstWhere((Class x) => x.name == next,
- orElse: () => Class(name: '', fields: <NamedType>[]));
- for (final NamedType field in aClass.fields) {
- if (!referencedTypes.contains(field.type.baseName) &&
- !validTypes.contains(field.type.baseName)) {
- referencedTypes.add(field.type.baseName);
- classesToCheck.add(field.type.baseName);
- }
- }
- }
-
+ final Map<TypeDeclaration, List<int>> referencedTypes =
+ getReferencedTypes(_apis, _classes);
+ final Set<String> referencedTypeNames =
+ referencedTypes.keys.map((TypeDeclaration e) => e.baseName).toSet();
final List<Class> referencedClasses = List<Class>.from(_classes);
referencedClasses
- .removeWhere((Class x) => !referencedTypes.contains(x.name));
+ .removeWhere((Class x) => !referencedTypeNames.contains(x.name));
final List<Enum> referencedEnums = List<Enum>.from(_enums);
referencedEnums.removeWhere(
- (final Enum anEnum) => !referencedTypes.contains(anEnum.name));
+ (final Enum anEnum) => !referencedTypeNames.contains(anEnum.name));
final Root completeRoot =
Root(apis: _apis, classes: referencedClasses, enums: referencedEnums);
@@ -582,6 +554,27 @@
final List<Error> totalErrors = List<Error>.from(_errors);
totalErrors.addAll(validateErrors);
+ for (final MapEntry<TypeDeclaration, List<int>> element
+ in referencedTypes.entries) {
+ if (!referencedClasses
+ .map((Class e) => e.name)
+ .contains(element.key.baseName) &&
+ !referencedEnums
+ .map((Enum e) => e.name)
+ .contains(element.key.baseName) &&
+ !validTypes.contains(element.key.baseName) &&
+ !element.key.isVoid &&
+ element.key.baseName != 'dynamic' &&
+ element.key.baseName.isNotEmpty) {
+ final int? lineNumber = element.value.isEmpty
+ ? null
+ : _calculateLineNumber(source, element.value.first);
+ totalErrors.add(Error(
+ message: 'Unknown type: ${element.key.baseName}',
+ lineNumber: lineNumber));
+ }
+ }
+
return ParseResults(
root: totalErrors.isEmpty
? completeRoot
diff --git a/packages/pigeon/pubspec.yaml b/packages/pigeon/pubspec.yaml
index 86c1a4f..52526a3 100644
--- a/packages/pigeon/pubspec.yaml
+++ b/packages/pigeon/pubspec.yaml
@@ -2,7 +2,7 @@
description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
repository: https://github.com/flutter/packages/tree/master/packages/pigeon
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3Apigeon
-version: 1.0.6 # This must match the version in lib/generator_tools.dart
+version: 1.0.7 # This must match the version in lib/generator_tools.dart
environment:
sdk: '>=2.12.0 <3.0.0'
diff --git a/packages/pigeon/test/generator_tools_test.dart b/packages/pigeon/test/generator_tools_test.dart
index ea6bb90..6daaf0c 100644
--- a/packages/pigeon/test/generator_tools_test.dart
+++ b/packages/pigeon/test/generator_tools_test.dart
@@ -87,7 +87,9 @@
isAsynchronous: true,
)
]);
- final List<EnumeratedClass> classes = getCodecClasses(api).toList();
+ final Root root =
+ Root(classes: <Class>[], apis: <Api>[api], enums: <Enum>[]);
+ final List<EnumeratedClass> classes = getCodecClasses(api, root).toList();
expect(classes.length, 2);
expect(
classes
@@ -123,7 +125,9 @@
isAsynchronous: true,
)
]);
- final List<EnumeratedClass> classes = getCodecClasses(api).toList();
+ final Root root =
+ Root(classes: <Class>[], apis: <Api>[api], enums: <Enum>[]);
+ final List<EnumeratedClass> classes = getCodecClasses(api, root).toList();
expect(classes.length, 2);
expect(
classes
@@ -162,7 +166,55 @@
isAsynchronous: true,
)
]);
- final List<EnumeratedClass> classes = getCodecClasses(api).toList();
+ final Root root =
+ Root(classes: <Class>[], apis: <Api>[api], enums: <Enum>[]);
+ final List<EnumeratedClass> classes = getCodecClasses(api, root).toList();
+ expect(classes.length, 2);
+ expect(
+ classes
+ .where((EnumeratedClass element) => element.name == 'Foo')
+ .length,
+ 1);
+ expect(
+ classes
+ .where((EnumeratedClass element) => element.name == 'Bar')
+ .length,
+ 1);
+ });
+
+ test('getCodecClasses: nested type arguments', () {
+ final Root root = Root(apis: <Api>[
+ Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[
+ Method(
+ name: 'foo',
+ arguments: <NamedType>[
+ NamedType(
+ name: 'x',
+ type: TypeDeclaration(
+ isNullable: false,
+ baseName: 'List',
+ typeArguments: <TypeDeclaration>[
+ TypeDeclaration(baseName: 'Foo', isNullable: true)
+ ])),
+ ],
+ returnType: TypeDeclaration.voidDeclaration(),
+ isAsynchronous: false,
+ )
+ ])
+ ], classes: <Class>[
+ Class(name: 'Foo', fields: <NamedType>[
+ NamedType(
+ name: 'bar',
+ type: TypeDeclaration(baseName: 'Bar', isNullable: true)),
+ ]),
+ Class(name: 'Bar', fields: <NamedType>[
+ NamedType(
+ name: 'value',
+ type: TypeDeclaration(baseName: 'int', isNullable: true))
+ ])
+ ], enums: <Enum>[]);
+ final List<EnumeratedClass> classes =
+ getCodecClasses(root.apis[0], root).toList();
expect(classes.length, 2);
expect(
classes
diff --git a/packages/pigeon/test/pigeon_lib_test.dart b/packages/pigeon/test/pigeon_lib_test.dart
index fed3af7..8f2a587 100644
--- a/packages/pigeon/test/pigeon_lib_test.dart
+++ b/packages/pigeon/test/pigeon_lib_test.dart
@@ -175,6 +175,10 @@
String? input;
}
+class Output1 {
+ int? output;
+}
+
@HostApi()
abstract class ApiTwoMethods {
Output1 method1(Input1 input);
@@ -220,6 +224,10 @@
String? input;
}
+class Output1 {
+ int? output;
+}
+
@FlutterApi()
abstract class AFlutterApi {
Output1 doit(Input1 input);
@@ -838,4 +846,48 @@
expect(results.root.classes.length, 1);
expect(results.root.classes[0].name, 'Foo');
});
+
+ test('recurse into type arguments', () {
+ const String code = '''
+class Foo {
+ int? foo;
+ List<Bar?> bars;
+}
+
+class Bar {
+ int? bar;
+}
+
+@HostApi()
+abstract class Api {
+ void storeAll(List<Foo?> foos);
+}
+''';
+ final ParseResults results = _parseSource(code);
+ expect(results.errors.length, 0);
+ expect(results.root.classes.length, 2);
+ expect(
+ results.root.classes
+ .where((Class element) => element.name == 'Foo')
+ .length,
+ 1);
+ expect(
+ results.root.classes
+ .where((Class element) => element.name == 'Bar')
+ .length,
+ 1);
+ });
+
+ test('undeclared class in argument type argument', () {
+ const String code = '''
+@HostApi()
+abstract class Api {
+ void storeAll(List<Foo?> foos);
+}
+''';
+ final ParseResults results = _parseSource(code);
+ expect(results.errors.length, 1);
+ expect(results.errors[0].lineNumber, 3);
+ expect(results.errors[0].message, contains('Unknown type: Foo'));
+ });
}
