Fix unnecessary toList/fromList calls during encode/decode process (f…

…lutter#6426) Fix unnecessary toList/fromList calls during encode/decode process. also makes some kotlin code more idiomatic. removes the ability to have collisions with the name `list`. fixes flutter/flutter#119351
Arctuition · Jun 14, 2024 · 960b798 · 960b798
1 parent ecc7d42
commit 960b798
Show file tree

Hide file tree

Showing 48 changed files with 1,309 additions and 1,357 deletions.
diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 18.0.1
+
+* Fixes unnecessary calls of `toList` and `fromList` when encoding/decoding data classes.
+* [kotlin] Changes to some code to make it more idiomatic.
+* Removes collisions with the word `list`.
+
 ## 18.0.0
 
 * Adds message channel suffix option to all APIs.

diff --git a/packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java b/packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java
@@ -186,15 +186,15 @@ ArrayList<Object> toList() {
       return toListResult;
     }
 
-    static @NonNull MessageData fromList(@NonNull ArrayList<Object> list) {
+    static @NonNull MessageData fromList(@NonNull ArrayList<Object> __pigeon_list) {
       MessageData pigeonResult = new MessageData();
-      Object name = list.get(0);
+      Object name = __pigeon_list.get(0);
       pigeonResult.setName((String) name);
-      Object description = list.get(1);
+      Object description = __pigeon_list.get(1);
       pigeonResult.setDescription((String) description);
-      Object code = list.get(2);
+      Object code = __pigeon_list.get(2);
       pigeonResult.setCode(Code.values()[(int) code]);
-      Object data = list.get(3);
+      Object data = __pigeon_list.get(3);
       pigeonResult.setData((Map<String, String>) data);
       return pigeonResult;
     }

diff --git a/...geon/example/app/android/app/src/main/kotlin/dev/flutter/pigeon_example_app/Messages.g.kt b/...geon/example/app/android/app/src/main/kotlin/dev/flutter/pigeon_example_app/Messages.g.kt
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 // Autogenerated from Pigeon, do not edit directly.
 // See also: https://pub.dev/packages/pigeon
+@file:Suppress("UNCHECKED_CAST", "ArrayInDataClass")
 
 import android.util.Log
 import io.flutter.plugin.common.BasicMessageChannel
@@ -17,10 +18,10 @@ private fun wrapResult(result: Any?): List<Any?> {
 }
 
 private fun wrapError(exception: Throwable): List<Any?> {
-  if (exception is FlutterError) {
-    return listOf(exception.code, exception.message, exception.details)
+  return if (exception is FlutterError) {
+    listOf(exception.code, exception.message, exception.details)
   } else {
-    return listOf(
+    listOf(
         exception.javaClass.simpleName,
         exception.toString(),
         "Cause: " + exception.cause + ", Stacktrace: " + Log.getStackTraceString(exception))
@@ -64,12 +65,12 @@ data class MessageData(
     val data: Map<String?, String?>
 ) {
   companion object {
-    @Suppress("UNCHECKED_CAST")
-    fun fromList(list: List<Any?>): MessageData {
-      val name = list[0] as String?
-      val description = list[1] as String?
-      val code = Code.ofRaw(list[2] as Int)!!
-      val data = list[3] as Map<String?, String?>
+    @Suppress("LocalVariableName")
+    fun fromList(__pigeon_list: List<Any?>): MessageData {
+      val name = __pigeon_list[0] as String?
+      val description = __pigeon_list[1] as String?
+      val code = Code.ofRaw(__pigeon_list[2] as Int)!!
+      val data = __pigeon_list[3] as Map<String?, String?>
       return MessageData(name, description, code, data)
     }
   }
@@ -84,7 +85,6 @@ data class MessageData(
   }
 }
 
-@Suppress("UNCHECKED_CAST")
 private object ExampleHostApiCodec : StandardMessageCodec() {
   override fun readValueOfType(type: Byte, buffer: ByteBuffer): Any? {
     return when (type) {
@@ -118,7 +118,6 @@ interface ExampleHostApi {
     /** The codec used by ExampleHostApi. */
     val codec: MessageCodec<Any?> by lazy { ExampleHostApiCodec }
     /** Sets up an instance of `ExampleHostApi` to handle messages through the `binaryMessenger`. */
-    @Suppress("UNCHECKED_CAST")
     fun setUp(
         binaryMessenger: BinaryMessenger,
         api: ExampleHostApi?,
@@ -134,12 +133,12 @@ interface ExampleHostApi {
                 codec)
         if (api != null) {
           channel.setMessageHandler { _, reply ->
-            var wrapped: List<Any?>
-            try {
-              wrapped = listOf<Any?>(api.getHostLanguage())
-            } catch (exception: Throwable) {
-              wrapped = wrapError(exception)
-            }
+            val wrapped: List<Any?> =
+                try {
+                  listOf<Any?>(api.getHostLanguage())
+                } catch (exception: Throwable) {
+                  wrapError(exception)
+                }
             reply.reply(wrapped)
           }
         } else {
@@ -155,14 +154,14 @@ interface ExampleHostApi {
         if (api != null) {
           channel.setMessageHandler { message, reply ->
             val args = message as List<Any?>
-            val aArg = args[0].let { if (it is Int) it.toLong() else it as Long }
-            val bArg = args[1].let { if (it is Int) it.toLong() else it as Long }
-            var wrapped: List<Any?>
-            try {
-              wrapped = listOf<Any?>(api.add(aArg, bArg))
-            } catch (exception: Throwable) {
-              wrapped = wrapError(exception)
-            }
+            val aArg = args[0].let { num -> if (num is Int) num.toLong() else num as Long }
+            val bArg = args[1].let { num -> if (num is Int) num.toLong() else num as Long }
+            val wrapped: List<Any?> =
+                try {
+                  listOf<Any?>(api.add(aArg, bArg))
+                } catch (exception: Throwable) {
+                  wrapError(exception)
+                }
             reply.reply(wrapped)
           }
         } else {
@@ -197,7 +196,6 @@ interface ExampleHostApi {
   }
 }
 /** Generated class from Pigeon that represents Flutter messages that can be called from Kotlin. */
-@Suppress("UNCHECKED_CAST")
 class MessageFlutterApi(
     private val binaryMessenger: BinaryMessenger,
     private val messageChannelSuffix: String = ""

diff --git a/packages/pigeon/example/app/ios/Runner/Messages.g.swift b/packages/pigeon/example/app/ios/Runner/Messages.g.swift
@@ -60,11 +60,12 @@ struct MessageData {
   var code: Code
   var data: [String?: String?]
 
-  static func fromList(_ list: [Any?]) -> MessageData? {
-    let name: String? = nilOrValue(list[0])
-    let description: String? = nilOrValue(list[1])
-    let code = Code(rawValue: list[2] as! Int)!
-    let data = list[3] as! [String?: String?]
+  // swift-format-ignore: AlwaysUseLowerCamelCase
+  static func fromList(_ __pigeon_list: [Any?]) -> MessageData? {
+    let name: String? = nilOrValue(__pigeon_list[0])
+    let description: String? = nilOrValue(__pigeon_list[1])
+    let code = Code(rawValue: __pigeon_list[2] as! Int)!
+    let data = __pigeon_list[3] as! [String?: String?]
 
     return MessageData(
       name: name,

diff --git a/packages/pigeon/lib/ast.dart b/packages/pigeon/lib/ast.dart
@@ -518,7 +518,7 @@ class TypeDeclaration {
   @override
   String toString() {
     final String typeArgumentsStr =
-        typeArguments.isEmpty ? '' : 'typeArguments:$typeArguments';
+        typeArguments.isEmpty ? '' : ' typeArguments:$typeArguments';
     return '(TypeDeclaration baseName:$baseName isNullable:$isNullable$typeArgumentsStr isEnum:$isEnum isClass:$isClass isProxyApi:$isProxyApi)';
   }
 }

diff --git a/packages/pigeon/lib/cpp_generator.dart b/packages/pigeon/lib/cpp_generator.dart
@@ -787,8 +787,12 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
         final HostDatatype hostDatatype =
             getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
         final String encodableValue = _wrappedHostApiArgumentExpression(
-            root, _makeInstanceVariableName(field), field.type, hostDatatype,
-            preSerializeClasses: true);
+          root,
+          _makeInstanceVariableName(field),
+          field.type,
+          hostDatatype,
+          true,
+        );
         indent.writeln('list.push_back($encodableValue);');
       }
       indent.writeln('return list;');
@@ -815,11 +819,8 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
       } else {
         final HostDatatype hostDatatype =
             getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
-        if (!hostDatatype.isBuiltin &&
-            root.classes
-                .map((Class x) => x.name)
-                .contains(field.type.baseName)) {
-          return '${hostDatatype.datatype}::FromEncodableList(std::get<EncodableList>($encodable))';
+        if (field.type.isClass) {
+          return _classReferenceFromEncodableValue(hostDatatype, encodable);
         } else {
           return 'std::get<${hostDatatype.datatype}>($encodable)';
         }
@@ -960,8 +961,12 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
           indent.addScoped('EncodableValue(EncodableList{', '});', () {
             for (final _HostNamedType param in hostParameters) {
               final String encodedArgument = _wrappedHostApiArgumentExpression(
-                  root, param.name, param.originalType, param.hostType,
-                  preSerializeClasses: false);
+                root,
+                param.name,
+                param.originalType,
+                param.hostType,
+                false,
+              );
               indent.writeln('$encodedArgument,');
             }
           });
@@ -1452,27 +1457,20 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
 
   /// Returns the expression to create an EncodableValue from a host API argument
   /// with the given [variableName] and types.
-  ///
-  /// If [preSerializeClasses] is true, custom classes will be returned as
-  /// encodable lists rather than CustomEncodableValues; see
-  /// https://github.com/flutter/flutter/issues/119351 for why this is currently
-  /// needed.
-  String _wrappedHostApiArgumentExpression(Root root, String variableName,
-      TypeDeclaration dartType, HostDatatype hostType,
-      {required bool preSerializeClasses}) {
+  String _wrappedHostApiArgumentExpression(
+    Root root,
+    String variableName,
+    TypeDeclaration dartType,
+    HostDatatype hostType,
+    bool isNestedClass,
+  ) {
     final String encodableValue;
     if (!hostType.isBuiltin &&
         root.classes.any((Class c) => c.name == dartType.baseName)) {
-      if (preSerializeClasses) {
-        final String operator =
-            hostType.isNullable || _isPointerField(hostType) ? '->' : '.';
-        encodableValue =
-            'EncodableValue($variableName${operator}ToEncodableList())';
-      } else {
-        final String nonNullValue =
-            hostType.isNullable ? '*$variableName' : variableName;
-        encodableValue = 'CustomEncodableValue($nonNullValue)';
-      }
+      final String nonNullValue = hostType.isNullable || isNestedClass
+          ? '*$variableName'
+          : variableName;
+      encodableValue = 'CustomEncodableValue($nonNullValue)';
     } else if (!hostType.isBuiltin &&
         root.enums.any((Enum e) => e.name == dartType.baseName)) {
       final String nonNullValue =
@@ -1537,7 +1535,7 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
         }
       } else {
         indent.writeln(
-            'const auto* $argName = &(std::any_cast<const ${hostType.datatype}&>(std::get<CustomEncodableValue>($encodableArgName)));');
+            'const auto* $argName = &(${_classReferenceFromEncodableValue(hostType, encodableArgName)});');
       }
     } else {
       // Non-nullable arguments are either passed by value or reference, but the
@@ -1562,7 +1560,7 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
             'const ${hostType.datatype}& $argName = (${hostType.datatype})$encodableArgName.LongValue();');
       } else {
         indent.writeln(
-            'const auto& $argName = std::any_cast<const ${hostType.datatype}&>(std::get<CustomEncodableValue>($encodableArgName));');
+            'const auto& $argName = ${_classReferenceFromEncodableValue(hostType, encodableArgName)};');
       }
     }
   }
@@ -1573,6 +1571,13 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
   String? _shortBaseCppTypeForBuiltinDartType(TypeDeclaration type) {
     return _baseCppTypeForBuiltinDartType(type, includeFlutterNamespace: false);
   }
+
+  /// Returns the code to extract a `const {type.datatype}&` from an EncodableValue
+  /// variable [variableName] that contains an instance of [type].
+  String _classReferenceFromEncodableValue(
+      HostDatatype type, String variableName) {
+    return 'std::any_cast<const ${type.datatype}&>(std::get<CustomEncodableValue>($variableName))';
+  }
 }
 
 /// Contains information about a host function argument.