[pigeon] Don't wrap non-nullable primitives in Obj-C (#5214)

Updates the Objective-C generator to use `BOOL`, `NSInteger`, and `double` for non-nullable versions of those primitives, instead of boxing them with `NSNumber*`. This makes the code more idiomatic, brings them into alignment with enum behavior, and importantly helps minimize the chances of clients having issues with silent conversion of `NSNumber*` to `BOOL` in the long term (although it may cause some such issues in the short term when people update; the changelog contains a prominent warning about this). As an incidental fix, this also fixes the use of `strong` for collections and strings, which is not best practice, since I was already changing related code. Fixes flutter/flutter#116680 Fixes flutter/flutter#127401
flutter · Oct 30, 2023 · 7319d3a · 7319d3a
1 parent 35b15de
commit 7319d3a
Show file tree

Hide file tree

Showing 20 changed files with 484 additions and 379 deletions.
diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md
@@ -1,3 +1,22 @@
+## 13.0.0
+
+* **Breaking Change** [objc] Eliminates boxing of non-nullable primitive types
+  (bool, int, double). Changes required:
+  * Implementations of host API methods that take non-nullable
+    primitives will need to be updated to match the new signatures.
+  * Calls to Flutter API methods that take non-nullable primitives will need to
+    be updated to pass unboxed values.
+  * Calls to non-nullable primitive property methods on generated data classes
+    will need to be updated.
+  * **WARNING**: Current versions of `Xcode` do not appear to warn about
+    implicit `NSNumber *` to `BOOL` conversions, so code that is no longer
+    correct after this breaking change may compile without warning. For example,
+    `myGeneratedClass.aBoolProperty = @NO` can silently set `aBoolProperty` to
+    `YES`. Any data class or Flutter API interactions involving `bool`s should
+    be carefully audited by hand when updating.
+
+
+
 ## 12.0.1
 
 * [swift] Adds protocol for Flutter APIs.

