Skip to content

Commit 394dd8d

Browse files
stuartmorgan-gstuartmorgan-g
authored
[pigeon] Fix Object arguments in Swift and C++ (#3020)
Fixes a warning in generated Swift output when an argument is of type Object. This is blocking flutter/plugins#6914 since we check our macOS and iOS plugin code for warnings in CI. Rather than add a Dart generator unit test for this one specific case, I tightened the Swift compilation settings for our test plugin to treat warnings as errors (per flutter/flutter#59116 (comment)) to catch the entire class of errors, and added echo* variants for Object to make sure this one then showed up. Incidental fixes: I had to make a similar fix to the Dart generator for a similar warning with casting to Object?, which we'd never noticed because we weren't analyzing any generated code that returning Object or Object? before. I had to make a change to the C++ generator so that generation would succeed, because it turned out we had no handling at all of Object in the C++ generator, causing it to throw. I'm not sure this is the output I'll keep for C++ (thus the TODO), but it's the simple fix to make it work at all. Fixes flutter/flutter#117994 Part of flutter/flutter#59116
1 parent 3ef0f63 commit 394dd8d

File tree

27 files changed

+546
-88
lines changed

27 files changed

+546
-88
lines changed

‎packages/pigeon/CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
## 4.2.16
2+
3+
* [swift] Fixes warnings with `Object` parameters.
4+
* [dart] Fixes warnings with `Object` return values.
5+
* [c++] Generation of APIs that use `Object` no longer fails.
6+
17
## 4.2.15
28

39
* Relocates generator classes. (Reverted)

‎packages/pigeon/lib/cpp_generator.dart

+15
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,11 @@ const flutter::StandardMessageCodec& ${api.name}::GetCodec() {
547547
// ... then declare the arg as a reference to that local.
548548
indent.writeln(
549549
'const auto* $argName = $encodableArgName.IsNull() ? nullptr : &$valueVarName;');
550+
} else if (hostType.datatype == 'flutter::EncodableValue') {
551+
// Generic objects just pass the EncodableValue through
552+
// directly.
553+
indent.writeln(
554+
'const auto* $argName = &$encodableArgName;');
550555
} else if (hostType.isBuiltin) {
551556
indent.writeln(
552557
'const auto* $argName = std::get_if<${hostType.datatype}>(&$encodableArgName);');
@@ -564,6 +569,13 @@ const flutter::StandardMessageCodec& ${api.name}::GetCodec() {
564569
// requires an int64_t so that it can handle any case.
565570
indent.writeln(
566571
'const int64_t $argName = $encodableArgName.LongValue();');
572+
} else if (hostType.datatype == 'flutter::EncodableValue') {
573+
// Generic objects just pass the EncodableValue through
574+
// directly. This creates an alias just to avoid having to
575+
// special-case the argName/encodableArgName distinction
576+
// at a higher level.
577+
indent
578+
.writeln('const auto& $argName = $encodableArgName;');
567579
} else if (hostType.isBuiltin) {
568580
indent.writeln(
569581
'const auto& $argName = std::get<${hostType.datatype}>($encodableArgName);');
@@ -902,6 +914,7 @@ String? _baseCppTypeForBuiltinDartType(TypeDeclaration type) {
902914
'Float64List': 'std::vector<double>',
903915
'Map': 'flutter::EncodableMap',
904916
'List': 'flutter::EncodableList',
917+
'Object': 'flutter::EncodableValue',
905918
};
906919
if (cppTypeForDartTypeMap.containsKey(type.baseName)) {
907920
return cppTypeForDartTypeMap[type.baseName];
@@ -931,6 +944,8 @@ String _unownedArgumentType(HostDatatype type) {
931944
if (isString || _isPodType(type)) {
932945
return type.isNullable ? 'const $baseType*' : baseType;
933946
}
947+
// TODO(stuartmorgan): Consider special-casing `Object?` here, so that there
948+
// aren't two ways of representing null (nullptr or an isNull EncodableValue).
934949
return type.isNullable ? 'const $baseType*' : 'const $baseType&';
935950
}
936951

‎packages/pigeon/lib/dart_generator.dart

+8-4
Original file line numberDiff line numberDiff line change
@@ -230,13 +230,17 @@ final BinaryMessenger? _binaryMessenger;
230230
indent.writeln('binaryMessenger: _binaryMessenger);');
231231
});
232232
final String returnType = _makeGenericTypeArguments(func.returnType);
233-
final String castCall = _makeGenericCastCall(func.returnType);
233+
final String genericCastCall = _makeGenericCastCall(func.returnType);
234234
const String accessor = 'replyList[0]';
235-
final String nullHandler =
236-
func.returnType.isNullable ? (castCall.isEmpty ? '' : '?') : '!';
235+
// Avoid warnings from pointlessly casting to `Object?`.
236+
final String nullablyTypedAccessor =
237+
returnType == 'Object' ? accessor : '($accessor as $returnType?)';
238+
final String nullHandler = func.returnType.isNullable
239+
? (genericCastCall.isEmpty ? '' : '?')
240+
: '!';
237241
final String returnStatement = func.returnType.isVoid
238242
? 'return;'
239-
: 'return ($accessor as $returnType?)$nullHandler$castCall;';
243+
: 'return $nullablyTypedAccessor$nullHandler$genericCastCall;';
240244
indent.format('''
241245
final List<Object?>? replyList =
242246
\t\tawait channel.send($sendArgument) as List<Object?>?;

‎packages/pigeon/lib/generator_tools.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'dart:mirrors';
99
import 'ast.dart';
1010

1111
/// The current version of pigeon. This must match the version in pubspec.yaml.
12-
const String pigeonVersion = '4.2.15';
12+
const String pigeonVersion = '4.2.16';
1313

1414
/// Read all the content from [stdin] to a String.
1515
String readStdin() {

‎packages/pigeon/lib/swift_generator.dart

+3
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,9 @@ String _castForceUnwrap(String value, TypeDeclaration type, Root root) {
365365
final String nullableConditionPrefix =
366366
type.isNullable ? '$value == nil ? nil : ' : '';
367367
return '$nullableConditionPrefix${_swiftTypeForDartType(type)}(rawValue: $value as! Int)$forceUnwrap';
368+
} else if (type.baseName == 'Object') {
369+
// Special-cased to avoid warnings about using 'as' with Any.
370+
return type.isNullable ? value : '$value!';
368371
} else {
369372
final String castUnwrap = type.isNullable ? '?' : '!';
370373
return '$value as$castUnwrap ${_swiftTypeForDartType(type)}';

‎packages/pigeon/pigeons/core_tests.dart

+8
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ abstract class HostIntegrationCoreApi {
125125
@ObjCSelector('echoUint8List:')
126126
Uint8List echoUint8List(Uint8List aUint8List);
127127

128+
/// Returns the passed in generic Object.
129+
@ObjCSelector('echoObject:')
130+
Object echoObject(Object anObject);
131+
128132
// ========== Syncronous nullable method tests ==========
129133

130134
/// Returns the inner `aString` value from the wrapped object, to test
@@ -162,6 +166,10 @@ abstract class HostIntegrationCoreApi {
162166
@ObjCSelector('echoNullableUint8List:')
163167
Uint8List? echoNullableUint8List(Uint8List? aNullableUint8List);
164168

169+
/// Returns the passed in generic Object.
170+
@ObjCSelector('echoNullableObject:')
171+
Object? echoNullableObject(Object? aNullableObject);
172+
165173
// ========== Asyncronous method tests ==========
166174

167175
/// A no-op function taking no arguments and returning no value, to sanity

‎packages/pigeon/platform_tests/alternate_language_test_plugin/android/src/main/java/com/example/alternate_language_test_plugin/AlternateLanguageTestPlugin.java

+10
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ public byte[] echoUint8List(@NonNull byte[] aUint8List) {
7272
return aUint8List;
7373
}
7474

75+
@Override
76+
public @NonNull Object echoObject(@NonNull Object anObject) {
77+
return anObject;
78+
}
79+
7580
@Override
7681
public @Nullable String extractNestedNullableString(@NonNull AllNullableTypesWrapper wrapper) {
7782
return wrapper.getValues().getANullableString();
@@ -124,6 +129,11 @@ public byte[] echoUint8List(@NonNull byte[] aUint8List) {
124129
return aNullableUint8List;
125130
}
126131

132+
@Override
133+
public @Nullable Object echoNullableObject(@Nullable Object aNullableObject) {
134+
return aNullableObject;
135+
}
136+
127137
@Override
128138
public void noopAsync(Result<Void> result) {
129139
result.success(null);

‎packages/pigeon/platform_tests/alternate_language_test_plugin/android/src/main/java/com/example/alternate_language_test_plugin/CoreTests.java

+65-10
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v4.2.15), do not edit directly.
5+
// Autogenerated from Pigeon (v4.2.16), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77

88
package com.example.alternate_language_test_plugin;
@@ -713,12 +713,9 @@ protected Object readValueOfType(byte type, @NonNull ByteBuffer buffer) {
713713
return AllNullableTypes.fromList((ArrayList<Object>) readValue(buffer));
714714

715715
case (byte) 129:
716-
return AllNullableTypes.fromList((ArrayList<Object>) readValue(buffer));
717-
718-
case (byte) 130:
719716
return AllNullableTypesWrapper.fromList((ArrayList<Object>) readValue(buffer));
720717

721-
case (byte) 131:
718+
case (byte) 130:
722719
return AllTypes.fromList((ArrayList<Object>) readValue(buffer));
723720

724721
default:
@@ -731,14 +728,11 @@ protected void writeValue(@NonNull ByteArrayOutputStream stream, Object value) {
731728
if (value instanceof AllNullableTypes) {
732729
stream.write(128);
733730
writeValue(stream, ((AllNullableTypes) value).toList());
734-
} else if (value instanceof AllNullableTypes) {
735-
stream.write(129);
736-
writeValue(stream, ((AllNullableTypes) value).toList());
737731
} else if (value instanceof AllNullableTypesWrapper) {
738-
stream.write(130);
732+
stream.write(129);
739733
writeValue(stream, ((AllNullableTypesWrapper) value).toList());
740734
} else if (value instanceof AllTypes) {
741-
stream.write(131);
735+
stream.write(130);
742736
writeValue(stream, ((AllTypes) value).toList());
743737
} else {
744738
super.writeValue(stream, value);
@@ -780,6 +774,9 @@ public interface HostIntegrationCoreApi {
780774
/** Returns the passed in Uint8List. */
781775
@NonNull
782776
byte[] echoUint8List(@NonNull byte[] aUint8List);
777+
/** Returns the passed in generic Object. */
778+
@NonNull
779+
Object echoObject(@NonNull Object anObject);
783780
/**
784781
* Returns the inner `aString` value from the wrapped object, to test sending of nested objects.
785782
*/
@@ -811,6 +808,9 @@ AllNullableTypes sendMultipleNullableTypes(
811808
/** Returns the passed in Uint8List. */
812809
@Nullable
813810
byte[] echoNullableUint8List(@Nullable byte[] aNullableUint8List);
811+
/** Returns the passed in generic Object. */
812+
@Nullable
813+
Object echoNullableObject(@Nullable Object aNullableObject);
814814
/**
815815
* A no-op function taking no arguments and returning no value, to sanity test basic
816816
* asynchronous calling.
@@ -1072,6 +1072,35 @@ static void setup(BinaryMessenger binaryMessenger, HostIntegrationCoreApi api) {
10721072
channel.setMessageHandler(null);
10731073
}
10741074
}
1075+
{
1076+
BasicMessageChannel<Object> channel =
1077+
new BasicMessageChannel<>(
1078+
binaryMessenger,
1079+
"dev.flutter.pigeon.HostIntegrationCoreApi.echoObject",
1080+
getCodec());
1081+
if (api != null) {
1082+
channel.setMessageHandler(
1083+
(message, reply) -> {
1084+
ArrayList wrapped = new ArrayList<>();
1085+
try {
1086+
ArrayList<Object> args = (ArrayList<Object>) message;
1087+
assert args != null;
1088+
Object anObjectArg = args.get(0);
1089+
if (anObjectArg == null) {
1090+
throw new NullPointerException("anObjectArg unexpectedly null.");
1091+
}
1092+
Object output = api.echoObject(anObjectArg);
1093+
wrapped.add(0, output);
1094+
} catch (Error | RuntimeException exception) {
1095+
ArrayList<Object> wrappedError = wrapError(exception);
1096+
wrapped = wrappedError;
1097+
}
1098+
reply.reply(wrapped);
1099+
});
1100+
} else {
1101+
channel.setMessageHandler(null);
1102+
}
1103+
}
10751104
{
10761105
BasicMessageChannel<Object> channel =
10771106
new BasicMessageChannel<>(
@@ -1292,6 +1321,32 @@ static void setup(BinaryMessenger binaryMessenger, HostIntegrationCoreApi api) {
12921321
channel.setMessageHandler(null);
12931322
}
12941323
}
1324+
{
1325+
BasicMessageChannel<Object> channel =
1326+
new BasicMessageChannel<>(
1327+
binaryMessenger,
1328+
"dev.flutter.pigeon.HostIntegrationCoreApi.echoNullableObject",
1329+
getCodec());
1330+
if (api != null) {
1331+
channel.setMessageHandler(
1332+
(message, reply) -> {
1333+
ArrayList wrapped = new ArrayList<>();
1334+
try {
1335+
ArrayList<Object> args = (ArrayList<Object>) message;
1336+
assert args != null;
1337+
Object aNullableObjectArg = args.get(0);
1338+
Object output = api.echoNullableObject(aNullableObjectArg);
1339+
wrapped.add(0, output);
1340+
} catch (Error | RuntimeException exception) {
1341+
ArrayList<Object> wrappedError = wrapError(exception);
1342+
wrapped = wrappedError;
1343+
}
1344+
reply.reply(wrapped);
1345+
});
1346+
} else {
1347+
channel.setMessageHandler(null);
1348+
}
1349+
}
12951350
{
12961351
BasicMessageChannel<Object> channel =
12971352
new BasicMessageChannel<>(

‎packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/AlternateLanguageTestPlugin.m

+9
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ - (nullable FlutterStandardTypedData *)echoUint8List:(FlutterStandardTypedData *
6363
return aUint8List;
6464
}
6565

66+
- (nullable id)echoObject:(id)anObject error:(FlutterError *_Nullable *_Nonnull)error {
67+
return anObject;
68+
}
69+
6670
- (nullable NSString *)extractNestedNullableStringFrom:(AllNullableTypesWrapper *)wrapper
6771
error:(FlutterError *_Nullable *_Nonnull)error {
6872
return wrapper.values.aNullableString;
@@ -114,6 +118,11 @@ - (nullable NSString *)echoNullableString:(nullable NSString *)aNullableString
114118
return aNullableUint8List;
115119
}
116120

121+
- (nullable id)echoNullableObject:(nullable id)aNullableObject
122+
error:(FlutterError *_Nullable *_Nonnull)error {
123+
return aNullableObject;
124+
}
125+
117126
- (void)noopAsyncWithCompletion:(void (^)(FlutterError *_Nullable))completion {
118127
completion(nil);
119128
}

‎packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/CoreTests.gen.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v4.2.15), do not edit directly.
5+
// Autogenerated from Pigeon (v4.2.16), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
#import <Foundation/Foundation.h>
88
@protocol FlutterBinaryMessenger;
@@ -131,6 +131,10 @@ NSObject<FlutterMessageCodec> *HostIntegrationCoreApiGetCodec(void);
131131
/// @return `nil` only when `error != nil`.
132132
- (nullable FlutterStandardTypedData *)echoUint8List:(FlutterStandardTypedData *)aUint8List
133133
error:(FlutterError *_Nullable *_Nonnull)error;
134+
/// Returns the passed in generic Object.
135+
///
136+
/// @return `nil` only when `error != nil`.
137+
- (nullable id)echoObject:(id)anObject error:(FlutterError *_Nullable *_Nonnull)error;
134138
/// Returns the inner `aString` value from the wrapped object, to test
135139
/// sending of nested objects.
136140
- (nullable NSString *)extractNestedNullableStringFrom:(AllNullableTypesWrapper *)wrapper
@@ -166,6 +170,9 @@ NSObject<FlutterMessageCodec> *HostIntegrationCoreApiGetCodec(void);
166170
- (nullable FlutterStandardTypedData *)
167171
echoNullableUint8List:(nullable FlutterStandardTypedData *)aNullableUint8List
168172
error:(FlutterError *_Nullable *_Nonnull)error;
173+
/// Returns the passed in generic Object.
174+
- (nullable id)echoNullableObject:(nullable id)aNullableObject
175+
error:(FlutterError *_Nullable *_Nonnull)error;
169176
/// A no-op function taking no arguments and returning no value, to sanity
170177
/// test basic asynchronous calling.
171178
- (void)noopAsyncWithCompletion:(void (^)(FlutterError *_Nullable))completion;

0 commit comments

Comments
 (0)