[pigeon] Implements primitive enums for arguments to HostApis (#1871)

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 3.1.3
+
+* Adds support for enums in arguments to methods for HostApis.
+
 ## 3.1.2
 
 * [c++] Fixes minor style issues in generated code. This includes the naming of

packages/pigeon/README.md

@@ -139,6 +139,7 @@ public:
 Pigeon supports generating null-safe code, but it doesn't yet support:
 
 1) Nullable generics type arguments
+1) Nullable enum arguments to methods
 
 It does support:
 

packages/pigeon/lib/cpp_generator.dart

@@ -892,5 +892,17 @@ List<Error> validateCpp(CppOptions options, Root root) {
       }
     }
   }
+  for (final Api api in root.apis) {
+    for (final Method method in api.methods) {
+      for (final NamedType arg in method.arguments) {
+        if (isEnum(root, arg.type)) {
+          // TODO(gaaclarke): Add line number and filename.
+          result.add(Error(
+              message:
+                  'Nullable enum types aren\'t supported in C++ arguments in method:${api.name}.${method.name} argument:(${arg.type.baseName} ${arg.name}).'));
+        }
+      }
+    }
+  }
   return result;
 }

packages/pigeon/lib/dart_generator.dart

@@ -173,8 +173,16 @@ final BinaryMessenger? _binaryMessenger;
       if (func.arguments.isNotEmpty) {
         String argNameFunc(int index, NamedType type) =>
             _getSafeArgumentName(index, type);
-        final Iterable<String> argNames = indexMap(func.arguments, argNameFunc);
+        final Iterable<String> argExpressions =
-        sendArgument = '<Object?>[${argNames.join(', ')}]';
+            indexMap(func.arguments, (int index, NamedType type) {
+          final String name = argNameFunc(index, type);
+          if (root.enums.map((Enum e) => e.name).contains(type.type.baseName)) {
+            return '$name${type.type.isNullable ? '?' : ''}.index';
+          } else {
+            return name;
+          }
+        });
+        sendArgument = '<Object?>[${argExpressions.join(', ')}]';
         argSignature = _getMethodArgumentsSignature(func, argNameFunc);
       }
       indent.write(

packages/pigeon/lib/generator_tools.dart

@@ -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.1.2';
+const String pigeonVersion = '3.1.3';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {
@@ -400,3 +400,7 @@ Iterable<EnumeratedClass> getCodecClasses(Api api, Root root) sync* {
     enumeration += 1;
   }
 }
+
+/// Returns true if the [TypeDeclaration] represents an enum.
+bool isEnum(Root root, TypeDeclaration type) =>
+    root.enums.map((Enum e) => e.name).contains(type.baseName);

packages/pigeon/lib/java_generator.dart

@@ -5,7 +5,7 @@
 import 'ast.dart';
 import 'functional.dart';
 import 'generator_tools.dart';
-import 'pigeon_lib.dart';
+import 'pigeon_lib.dart' show TaskQueueType;
 
 /// Options that control how Java code will be generated.
 class JavaOptions {
@@ -58,6 +58,11 @@ class JavaOptions {
 /// Calculates the name of the codec that will be generated for [api].
 String _getCodecName(Api api) => '${api.name}Codec';
 
+/// Converts an expression that evaluates to an nullable int to an expression
+/// that evaluates to a nullable enum.
+String _intToEnum(String expression, String enumName) =>
+    '$expression == null ? null : $enumName.values()[(int)$expression]';
+
 /// Writes the codec class that will be used by [api].
 /// Example:
 /// private static class FooCodec extends StandardMessageCodec {...}
@@ -115,9 +120,12 @@ void _writeCodec(Indent indent, Api api, Root root) {
 ///   int add(int x, int y);
 ///   static void setup(BinaryMessenger binaryMessenger, Foo api) {...}
 /// }
-void _writeHostApi(Indent indent, Api api) {
+void _writeHostApi(Indent indent, Api api, Root root) {
   assert(api.location == ApiLocation.host);
 
+  bool isEnum(TypeDeclaration type) =>
+      root.enums.map((Enum e) => e.name).contains(type.baseName);
+
   /// Write a method in the interface.
   /// Example:
   ///   int add(int x, int y);
@@ -195,8 +203,13 @@ void _writeHostApi(Indent indent, Api api) {
                 final String argExpression = isInt
                     ? '($argName == null) ? null : $argName.longValue()'
                     : argName;
-                indent
+                String accessor = 'args.get($index)';
-                    .writeln('$argType $argName = ($argType)args.get($index);');
+                if (isEnum(arg.type)) {
+                  accessor = _intToEnum(accessor, arg.type.baseName);
+                } else {
+                  accessor = '($argType)$accessor';
+                }
+                indent.writeln('$argType $argName = $accessor;');
                 if (!arg.type.isNullable) {
                   indent.write('if ($argName == null) ');
                   indent.scoped('{', '}', () {
@@ -556,7 +569,7 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
           indent.writeln('Object $fieldVariable = map.get("${field.name}");');
           if (rootEnumNameSet.contains(field.type.baseName)) {
             indent.writeln(
-                '$result.$setter($fieldVariable == null ? null : ${field.type.baseName}.values()[(int)$fieldVariable]);');
+                '$result.$setter(${_intToEnum(fieldVariable, field.type.baseName)});');
           } else {
             indent.writeln(
                 '$result.$setter(${_castObject(field, root.classes, root.enums, fieldVariable)});');
@@ -628,7 +641,7 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
 
   void writeApi(Api api) {
     if (api.location == ApiLocation.host) {
-      _writeHostApi(indent, api);
+      _writeHostApi(indent, api, root);
     } else if (api.location == ApiLocation.flutter) {
       _writeFlutterApi(indent, api);
     }

packages/pigeon/lib/objc_generator.dart

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:pigeon/functional.dart';
-import 'package:pigeon/pigeon_lib.dart';
+import 'package:pigeon/pigeon_lib.dart' show TaskQueueType, Error;
 
 import 'ast.dart';
 import 'generator_tools.dart';
@@ -367,6 +367,7 @@ String _makeObjcSignature({
   required String returnType,
   required String lastArgType,
   required String lastArgName,
+  required bool Function(TypeDeclaration) isEnum,
   String Function(int, NamedType)? argNameFunc,
 }) {
   argNameFunc = argNameFunc ?? (int _, NamedType e) => e.name;
@@ -376,9 +377,13 @@ String _makeObjcSignature({
       _getSelectorComponents(func, lastArgName);
   final Iterable<String> argTypes = followedByOne(
     func.arguments.map((NamedType arg) {
-      final String nullable = arg.type.isNullable ? 'nullable ' : '';
+      if (isEnum(arg.type)) {
-      final _ObjcPtr argType = _objcTypeForDartType(options.prefix, arg.type);
+        return _className(options.prefix, arg.type.baseName);
-      return '$nullable${argType.ptr.trim()}';
+      } else {
+        final String nullable = arg.type.isNullable ? 'nullable ' : '';
+        final _ObjcPtr argType = _objcTypeForDartType(options.prefix, arg.type);
+        return '$nullable${argType.ptr.trim()}';
+      }
     }),
     lastArgType,
   );
@@ -401,7 +406,8 @@ String _makeObjcSignature({
 /// @end
 ///
 /// extern void FooSetup(id<FlutterBinaryMessenger> binaryMessenger, NSObject<Foo> *_Nullable api);
-void _writeHostApiDeclaration(Indent indent, Api api, ObjcOptions options) {
+void _writeHostApiDeclaration(
+    Indent indent, Api api, ObjcOptions options, Root root) {
   final String apiName = _className(options.prefix, api.name);
   indent.writeln('@protocol $apiName');
   for (final Method func in api.methods) {
@@ -438,7 +444,8 @@ void _writeHostApiDeclaration(Indent indent, Api api, ObjcOptions options) {
             options: options,
             returnType: returnType,
             lastArgName: lastArgName,
-            lastArgType: lastArgType) +
+            lastArgType: lastArgType,
+            isEnum: (TypeDeclaration t) => isEnum(root, t)) +
         ';');
   }
   indent.writeln('@end');
@@ -456,7 +463,8 @@ void _writeHostApiDeclaration(Indent indent, Api api, ObjcOptions options) {
 /// - (instancetype)initWithBinaryMessenger:(id<FlutterBinaryMessenger>)binaryMessenger;
 /// - (void)add:(NSInteger)x to:(NSInteger)y completion:(void(^)(NSError *, NSInteger result)completion;
 /// @end
-void _writeFlutterApiDeclaration(Indent indent, Api api, ObjcOptions options) {
+void _writeFlutterApiDeclaration(
+    Indent indent, Api api, ObjcOptions options, Root root) {
   final String apiName = _className(options.prefix, api.name);
   indent.writeln('@interface $apiName : NSObject');
   indent.writeln(
@@ -471,6 +479,7 @@ void _writeFlutterApiDeclaration(Indent indent, Api api, ObjcOptions options) {
           returnType: 'void',
           lastArgName: 'completion',
           lastArgType: callbackType,
+          isEnum: (TypeDeclaration t) => isEnum(root, t),
         ) +
         ';');
   }
@@ -543,9 +552,9 @@ void generateObjcHeader(ObjcOptions options, Root root, StringSink sink) {
         'NSObject<FlutterMessageCodec> *${_getCodecGetterName(options.prefix, api.name)}(void);');
     indent.addln('');
     if (api.location == ApiLocation.host) {
-      _writeHostApiDeclaration(indent, api, options);
+      _writeHostApiDeclaration(indent, api, options, root);
     } else if (api.location == ApiLocation.flutter) {
-      _writeFlutterApiDeclaration(indent, api, options);
+      _writeFlutterApiDeclaration(indent, api, options, root);
     }
   }
 
@@ -585,7 +594,8 @@ String _getSafeArgName(int count, NamedType arg) =>
 
 /// Writes the definition code for a host [Api].
 /// See also: [_writeHostApiDeclaration]
-void _writeHostApiSource(Indent indent, ObjcOptions options, Api api) {
+void _writeHostApiSource(
+    Indent indent, ObjcOptions options, Api api, Root root) {
   assert(api.location == ApiLocation.host);
   final String apiName = _className(options.prefix, api.name);
 
@@ -612,8 +622,13 @@ void _writeHostApiSource(Indent indent, ObjcOptions options, Api api) {
       indent.writeln('NSArray *args = $variable;');
       map3(wholeNumbers.take(func.arguments.length), argNames, func.arguments,
           (int count, String argName, NamedType arg) {
-        final _ObjcPtr argType = _objcTypeForDartType(options.prefix, arg.type);
+        if (isEnum(root, arg.type)) {
-        return '${argType.ptr}$argName = GetNullableObjectAtIndex(args, $count);';
+          return '${_className(options.prefix, arg.type.baseName)} $argName = [GetNullableObjectAtIndex(args, $count) integerValue];';
+        } else {
+          final _ObjcPtr argType =
+              _objcTypeForDartType(options.prefix, arg.type);
+          return '${argType.ptr}$argName = GetNullableObjectAtIndex(args, $count);';
+        }
       }).forEach(indent.writeln);
     }
 
@@ -726,7 +741,8 @@ void _writeHostApiSource(Indent indent, ObjcOptions options, Api api) {
 
 /// Writes the definition code for a flutter [Api].
 /// See also: [_writeFlutterApiDeclaration]
-void _writeFlutterApiSource(Indent indent, ObjcOptions options, Api api) {
+void _writeFlutterApiSource(
+    Indent indent, ObjcOptions options, Api api, Root root) {
   assert(api.location == ApiLocation.flutter);
   final String apiName = _className(options.prefix, api.name);
 
@@ -771,6 +787,7 @@ void _writeFlutterApiSource(Indent indent, ObjcOptions options, Api api) {
       lastArgName: 'completion',
       lastArgType: callbackType,
       argNameFunc: argNameFunc,
+      isEnum: (TypeDeclaration t) => isEnum(root, t),
     ));
     indent.scoped(' {', '}', () {
       indent.writeln('FlutterBasicMessageChannel *channel =');
@@ -929,9 +946,9 @@ static id GetNullableObjectAtIndex(NSArray* array, NSInteger key) {
     _writeCodec(indent, codecName, options, api, root);
     indent.addln('');
     if (api.location == ApiLocation.host) {
-      _writeHostApiSource(indent, options, api);
+      _writeHostApiSource(indent, options, api, root);
     } else if (api.location == ApiLocation.flutter) {
-      _writeFlutterApiSource(indent, options, api);
+      _writeFlutterApiSource(indent, options, api, root);
     }
   }
 
@@ -950,3 +967,23 @@ static id GetNullableObjectAtIndex(NSArray* array, NSInteger key) {
   }
   root.apis.forEach(writeApi);
 }
+
+/// Looks through the AST for features that aren't supported by the ObjC
+/// generator.
+List<Error> validateObjc(ObjcOptions options, Root root) {
+  final List<Error> errors = <Error>[];
+  for (final Api api in root.apis) {
+    for (final Method method in api.methods) {
+      for (final NamedType arg in method.arguments) {
+        if (isEnum(root, arg.type) && arg.type.isNullable) {
+          // TODO(gaaclarke): Add line number.
+          errors.add(Error(
+              message:
+                  'Nullable enum types aren\'t support in ObjC arguments in method:${api.name}.${method.name} argument:(${arg.type.baseName} ${arg.name}).'));
+        }
+      }
+    }
+  }
+
+  return errors;
+}

packages/pigeon/lib/pigeon_lib.dart

@@ -440,7 +440,8 @@ class ObjcHeaderGenerator implements Generator {
       _openSink(options.objcHeaderOut);
 
   @override
-  List<Error> validate(PigeonOptions options, Root root) => <Error>[];
+  List<Error> validate(PigeonOptions options, Root root) =>
+      validateObjc(options.objcOptions!, root);
 }
 
 /// A [Generator] that generates Objective-C source code.
@@ -593,12 +594,13 @@ List<Error> _validateAst(Root root, String source) {
   }
   for (final Api api in root.apis) {
     for (final Method method in api.methods) {
-      if (method.arguments.isNotEmpty &&
+      if (api.location == ApiLocation.flutter &&
+          method.arguments.isNotEmpty &&
           method.arguments.any((NamedType element) =>
               customEnums.contains(element.type.baseName))) {
         result.add(Error(
           message:
-              'Enums aren\'t yet supported for primitive arguments: "${method.arguments[0]}" in API: "${api.name}" method: "${method.name}" (https://github.com/flutter/flutter/issues/87307)',
+              'Enums aren\'t yet supported for primitive arguments in FlutterApis: "${method.arguments[0]}" in API: "${api.name}" method: "${method.name}" (https://github.com/flutter/flutter/issues/87307)',
           lineNumber: _calculateLineNumberNullable(source, method.offset),
         ));
       }

packages/pigeon/pigeons/enum_args.dart

@@ -0,0 +1,20 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:pigeon/pigeon.dart';
+
+enum State {
+  Pending,
+  Success,
+  Error,
+}
+
+class Data {
+  State? state;
+}
+
+@HostApi()
+abstract class EnumArg2Host {
+  void foo(State state);
+}

packages/pigeon/pubspec.yaml

@@ -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.1.2 # This must match the version in lib/generator_tools.dart
+version: 3.1.3 # This must match the version in lib/generator_tools.dart
 
 environment:
   sdk: ">=2.12.0 <3.0.0"

packages/pigeon/run_tests.sh

@@ -216,6 +216,7 @@ run_ios_unittests() {
   gen_ios_unittests_code ./pigeons/async_handlers.dart ""
   gen_ios_unittests_code ./pigeons/background_platform_channels.dart "BC"
   gen_ios_unittests_code ./pigeons/enum.dart "AC"
+  gen_ios_unittests_code ./pigeons/enum_args.dart "EA"
   gen_ios_unittests_code ./pigeons/host2flutter.dart ""
   gen_ios_unittests_code ./pigeons/list.dart "LST"
   gen_ios_unittests_code ./pigeons/message.dart ""
@@ -277,6 +278,7 @@ run_android_unittests() {
   gen_android_unittests_code ./pigeons/async_handlers.dart AsyncHandlers
   gen_android_unittests_code ./pigeons/background_platform_channels.dart BackgroundPlatformChannels
   gen_android_unittests_code ./pigeons/enum.dart Enum
+  gen_android_unittests_code ./pigeons/enum_args.dart EnumArgs
   gen_android_unittests_code ./pigeons/host2flutter.dart Host2Flutter
   gen_android_unittests_code ./pigeons/java_double_host_api.dart JavaDoubleHostApi
   gen_android_unittests_code ./pigeons/list.dart PigeonList

packages/pigeon/test/cpp_generator_test.dart

@@ -296,4 +296,28 @@ void main() {
     expect(errors[0].message, contains('foo'));
     expect(errors[0].message, contains('Foo'));
   });
+
+  test('enum argument', () {
+    final Root root = Root(
+      apis: <Api>[
+        Api(name: 'Bar', location: ApiLocation.host, methods: <Method>[
+          Method(
+              name: 'bar',
+              returnType: const TypeDeclaration.voidDeclaration(),
+              arguments: <NamedType>[
+                NamedType(
+                    name: 'foo',
+                    type: const TypeDeclaration(
+                        baseName: 'Foo', isNullable: false))
+              ])
+        ])
+      ],
+      classes: <Class>[],
+      enums: <Enum>[
+        Enum(name: 'Foo', members: <String>['one', 'two'])
+      ],
+    );
+    final List<Error> errors = validateCpp(const CppOptions(), root);
+    expect(errors.length, 1);
+  });
 }

packages/pigeon/test/dart_generator_test.dart

@@ -445,6 +445,30 @@ void main() {
     expect(code, contains('EnumClass doSomething(EnumClass arg0);'));
   });
 
+  test('primitive enum host', () {
+    final Root root = Root(apis: <Api>[
+      Api(name: 'Bar', location: ApiLocation.host, methods: <Method>[
+        Method(
+            name: 'bar',
+            returnType: const TypeDeclaration.voidDeclaration(),
+            arguments: <NamedType>[
+              NamedType(
+                  name: 'foo',
+                  type:
+                      const TypeDeclaration(baseName: 'Foo', isNullable: true))
+            ])
+      ])
+    ], classes: <Class>[], enums: <Enum>[
+      Enum(name: 'Foo', members: <String>['one', 'two'])
+    ]);
+    final StringBuffer sink = StringBuffer();
+    generateDart(const DartOptions(), root, sink);
+    final String code = sink.toString();
+    expect(code, contains('enum Foo {'));
+    expect(code, contains('Future<void> bar(Foo? arg_foo) async'));
+    expect(code, contains('channel.send(<Object?>[arg_foo?.index])'));
+  });
+
   test('flutter non-nullable enum argument with enum class', () {
     final Root root = Root(apis: <Api>[
       Api(name: 'Api', location: ApiLocation.flutter, methods: <Method>[

packages/pigeon/test/java_generator_test.dart

@@ -624,6 +624,33 @@ void main() {
             'pigeonResult.setEnum1(enum1 == null ? null : Enum1.values()[(int)enum1])'));
   });
 
+  test('primitive enum host', () {
+    final Root root = Root(apis: <Api>[
+      Api(name: 'Bar', location: ApiLocation.host, methods: <Method>[
+        Method(
+            name: 'bar',
+            returnType: const TypeDeclaration.voidDeclaration(),
+            arguments: <NamedType>[
+              NamedType(
+                  name: 'foo',
+                  type:
+                      const TypeDeclaration(baseName: 'Foo', isNullable: true))
+            ])
+      ])
+    ], classes: <Class>[], enums: <Enum>[
+      Enum(name: 'Foo', members: <String>['one', 'two'])
+    ]);
+    final StringBuffer sink = StringBuffer();
+    const JavaOptions javaOptions = JavaOptions(className: 'Messages');
+    generateJava(javaOptions, root, sink);
+    final String code = sink.toString();
+    expect(code, contains('public enum Foo'));
+    expect(
+        code,
+        contains(
+            'Foo fooArg = args.get(0) == null ? null : Foo.values()[(int)args.get(0)];'));
+  });
+
   Iterable<String> _makeIterable(String string) sync* {
     yield string;
   }

packages/pigeon/test/objc_generator_test.dart

@@ -117,6 +117,62 @@ void main() {
             'pigeonResult.enum1 = [GetNullableObject(dict, @"enum1") integerValue];'));
   });
 
+  test('primitive enum host', () {
+    final Root root = Root(apis: <Api>[
+      Api(name: 'Bar', location: ApiLocation.host, methods: <Method>[
+        Method(
+            name: 'bar',
+            returnType: const TypeDeclaration.voidDeclaration(),
+            arguments: <NamedType>[
+              NamedType(
+                  name: 'foo',
+                  type:
+                      const TypeDeclaration(baseName: 'Foo', isNullable: false))
+            ])
+      ])
+    ], classes: <Class>[], enums: <Enum>[
+      Enum(name: 'Foo', members: <String>['one', 'two'])
+    ]);
+    final StringBuffer sink = StringBuffer();
+    const ObjcOptions options = ObjcOptions(header: 'foo.h', prefix: 'AC');
+    {
+      generateObjcHeader(options, root, sink);
+      final String code = sink.toString();
+      expect(code, contains('typedef NS_ENUM(NSUInteger, ACFoo)'));
+      expect(code, contains(':(ACFoo)foo error:'));
+    }
+    {
+      generateObjcSource(options, root, sink);
+      final String code = sink.toString();
+      expect(
+          code,
+          contains(
+              'ACFoo arg_foo = [GetNullableObjectAtIndex(args, 0) integerValue];'));
+    }
+  });
+
+  test('validate nullable primitive enum', () {
+    final Root root = Root(apis: <Api>[
+      Api(name: 'Bar', location: ApiLocation.host, methods: <Method>[
+        Method(
+            name: 'bar',
+            returnType: const TypeDeclaration.voidDeclaration(),
+            arguments: <NamedType>[
+              NamedType(
+                  name: 'foo',
+                  type:
+                      const TypeDeclaration(baseName: 'Foo', isNullable: true))
+            ])
+      ])
+    ], classes: <Class>[], enums: <Enum>[
+      Enum(name: 'Foo', members: <String>['one', 'two'])
+    ]);
+    const ObjcOptions options = ObjcOptions(header: 'foo.h');
+    final List<Error> errors = validateObjc(options, root);
+    expect(errors.length, 1);
+    expect(errors[0].message, contains('Nullable enum'));
+  });
+
   test('gen one class header with enum', () {
     final Root root = Root(
       apis: <Api>[],

packages/pigeon/test/pigeon_lib_test.dart

@@ -699,7 +699,23 @@ abstract class Api {
     expect(parseResult.errors[0].lineNumber, 2);
   });
 
-  test('enums argument', () {
+  test('enums argument host', () {
+    const String code = '''
+enum Foo {
+  one,
+  two,
+}
+
+@HostApi()
+abstract class Api {
+  void doit(Foo foo);
+}
+''';
+    final ParseResults parseResult = _parseSource(code);
+    expect(parseResult.errors.length, equals(0));
+  });
+
+  test('enums argument flutter', () {
     // TODO(gaaclarke): Make this not an error: https://github.com/flutter/flutter/issues/87307
     const String code = '''
 
@@ -708,7 +724,7 @@ enum Foo {
   two,
 }
 
-@HostApi()
+@FlutterApi()
 abstract class Api {
   void doit(Foo foo);
 }