[pigeon] Fix C++ generator's handling of non-class host APIs

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 3.2.4
+
+* [c++] Fixes most non-class arguments and return values in host APIs. The
+  types of arguments and return values have changed, so this may require updates
+  to existing code.
+
 ## 3.2.3
 
 * Adds `unnecessary_import` to linter ignore list in generated dart tests.

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.2.3';
+const String pigeonVersion = '3.2.4';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {
@@ -172,39 +172,58 @@ class HostDatatype {
   final bool isNullable;
 }
 
-/// 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,
+/// 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 getFieldHostDatatype(NamedType field, List<Class> classes,
+    List<Enum> enums, String? Function(TypeDeclaration) builtinResolver,
     {String Function(String)? customResolver}) {
-  final String? datatype = builtinResolver(field);
+  return _getHostDatatype(field.type, classes, enums, builtinResolver,
+      customResolver: customResolver, fieldName: field.name);
+}
+
+/// Calculates the [HostDatatype] for the provided [TypeDeclaration].
+///
+/// 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(TypeDeclaration type, List<Class> classes,
+    List<Enum> enums, String? Function(TypeDeclaration) builtinResolver,
+    {String Function(String)? customResolver}) {
+  return _getHostDatatype(type, classes, enums, builtinResolver,
+      customResolver: customResolver);
+}
+
+HostDatatype _getHostDatatype(TypeDeclaration type, List<Class> classes,
+    List<Enum> enums, String? Function(TypeDeclaration) builtinResolver,
+    {String Function(String)? customResolver, String? fieldName}) {
+  final String? datatype = builtinResolver(type);
   if (datatype == null) {
-    if (classes.map((Class x) => x.name).contains(field.type.baseName)) {
+    if (classes.map((Class x) => x.name).contains(type.baseName)) {
       final String customName = customResolver != null
-          ? customResolver(field.type.baseName)
-          : field.type.baseName;
+          ? customResolver(type.baseName)
+          : type.baseName;
       return HostDatatype(
-          datatype: customName,
-          isBuiltin: false,
-          isNullable: field.type.isNullable);
-    } else if (enums.map((Enum x) => x.name).contains(field.type.baseName)) {
+          datatype: customName, isBuiltin: false, isNullable: type.isNullable);
+    } else if (enums.map((Enum x) => x.name).contains(type.baseName)) {
       final String customName = customResolver != null
-          ? customResolver(field.type.baseName)
-          : field.type.baseName;
+          ? customResolver(type.baseName)
+          : type.baseName;
       return HostDatatype(
-          datatype: customName,
-          isBuiltin: false,
-          isNullable: field.type.isNullable);
+          datatype: customName, isBuiltin: false, isNullable: type.isNullable);
     } else {
       throw Exception(
-          'unrecognized datatype for field:"${field.name}" of type:"${field.type.baseName}"');
+          'unrecognized datatype ${fieldName == null ? '' : 'for field:"$fieldName" '}of type:"${type.baseName}"');
     }
   } else {
     return HostDatatype(
-        datatype: datatype, isBuiltin: true, isNullable: field.type.isNullable);
+        datatype: datatype, isBuiltin: true, isNullable: type.isNullable);
   }
 }
 

packages/pigeon/lib/java_generator.dart

