[pigeon] Add errors for ProxyAPI callback methods and null instances when reading in a ProxyApiBaseCodec

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 24.0.1
+
+* [swift, kotlin] Adds an error message when a ProxyAPI callback method that returns a non-null
+  value is nullable.
+* [swift, kotlin] Adds an error message in the `ProxyApiBaseCodec` when an instance could not be
+  retrieved when reading a value.
+
 ## 24.0.0
 
 * **Breaking Change** Relocates some files in `lib` that were not intended for direct client use to `lib/src`.

packages/pigeon/lib/src/generator_tools.dart

@@ -14,7 +14,7 @@ import 'ast.dart';
 /// The current version of pigeon.
 ///
 /// This must match the version in pubspec.yaml.
-const String pigeonVersion = '24.0.0';
+const String pigeonVersion = '24.0.1';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {

packages/pigeon/lib/src/kotlin/kotlin_generator.dart

@@ -835,7 +835,15 @@ if (wrapped == null) {
           override fun readValueOfType(type: Byte, buffer: ByteBuffer): Any? {
             return when (type) {
               $proxyApiCodecInstanceManagerKey.toByte() -> {
-                return registrar.instanceManager.getInstance(readValue(buffer) as Long)
+                val identifier: Long = readValue(buffer) as Long
+                val instance: Any? = registrar.instanceManager.getInstance(identifier)
+                if (instance == null) {
+                  Log.e(
+                    "${proxyApiCodecName(const KotlinOptions())}",
+                    "Failed to find instance with identifier: \$identifier"
+                  )
+                }
+                return instance
               }
               else -> super.readValueOfType(type, buffer)
             }

packages/pigeon/lib/src/pigeon_lib.dart

@@ -1380,11 +1380,22 @@ List<Error> _validateProxyApi(
       }
     }
 
-    if (method.location == ApiLocation.flutter && method.isStatic) {
-      result.add(Error(
-        message: 'Static callback methods are not supported: ${method.name}.',
-        lineNumber: _calculateLineNumberNullable(source, method.offset),
-      ));
+    if (method.location == ApiLocation.flutter) {
+      if (!method.returnType.isVoid &&
+          !method.returnType.isNullable &&
+          !method.isRequired) {
+        result.add(Error(
+          message:
+              'Callback methods that return a non-null value must be non-null: ${method.name}.',
+          lineNumber: _calculateLineNumberNullable(source, method.offset),
+        ));
+      }
+      if (method.isStatic) {
+        result.add(Error(
+          message: 'Static callback methods are not supported: ${method.name}.',
+          lineNumber: _calculateLineNumberNullable(source, method.offset),
+        ));
+      }
     }
   }
 

packages/pigeon/lib/src/swift/swift_generator.dart

@@ -937,6 +937,9 @@ if (wrapped == nil) {
                     let identifier = self.readValue()
                     let instance: AnyObject? = pigeonRegistrar.instanceManager.instance(
                       forIdentifier: identifier is Int64 ? identifier as! Int64 : Int64(identifier as! Int32))
+                    if instance == nil {
+                      print("Failed to find instance with identifier: \\(identifier!)")
+                    }
                     return instance
                   default:
                     return super.readValue(ofType: type)

packages/pigeon/pigeons/proxy_api_tests.dart

@@ -95,42 +95,42 @@ abstract class ProxyApiTestClass extends ProxyApiSuperClass
   // ========== Non-nullable argument/return type tests ==========
 
   /// Returns the passed boolean, to test serialization and deserialization.
-  late bool Function(bool aBool)? flutterEchoBool;
+  late bool Function(bool aBool) flutterEchoBool;
 
   /// Returns the passed int, to test serialization and deserialization.
-  late int Function(int anInt)? flutterEchoInt;
+  late int Function(int anInt) flutterEchoInt;
 
   /// Returns the passed double, to test serialization and deserialization.
-  late double Function(double aDouble)? flutterEchoDouble;
+  late double Function(double aDouble) flutterEchoDouble;
 
   /// Returns the passed string, to test serialization and deserialization.
-  late String Function(String aString)? flutterEchoString;
+  late String Function(String aString) flutterEchoString;
 
   /// Returns the passed byte list, to test serialization and deserialization.
-  late Uint8List Function(Uint8List aList)? flutterEchoUint8List;
+  late Uint8List Function(Uint8List aList) flutterEchoUint8List;
 
   /// Returns the passed list, to test serialization and deserialization.
-  late List<Object?> Function(List<Object?> aList)? flutterEchoList;
+  late List<Object?> Function(List<Object?> aList) flutterEchoList;
 
   /// Returns the passed list with ProxyApis, to test serialization and
   /// deserialization.
-  late List<ProxyApiTestClass?> Function(List<ProxyApiTestClass?> aList)?
+  late List<ProxyApiTestClass?> Function(List<ProxyApiTestClass?> aList)
       flutterEchoProxyApiList;
 
   /// Returns the passed map, to test serialization and deserialization.
-  late Map<String?, Object?> Function(Map<String?, Object?> aMap)?
+  late Map<String?, Object?> Function(Map<String?, Object?> aMap)
       flutterEchoMap;
 
   /// Returns the passed map with ProxyApis, to test serialization and
   /// deserialization.
   late Map<String?, ProxyApiTestClass?> Function(
-      Map<String?, ProxyApiTestClass?> aMap)? flutterEchoProxyApiMap;
+      Map<String?, ProxyApiTestClass?> aMap) flutterEchoProxyApiMap;
 
   /// Returns the passed enum to test serialization and deserialization.
-  late ProxyApiTestEnum Function(ProxyApiTestEnum anEnum)? flutterEchoEnum;
+  late ProxyApiTestEnum Function(ProxyApiTestEnum anEnum) flutterEchoEnum;
 
   /// Returns the passed ProxyApi to test serialization and deserialization.
-  late ProxyApiSuperClass Function(ProxyApiSuperClass aProxyApi)?
+  late ProxyApiSuperClass Function(ProxyApiSuperClass aProxyApi)
       flutterEchoProxyApi;
 
   // ========== Nullable argument/return type tests ==========
@@ -177,7 +177,7 @@ abstract class ProxyApiTestClass extends ProxyApiSuperClass
 
   /// Returns the passed in generic Object asynchronously.
   @async
-  late String Function(String aString)? flutterEchoAsyncString;
+  late String Function(String aString) flutterEchoAsyncString;
 
   // ========== Synchronous host method tests ==========
 

packages/pigeon/platform_tests/shared_test_plugin_code/lib/integration_tests.dart

@@ -2183,6 +2183,18 @@ void runPigeonIntegrationTests(TargetGenerator targetGenerator) {
         aMap: const <String?, Object?>{},
         anEnum: ProxyApiTestEnum.one,
         aProxyApi: ProxyApiSuperClass(),
+        flutterEchoBool: (ProxyApiTestClass instance, bool aBool) => true,
+        flutterEchoInt: (_, __) => 3,
+        flutterEchoDouble: (_, __) => 1.0,
+        flutterEchoString: (_, __) => '',
+        flutterEchoUint8List: (_, __) => Uint8List(0),
+        flutterEchoList: (_, __) => <Object?>[],
+        flutterEchoProxyApiList: (_, __) => <ProxyApiTestClass?>[],
+        flutterEchoMap: (_, __) => <String?, Object?>{},
+        flutterEchoEnum: (_, __) => ProxyApiTestEnum.one,
+        flutterEchoProxyApi: (_, __) => ProxyApiSuperClass(),
+        flutterEchoAsyncString: (_, __) async => '',
+        flutterEchoProxyApiMap: (_, __) => <String?, ProxyApiTestClass?>{},
       );
       // Ensure no error calling method on instance.
       await instance.noop();
@@ -3283,17 +3295,18 @@ ProxyApiTestClass _createGenericProxyApiTestClass({
     flutterNoop: flutterNoop,
     flutterThrowError: flutterThrowError,
     flutterThrowErrorFromVoid: flutterThrowErrorFromVoid,
-    flutterEchoBool: flutterEchoBool,
-    flutterEchoInt: flutterEchoInt,
-    flutterEchoDouble: flutterEchoDouble,
-    flutterEchoString: flutterEchoString,
-    flutterEchoUint8List: flutterEchoUint8List,
-    flutterEchoList: flutterEchoList,
-    flutterEchoProxyApiList: flutterEchoProxyApiList,
-    flutterEchoMap: flutterEchoMap,
-    flutterEchoProxyApiMap: flutterEchoProxyApiMap,
-    flutterEchoEnum: flutterEchoEnum,
-    flutterEchoProxyApi: flutterEchoProxyApi,
+    flutterEchoBool:
+        flutterEchoBool ?? (ProxyApiTestClass instance, bool aBool) => true,
+    flutterEchoInt: flutterEchoInt ?? (_, __) => 3,
+    flutterEchoDouble: flutterEchoDouble ?? (_, __) => 1.0,
+    flutterEchoString: flutterEchoString ?? (_, __) => '',
+    flutterEchoUint8List: flutterEchoUint8List ?? (_, __) => Uint8List(0),
+    flutterEchoList: flutterEchoList ?? (_, __) => <Object?>[],
+    flutterEchoProxyApiList:
+        flutterEchoProxyApiList ?? (_, __) => <ProxyApiTestClass?>[],
+    flutterEchoMap: flutterEchoMap ?? (_, __) => <String?, Object?>{},
+    flutterEchoEnum: flutterEchoEnum ?? (_, __) => ProxyApiTestEnum.one,
+    flutterEchoProxyApi: flutterEchoProxyApi ?? (_, __) => ProxyApiSuperClass(),
     flutterEchoNullableBool: flutterEchoNullableBool,
     flutterEchoNullableInt: flutterEchoNullableInt,
     flutterEchoNullableDouble: flutterEchoNullableDouble,
@@ -3304,6 +3317,8 @@ ProxyApiTestClass _createGenericProxyApiTestClass({
     flutterEchoNullableEnum: flutterEchoNullableEnum,
     flutterEchoNullableProxyApi: flutterEchoNullableProxyApi,
     flutterNoopAsync: flutterNoopAsync,
-    flutterEchoAsyncString: flutterEchoAsyncString,
+    flutterEchoAsyncString: flutterEchoAsyncString ?? (_, __) async => '',
+    flutterEchoProxyApiMap:
+        flutterEchoProxyApiMap ?? (_, __) => <String?, ProxyApiTestClass?>{},
   );
 }