diff --git a/packages/pigeon/example/app/macos/Runner/messages.g.h b/packages/pigeon/example/app/macos/Runner/messages.g.h
@@ -36,7 +36,7 @@ typedef NS_ENUM(NSUInteger, PGNCode) {
 @property(nonatomic, copy, nullable) NSString *name;
 @property(nonatomic, copy, nullable) NSString *description;
 @property(nonatomic, assign) PGNCode code;
-@property(nonatomic, strong) NSDictionary<NSString *, NSString *> *data;
+@property(nonatomic, copy) NSDictionary<NSString *, NSString *> *data;
 @end
 
 /// The codec used by PGNExampleHostApi.
@@ -46,8 +46,8 @@ NSObject<FlutterMessageCodec> *PGNExampleHostApiGetCodec(void);
 /// @return `nil` only when `error != nil`.
 - (nullable NSString *)getHostLanguageWithError:(FlutterError *_Nullable *_Nonnull)error;
 /// @return `nil` only when `error != nil`.
-- (nullable NSNumber *)addNumber:(NSNumber *)a
-                        toNumber:(NSNumber *)b
+- (nullable NSNumber *)addNumber:(NSInteger)a
+                        toNumber:(NSInteger)b
                            error:(FlutterError *_Nullable *_Nonnull)error;
 - (void)sendMessageMessage:(PGNMessageData *)message
                 completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion;

diff --git a/packages/pigeon/example/app/macos/Runner/messages.g.m b/packages/pigeon/example/app/macos/Runner/messages.g.m
@@ -63,18 +63,17 @@ + (PGNMessageData *)fromList:(NSArray *)list {
   pigeonResult.description = GetNullableObjectAtIndex(list, 1);
   pigeonResult.code = [GetNullableObjectAtIndex(list, 2) integerValue];
   pigeonResult.data = GetNullableObjectAtIndex(list, 3);
-  NSAssert(pigeonResult.data != nil, @"");
   return pigeonResult;
 }
 + (nullable PGNMessageData *)nullableFromList:(NSArray *)list {
   return (list) ? [PGNMessageData fromList:list] : nil;
 }
 - (NSArray *)toList {
   return @[
-    (self.name ?: [NSNull null]),
-    (self.description ?: [NSNull null]),
+    self.name ?: [NSNull null],
+    self.description ?: [NSNull null],
     @(self.code),
-    (self.data ?: [NSNull null]),
+    self.data ?: [NSNull null],
   ];
 }
 @end
@@ -160,8 +159,8 @@ void SetUpPGNExampleHostApi(id<FlutterBinaryMessenger> binaryMessenger,
           api);
       [channel setMessageHandler:^(id _Nullable message, FlutterReply callback) {
         NSArray *args = message;
-        NSNumber *arg_a = GetNullableObjectAtIndex(args, 0);
-        NSNumber *arg_b = GetNullableObjectAtIndex(args, 1);
+        NSInteger arg_a = [GetNullableObjectAtIndex(args, 0) integerValue];
+        NSInteger arg_b = [GetNullableObjectAtIndex(args, 1) integerValue];
         FlutterError *error;
         NSNumber *output = [api addNumber:arg_a toNumber:arg_b error:&error];
         callback(wrapResult(output, error));

diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart
@@ -13,7 +13,7 @@ import 'ast.dart';
 /// The current version of pigeon.
 ///
 /// This must match the version in pubspec.yaml.
-const String pigeonVersion = '12.0.1';
+const String pigeonVersion = '13.0.0';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {

diff --git a/packages/pigeon/lib/objc_generator.dart b/packages/pigeon/lib/objc_generator.dart
diff --git a/...platform_tests/alternate_language_test_plugin/example/ios/RunnerTests/MultipleArityTest.m b/...platform_tests/alternate_language_test_plugin/example/ios/RunnerTests/MultipleArityTest.m
@@ -25,8 +25,8 @@ - (void)testSimple {
   MultipleArityFlutterApi *api =
       [[MultipleArityFlutterApi alloc] initWithBinaryMessenger:binaryMessenger];
   XCTestExpectation *expectation = [self expectationWithDescription:@"subtraction"];
-  [api subtractX:@(30)
-               y:@(10)
+  [api subtractX:30
+               y:10
       completion:^(NSNumber *_Nonnull result, FlutterError *_Nullable error) {
         XCTAssertNil(error);
         XCTAssertEqual(20, result.intValue);

diff --git a/...on/platform_tests/alternate_language_test_plugin/example/ios/RunnerTests/NullFieldsTest.m b/...on/platform_tests/alternate_language_test_plugin/example/ios/RunnerTests/NullFieldsTest.m
@@ -29,7 +29,7 @@ @interface NullFieldsTest : XCTestCase
 @implementation NullFieldsTest
 
 - (void)testMakeWithValues {
-  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:@"hello" identifier:@1];
+  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:@"hello" identifier:1];
 
   NullFieldsSearchReplyTypeBox *typeWrapper =
       [[NullFieldsSearchReplyTypeBox alloc] initWithValue:NullFieldsSearchReplyTypeSuccess];
@@ -48,7 +48,7 @@ - (void)testMakeWithValues {
 }
 
 - (void)testMakeRequestWithNulls {
-  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:nil identifier:@1];
+  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:nil identifier:1];
   XCTAssertNil(request.query);
 }
 
@@ -121,13 +121,13 @@ - (void)testReplyFromListWithNulls {
 }
 
 - (void)testRequestToListWithValuess {
-  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:@"hello" identifier:@1];
+  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:@"hello" identifier:1];
   NSArray *list = [request toList];
   XCTAssertEqual(@"hello", list[0]);
 }
 
 - (void)testRequestToListWithNulls {
-  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:nil identifier:@1];
+  NullFieldsSearchRequest *request = [NullFieldsSearchRequest makeWithQuery:nil identifier:1];
   NSArray *list = [request toList];
   XCTAssertEqual([NSNull null], list[0]);
 }
@@ -139,7 +139,7 @@ - (void)testReplyToListWithValuess {
       makeWithResult:@"result"
                error:@"error"
              indices:@[ @1, @2, @3 ]
-             request:[NullFieldsSearchRequest makeWithQuery:@"hello" identifier:@1]
+             request:[NullFieldsSearchRequest makeWithQuery:@"hello" identifier:1]
                 type:typeWrapper];
   NSArray *list = [reply toList];
   NSArray *indices = @[ @1, @2, @3 ];

diff --git a/...eon/platform_tests/alternate_language_test_plugin/example/ios/RunnerTests/PrimitiveTest.m b/...eon/platform_tests/alternate_language_test_plugin/example/ios/RunnerTests/PrimitiveTest.m
@@ -21,7 +21,7 @@ - (void)testIntPrimitive {
       [[EchoBinaryMessenger alloc] initWithCodec:PrimitiveFlutterApiGetCodec()];
   PrimitiveFlutterApi *api = [[PrimitiveFlutterApi alloc] initWithBinaryMessenger:binaryMessenger];
   XCTestExpectation *expectation = [self expectationWithDescription:@"callback"];
-  [api anIntValue:@1
+  [api anIntValue:1
        completion:^(NSNumber *_Nonnull result, FlutterError *_Nullable err) {
          XCTAssertEqualObjects(@1, result);
          [expectation fulfill];
@@ -34,10 +34,11 @@ - (void)testBoolPrimitive {
       [[EchoBinaryMessenger alloc] initWithCodec:PrimitiveFlutterApiGetCodec()];
   PrimitiveFlutterApi *api = [[PrimitiveFlutterApi alloc] initWithBinaryMessenger:binaryMessenger];
   XCTestExpectation *expectation = [self expectationWithDescription:@"callback"];
-  NSNumber *arg = @YES;
+  BOOL arg = YES;
   [api aBoolValue:arg
        completion:^(NSNumber *_Nonnull result, FlutterError *_Nullable err) {
-         XCTAssertEqualObjects(arg, result);
+         XCTAssertNotNil(result);
+         XCTAssertEqual(arg, result.boolValue);
          [expectation fulfill];
        }];
   [self waitForExpectations:@[ expectation ] timeout:1.0];
@@ -48,10 +49,11 @@ - (void)testDoublePrimitive {
       [[EchoBinaryMessenger alloc] initWithCodec:PrimitiveFlutterApiGetCodec()];
   PrimitiveFlutterApi *api = [[PrimitiveFlutterApi alloc] initWithBinaryMessenger:binaryMessenger];
   XCTestExpectation *expectation = [self expectationWithDescription:@"callback"];
-  NSNumber *arg = @(1.5);
+  NSInteger arg = 1.5;
   [api aDoubleValue:arg
          completion:^(NSNumber *_Nonnull result, FlutterError *_Nullable err) {
-           XCTAssertEqualObjects(arg, result);
+           XCTAssertNotNil(result);
+           XCTAssertEqual(arg, result.integerValue);
            [expectation fulfill];
          }];
   [self waitForExpectations:@[ expectation ] timeout:1.0];

diff --git a/...tform_tests/alternate_language_test_plugin/example/macos/Runner.xcodeproj/project.pbxproj b/...tform_tests/alternate_language_test_plugin/example/macos/Runner.xcodeproj/project.pbxproj
@@ -259,7 +259,7 @@
 			isa = PBXProject;
 			attributes = {
 				LastSwiftUpdateCheck = 0920;
-				LastUpgradeCheck = 1300;
+				LastUpgradeCheck = 1430;
 				ORGANIZATIONNAME = "";
 				TargetAttributes = {
 					331C80D4294CF70F00263BE5 = {

diff --git a/...anguage_test_plugin/example/macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme b/...anguage_test_plugin/example/macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1300"
+   LastUpgradeVersion = "1430"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"

diff --git a/...n/platform_tests/alternate_language_test_plugin/ios/Classes/AlternateLanguageTestPlugin.m b/...n/platform_tests/alternate_language_test_plugin/ios/Classes/AlternateLanguageTestPlugin.m
@@ -50,17 +50,16 @@ - (nullable id)throwFlutterErrorWithError:(FlutterError *_Nullable *_Nonnull)err
   return nil;
 }
 
-- (nullable NSNumber *)echoInt:(NSNumber *)anInt error:(FlutterError *_Nullable *_Nonnull)error {
-  return anInt;
+- (nullable NSNumber *)echoInt:(NSInteger)anInt error:(FlutterError *_Nullable *_Nonnull)error {
+  return @(anInt);
 }
 
-- (nullable NSNumber *)echoDouble:(NSNumber *)aDouble
-                            error:(FlutterError *_Nullable *_Nonnull)error {
-  return aDouble;
+- (nullable NSNumber *)echoDouble:(double)aDouble error:(FlutterError *_Nullable *_Nonnull)error {
+  return @(aDouble);
 }
 
-- (nullable NSNumber *)echoBool:(NSNumber *)aBool error:(FlutterError *_Nullable *_Nonnull)error {
-  return aBool;
+- (nullable NSNumber *)echoBool:(BOOL)aBool error:(FlutterError *_Nullable *_Nonnull)error {
+  return @(aBool);
 }
 
 - (nullable NSString *)echoString:(NSString *)aString
@@ -196,19 +195,19 @@ - (void)echoAsyncNullableAllNullableTypes:(nullable AllNullableTypes *)everythin
   completion(everything, nil);
 }
 
-- (void)echoAsyncInt:(NSNumber *)anInt
+- (void)echoAsyncInt:(NSInteger)anInt
           completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion {
-  completion(anInt, nil);
+  completion(@(anInt), nil);
 }
 
-- (void)echoAsyncDouble:(NSNumber *)aDouble
+- (void)echoAsyncDouble:(double)aDouble
              completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion {
-  completion(aDouble, nil);
+  completion(@(aDouble), nil);
 }
 
-- (void)echoAsyncBool:(NSNumber *)aBool
+- (void)echoAsyncBool:(BOOL)aBool
            completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion {
-  completion(aBool, nil);
+  completion(@(aBool), nil);
 }
 
 - (void)echoAsyncString:(NSString *)aString
@@ -331,23 +330,23 @@ - (void)callFlutterSendMultipleNullableTypesABool:(nullable NSNumber *)aNullable
                                        }];
 }
 
-- (void)callFlutterEchoBool:(NSNumber *)aBool
+- (void)callFlutterEchoBool:(BOOL)aBool
                  completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion {
   [self.flutterAPI echoBool:aBool
                  completion:^(NSNumber *value, FlutterError *error) {
                    completion(value, error);
                  }];
 }
 
-- (void)callFlutterEchoInt:(NSNumber *)anInt
+- (void)callFlutterEchoInt:(NSInteger)anInt
                 completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion {
   [self.flutterAPI echoInt:anInt
                 completion:^(NSNumber *value, FlutterError *error) {
                   completion(value, error);
                 }];
 }
 
-- (void)callFlutterEchoDouble:(NSNumber *)aDouble
+- (void)callFlutterEchoDouble:(double)aDouble
                    completion:(void (^)(NSNumber *_Nullable, FlutterError *_Nullable))completion {
   [self.flutterAPI echoDouble:aDouble
                    completion:^(NSNumber *value, FlutterError *error) {