@@ -451,8 +451,8 @@ String _nullsafeJavaTypeForDartType(TypeDeclaration type) {
 /// object.
 String _castObject(
     NamedType field, List<Class> classes, List<Enum> enums, String varName) {
-  final HostDatatype hostDatatype = getHostDatatype(field, classes, enums,
-      (NamedType x) => _javaTypeForBuiltinDartType(x.type));
+  final HostDatatype hostDatatype = getFieldHostDatatype(field, classes, enums,
+      (TypeDeclaration x) => _javaTypeForBuiltinDartType(x));
   if (field.type.baseName == 'int') {
     return '($varName == null) ? null : (($varName instanceof Integer) ? (Integer)$varName : (${hostDatatype.datatype})$varName)';
   } else if (!hostDatatype.isBuiltin &&
@@ -521,8 +521,11 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
 
   void writeDataClass(Class klass) {
     void writeField(NamedType field) {
-      final HostDatatype hostDatatype = getHostDatatype(field, root.classes,
-          root.enums, (NamedType x) => _javaTypeForBuiltinDartType(x.type));
+      final HostDatatype hostDatatype = getFieldHostDatatype(
+          field,
+          root.classes,
+          root.enums,
+          (TypeDeclaration x) => _javaTypeForBuiltinDartType(x));
       final String nullability =
           field.type.isNullable ? '@Nullable' : '@NonNull';
       indent.writeln(
@@ -547,8 +550,11 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
       indent.scoped('{', '}', () {
         indent.writeln('Map<String, Object> toMapResult = new HashMap<>();');
         for (final NamedType field in klass.fields) {
-          final HostDatatype hostDatatype = getHostDatatype(field, root.classes,
-              root.enums, (NamedType x) => _javaTypeForBuiltinDartType(x.type));
+          final HostDatatype hostDatatype = getFieldHostDatatype(
+              field,
+              root.classes,
+              root.enums,
+              (TypeDeclaration x) => _javaTypeForBuiltinDartType(x));
           String toWriteValue = '';
           final String fieldName = field.name;
           if (!hostDatatype.isBuiltin &&
@@ -592,8 +598,11 @@ void generateJava(JavaOptions options, Root root, StringSink sink) {
       indent.write('public static final class Builder ');
       indent.scoped('{', '}', () {
         for (final NamedType field in klass.fields) {
-          final HostDatatype hostDatatype = getHostDatatype(field, root.classes,
-              root.enums, (NamedType x) => _javaTypeForBuiltinDartType(x.type));
+          final HostDatatype hostDatatype = getFieldHostDatatype(
+              field,
+              root.classes,
+              root.enums,
+              (TypeDeclaration x) => _javaTypeForBuiltinDartType(x));
           final String nullability =
               field.type.isNullable ? '@Nullable' : '@NonNull';
           indent.writeln(

packages/pigeon/lib/objc_generator.dart

@@ -107,9 +107,10 @@ String _flattenTypeArguments(String? classPrefix, List<TypeDeclaration> args) {
   return result;
 }
 
-String? _objcTypePtrForPrimitiveDartType(String? classPrefix, NamedType field) {
-  return _objcTypeForDartTypeMap.containsKey(field.type.baseName)
-      ? _objcTypeForDartType(classPrefix, field.type).ptr
+String? _objcTypePtrForPrimitiveDartType(
+    String? classPrefix, TypeDeclaration type) {
+  return _objcTypeForDartTypeMap.containsKey(type.baseName)
+      ? _objcTypeForDartType(classPrefix, type).ptr
       : null;
 }
 
@@ -170,8 +171,11 @@ void _writeInitializerDeclaration(Indent indent, Class klass,
               indent.write(x);
             };
       isFirst = false;
-      final HostDatatype hostDatatype = getHostDatatype(field, classes, enums,
-          (NamedType x) => _objcTypePtrForPrimitiveDartType(prefix, x),
+      final HostDatatype hostDatatype = getFieldHostDatatype(
+          field,
+          classes,
+          enums,
+          (TypeDeclaration x) => _objcTypePtrForPrimitiveDartType(prefix, x),
           customResolver: enumNames.contains(field.type.baseName)
               ? (String x) => _className(prefix, x)
               : (String x) => '${_className(prefix, x)} *');
@@ -205,8 +209,11 @@ void _writeClassDeclarations(
       indent.addln(';');
     }
     for (final NamedType field in klass.fields) {
-      final HostDatatype hostDatatype = getHostDatatype(field, classes, enums,
-          (NamedType x) => _objcTypePtrForPrimitiveDartType(prefix, x),
+      final HostDatatype hostDatatype = getFieldHostDatatype(
+          field,
+          classes,
+          enums,
+          (TypeDeclaration x) => _objcTypePtrForPrimitiveDartType(prefix, x),
           customResolver: enumNames.contains(field.type.baseName)
               ? (String x) => _className(prefix, x)
               : (String x) => '${_className(prefix, x)} *');

packages/pigeon/lib/swift_generator.dart

@@ -408,8 +408,8 @@ void generateSwift(SwiftOptions options, Root root, StringSink sink) {
   final Indent indent = Indent(sink);
 
   HostDatatype _getHostDatatype(NamedType field) {
-    return getHostDatatype(field, root.classes, root.enums,
-        (NamedType x) => _swiftTypeForBuiltinDartType(x.type));
+    return getFieldHostDatatype(field, root.classes, root.enums,
+        (TypeDeclaration x) => _swiftTypeForBuiltinDartType(x));
   }
 
   void writeHeader() {

packages/pigeon/platform_tests/windows_unit_tests/windows/CMakeLists.txt

@@ -52,9 +52,14 @@ FetchContent_MakeAvailable(googletest)
 # The plugin's C API is not very useful for unit testing, so build the sources
 # directly into the test binary rather than using the DLL.
 add_executable(${TEST_RUNNER}
+  # Tests.
+  test/multiple_arity_test.cpp
   test/non_null_fields_test.cpp
+  test/nullable_returns_test.cpp
   test/null_fields_test.cpp
   test/pigeon_test.cpp
+  test/primitive_test.cpp
+  # Generated sources.
   test/all_datatypes.g.cpp
   test/all_datatypes.g.h
   test/all_void.g.cpp
@@ -87,6 +92,11 @@ add_executable(${TEST_RUNNER}
   test/voidflutter.g.h
   test/voidhost.g.cpp
   test/voidhost.g.h
+  # Test utilities.
+  test/utils/echo_messenger.cpp
+  test/utils/echo_messenger.h
+  test/utils/fake_host_messenger.cpp
+  test/utils/fake_host_messenger.h
 
   ${PLUGIN_SOURCES}
 )

packages/pigeon/platform_tests/windows_unit_tests/windows/test/multiple_arity_test.cpp

@@ -0,0 +1,52 @@
+// 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.
+
+#include <gtest/gtest.h>
+
+#include "test/multiple_arity.g.h"
+#include "test/utils/fake_host_messenger.h"
+
+namespace multiple_arity_pigeontest {
+
+namespace {
+using flutter::EncodableList;
+using flutter::EncodableMap;
+using flutter::EncodableValue;
+using testing::FakeHostMessenger;
+
+class TestHostApi : public MultipleArityHostApi {
+ public:
+  TestHostApi() {}
+  virtual ~TestHostApi() {}
+
+ protected:
+  ErrorOr<int64_t> Subtract(int64_t x, int64_t y) override { return x - y; }
+};
+
+const EncodableValue& GetResult(const EncodableValue& pigeon_response) {
+  return std::get<EncodableMap>(pigeon_response).at(EncodableValue("result"));
+}
+}  // namespace
+
+TEST(MultipleArity, HostSimple) {
+  FakeHostMessenger messenger(&MultipleArityHostApi::GetCodec());
+  TestHostApi api;
+  MultipleArityHostApi::SetUp(&messenger, &api);
+
+  int64_t result = 0;
+  messenger.SendHostMessage("dev.flutter.pigeon.MultipleArityHostApi.subtract",
+                            EncodableValue(EncodableList({
+                                EncodableValue(30),
+                                EncodableValue(10),
+                            })),
+                            [&result](const EncodableValue& reply) {
+                              result = GetResult(reply).LongValue();
+                            });
+
+  EXPECT_EQ(result, 20);
+}
+
+// TODO(stuartmorgan): Add a FlutterApi version of the test.
+
+}  // namespace multiple_arity_pigeontest

packages/pigeon/platform_tests/windows_unit_tests/windows/test/nullable_returns_test.cpp

@@ -0,0 +1,112 @@
+// 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.
+
+#include <gtest/gtest.h>
+
+#include <optional>
+
+#include "test/nullable_returns.g.h"
+#include "test/utils/fake_host_messenger.h"
+
+namespace nullable_returns_pigeontest {
+
+namespace {
+using flutter::EncodableList;
+using flutter::EncodableMap;
+using flutter::EncodableValue;
+using testing::FakeHostMessenger;
+
+class TestNullableArgHostApi : public NullableArgHostApi {
+ public:
+  TestNullableArgHostApi() {}
+  virtual ~TestNullableArgHostApi() {}
+
+ protected:
+  ErrorOr<int64_t> Doit(const int64_t* x) override {
+    return x == nullptr ? 42 : *x;
+  }
+};
+
+class TestNullableReturnHostApi : public NullableReturnHostApi {
+ public:
+  TestNullableReturnHostApi(std::optional<int64_t> return_value)
+      : value_(return_value) {}
+  virtual ~TestNullableReturnHostApi() {}
+
+ protected:
+  ErrorOr<std::optional<int64_t>> Doit() override { return value_; }
+
+ private:
+  std::optional<int64_t> value_;
+};
+
+const EncodableValue& GetResult(const EncodableValue& pigeon_response) {
+  return std::get<EncodableMap>(pigeon_response).at(EncodableValue("result"));
+}
+}  // namespace
+
+TEST(NullableReturns, HostNullableArgNull) {
+  FakeHostMessenger messenger(&NullableArgHostApi::GetCodec());
+  TestNullableArgHostApi api;
+  NullableArgHostApi::SetUp(&messenger, &api);
+
+  int64_t result = 0;
+  messenger.SendHostMessage("dev.flutter.pigeon.NullableArgHostApi.doit",
+                            EncodableValue(EncodableList({EncodableValue()})),
+                            [&result](const EncodableValue& reply) {
+                              result = GetResult(reply).LongValue();
+                            });
+
+  EXPECT_EQ(result, 42);
+}
+
+TEST(NullableReturns, HostNullableArgNonNull) {
+  FakeHostMessenger messenger(&NullableArgHostApi::GetCodec());
+  TestNullableArgHostApi api;
+  NullableArgHostApi::SetUp(&messenger, &api);
+
+  int64_t result = 0;
+  messenger.SendHostMessage("dev.flutter.pigeon.NullableArgHostApi.doit",
+                            EncodableValue(EncodableList({EncodableValue(7)})),
+                            [&result](const EncodableValue& reply) {
+                              result = GetResult(reply).LongValue();
+                            });
+
+  EXPECT_EQ(result, 7);
+}
+
+TEST(NullableReturns, HostNullableReturnNull) {
+  FakeHostMessenger messenger(&NullableReturnHostApi::GetCodec());
+  TestNullableReturnHostApi api(std::nullopt);
+  NullableReturnHostApi::SetUp(&messenger, &api);
+
+  // Initialize to a non-null value to ensure that it's actually set to null,
+  // rather than just never set.
+  EncodableValue result(true);
+  messenger.SendHostMessage(
+      "dev.flutter.pigeon.NullableReturnHostApi.doit",
+      EncodableValue(EncodableList({})),
+      [&result](const EncodableValue& reply) { result = GetResult(reply); });
+
+  EXPECT_TRUE(result.IsNull());
+}
+
+TEST(NullableReturns, HostNullableReturnNonNull) {
+  FakeHostMessenger messenger(&NullableReturnHostApi::GetCodec());
+  TestNullableReturnHostApi api(42);
+  NullableReturnHostApi::SetUp(&messenger, &api);
+
+  EncodableValue result;
+  messenger.SendHostMessage(
+      "dev.flutter.pigeon.NullableReturnHostApi.doit",
+      EncodableValue(EncodableList({})),
+      [&result](const EncodableValue& reply) { result = GetResult(reply); });
+
+  EXPECT_FALSE(result.IsNull());
+  EXPECT_EQ(result.LongValue(), 42);
+}
+
+// TODO(stuartmorgan): Add FlutterApi versions of the tests.
+
+}  // namespace nullable_returns_pigeontest