[pigeon] Warn when trying to use enums in collections (flutter#3782)

stuartmorgan-g · web-flow · commit c459977f8b93 · 2023-04-21T21:54:37.000Z
Currently using a enum in a `List` or `Map` doesn't work; add that to the list of things the validator reports an error for. Fixes flutter#125230
diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 9.2.5
+
+* Reports an error when trying to use an enum directly in a `List` or `Map`
+  argument.
+
 ## 9.2.4
 
 * [objc] Fixes a warning due to a C++-style function signature in the codec
diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart
@@ -11,7 +11,7 @@ import 'ast.dart';
 /// The current version of pigeon.
 ///
 /// This must match the version in pubspec.yaml.
-const String pigeonVersion = '9.2.4';
+const String pigeonVersion = '9.2.5';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {
diff --git a/packages/pigeon/lib/pigeon_lib.dart b/packages/pigeon/lib/pigeon_lib.dart
@@ -718,6 +718,16 @@ List<Error> _validateAst(Root root, String source) {
               'Enums aren\'t yet supported for primitive return types: "${method.returnType}" in API: "${api.name}" method: "${method.name}" (https://github.com/flutter/flutter/issues/87307)',
         ));
       }
+      if (method.arguments.any((NamedType arg) =>
+          (arg.type.baseName == 'List' || arg.type.baseName == 'Map') &&
+          arg.type.typeArguments.any(
+              (TypeDeclaration genericType) => isEnum(root, genericType)))) {
+        result.add(Error(
+          message:
+              'Enums aren\'t yet supported for collection types: "${method.arguments[0]}" in API: "${api.name}" method: "${method.name}" (https://github.com/flutter/flutter/issues/87307)',
+          lineNumber: _calculateLineNumberNullable(source, method.offset),
+        ));
+      }
       for (final NamedType unnamedType in method.arguments
           .where((NamedType element) => element.type.baseName.isEmpty)) {
         result.add(Error(
diff --git a/packages/pigeon/mock_handler_tester/test/message.dart b/packages/pigeon/mock_handler_tester/test/message.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/mock_handler_tester/test/test.dart b/packages/pigeon/mock_handler_tester/test/test.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, unnecessary_import
 // ignore_for_file: avoid_relative_lib_imports
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/core_tests.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/core_tests.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/flutter_unittests.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/flutter_unittests.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/multiple_arity.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/multiple_arity.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/non_null_fields.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/non_null_fields.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/null_fields.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/null_fields.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/nullable_returns.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/nullable_returns.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/primitive.gen.dart b/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/primitive.gen.dart
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Autogenerated from Pigeon (v9.2.4), do not edit directly.
+// Autogenerated from Pigeon (v9.2.5), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
 
diff --git a/packages/pigeon/pubspec.yaml b/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: 9.2.4 # This must match the version in lib/generator_tools.dart
+version: 9.2.5 # This must match the version in lib/generator_tools.dart
 
 environment:
   sdk: ">=2.18.0 <4.0.0"
diff --git a/packages/pigeon/test/pigeon_lib_test.dart b/packages/pigeon/test/pigeon_lib_test.dart
@@ -777,6 +777,51 @@ abstract class Api {
     expect(parseResult.errors[0].message, contains('Enums'));
   });
 
+  test('enums list argument', () {
+    // TODO(tarrinneal): Make this not an error: https://github.com/flutter/flutter/issues/87307
+    const String code = '''
+enum Foo { one, two }
+
+@HostApi()
+abstract class Api {
+  void doit(List<Foo> foo);
+}
+''';
+    final ParseResults parseResult = parseSource(code);
+    expect(parseResult.errors.length, equals(1));
+    expect(parseResult.errors[0].message, contains('Enums'));
+  });
+
+  test('enums map argument key', () {
+    // TODO(tarrinneal): Make this not an error: https://github.com/flutter/flutter/issues/87307
+    const String code = '''
+enum Foo { one, two }
+
+@HostApi()
+abstract class Api {
+  void doit(Map<Foo, Object> foo);
+}
+''';
+    final ParseResults parseResult = parseSource(code);
+    expect(parseResult.errors.length, equals(1));
+    expect(parseResult.errors[0].message, contains('Enums'));
+  });
+
+  test('enums map argument value', () {
+    // TODO(tarrinneal): Make this not an error: https://github.com/flutter/flutter/issues/87307
+    const String code = '''
+enum Foo { one, two }
+
+@HostApi()
+abstract class Api {
+  void doit(Map<Foo, Object> foo);
+}
+''';
+    final ParseResults parseResult = parseSource(code);
+    expect(parseResult.errors.length, equals(1));
+    expect(parseResult.errors[0].message, contains('Enums'));
+  });
+
   test('enums return value', () {
     // TODO(gaaclarke): Make this not an error: https://github.com/flutter/flutter/issues/87307
     const String code = '''
