Fix nullability bug

lib/src/code_generator/objc_interface.dart

@@ -258,6 +258,7 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
     if (superType != null) {
       superType!.addDependencies(dependencies);
       _copyMethodsFromSuperType();
+      _fixNullabilityOfOverriddenMethods();
     }
 
     for (final m in methods.values) {
@@ -281,6 +282,35 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
     }
   }
 
+  void _fixNullabilityOfOverriddenMethods() {
+    // ObjC ignores nullability when deciding if an override for an inherited
+    // method is valid. But in Dart it's invalid to override a method and change
+    // it's return type from non-null to nullable, or its arg type from nullable
+    // to non-null. So in these cases we have to make the non-null type
+    // nullable, to avoid Dart compile errors.
+    var superType_ = superType;
+    while (superType_ != null) {
+      for (final method in methods.values) {
+        final superMethod = superType_.methods[method.originalName];
+        if (superMethod != null) {
+          if (superMethod.returnType.typealiasType is! ObjCNullable && method.returnType.typealiasType is ObjCNullable) {
+            superMethod.returnType = ObjCNullable(superMethod.returnType);
+          }
+          final numArgs = method.params.length < superMethod.params.length ?
+              method.params.length : superMethod.params.length;
+          for (int i = 0; i < numArgs; ++i) {
+            final param = method.params[i];
+            final superParam = superMethod.params[i];
+            if (superParam.type.typealiasType is ObjCNullable && param.type.typealiasType is! ObjCNullable) {
+              param.type = ObjCNullable(param.type);
+            }
+          }
+        }
+      }
+      superType_ = superType_.superType;
+    }
+  }
+
   static bool _isInstanceType(Type type) =>
       type is ObjCInstanceType ||
       (type is ObjCNullable && type.child is ObjCInstanceType);
@@ -428,7 +458,7 @@ class ObjCMethod {
   final String? dartDoc;
   final String originalName;
   final ObjCProperty? property;
-  final Type returnType;
+  Type returnType;
   final List<ObjCMethodParam> params;
   final ObjCMethodKind kind;
   final bool isClass;
@@ -497,7 +527,7 @@ class ObjCMethod {
 }
 
 class ObjCMethodParam {
-  final Type type;
+  Type type;
   final String name;
   ObjCMethodParam(this.type, this.name);
 }

lib/src/code_generator/objc_nullable.dart

@@ -6,7 +6,8 @@ import 'package:ffigen/src/code_generator.dart';
 
 import 'writer.dart';
 
-/// An ObjC type annotated with nullable.
+/// An ObjC type annotated with nullable. Eg:
+/// +(nullable NSObject*) methodWithNullableResult;
 class ObjCNullable extends Type {
   Type child;
 

test/native_objc_test/nullable_config.yaml

@@ -1,5 +1,5 @@
 name: NullableTestObjCLibrary
-description: 'Tests calling Objective-C methods'
+description: 'Tests nullability for Objective-C methods'
 language: objc
 output: 'nullable_bindings.dart'
 exclude-all-by-default: true

test/native_objc_test/nullable_inheritance_config.yaml

@@ -0,0 +1,14 @@
+name: NullableInheritanceTestObjCLibrary
+description: 'Tests nullability of inherited Objective-C methods'
+language: objc
+output: 'nullable_inheritance_bindings.dart'
+exclude-all-by-default: true
+objc-interfaces:
+  include:
+    - NullableBase
+    - NullableChild
+headers:
+  entry-points:
+    - 'nullable_inheritance_test.m'
+preamble: |
+  // ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field

test/native_objc_test/nullable_inheritance_test.dart

@@ -0,0 +1,73 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Objective C support is only available on mac.
+@TestOn('mac-os')
+
+import 'dart:ffi';
+import 'dart:io';
+
+import 'package:test/test.dart';
+import '../test_utils.dart';
+import 'nullable_inheritance_bindings.dart';
+import 'util.dart';
+
+void main() {
+  late NullableInheritanceTestObjCLibrary lib;
+  late NullableBase nullableBase;
+  late NullableChild nullableChild;
+  late NSObject obj;
+  group('Nullable inheritance', () {
+    setUpAll(() {
+      logWarnings();
+      final dylib = File('test/native_objc_test/nullable_inheritance_test.dylib');
+      verifySetupFile(dylib);
+      lib = NullableInheritanceTestObjCLibrary(
+          DynamicLibrary.open(dylib.absolute.path));
+      nullableBase = NullableBase.new1(lib);
+      nullableChild = NullableChild.new1(lib);
+      obj = NSObject.new1(lib);
+      generateBindingsForCoverage('nullable');
+    });
+
+    group('Base', () {
+      test('Nullable arguments', () {
+        expect(nullableBase.nullableArg_(obj), false);
+        expect(nullableBase.nullableArg_(null), true);
+      });
+
+      test('Non-null arguments', () {
+        expect(nullableBase.nonNullArg_(obj), false);
+      });
+
+      test('Nullable return', () {
+        expect(nullableBase.nullableReturn_(false), isA<NSObject>());
+        expect(nullableBase.nullableReturn_(true), null);
+      });
+
+      test('Non-null return', () {
+        expect(nullableBase.nonNullReturn(), isA<NSObject>());
+      });
+    });
+
+    group('Child', () {
+      test('Nullable arguments, changed to non-null', () {
+        expect(nullableChild.nullableArg_(obj), false);
+      });
+
+      test('Non-null arguments, changed to nullable', () {
+        expect(nullableChild.nonNullArg_(obj), false);
+        expect(nullableChild.nonNullArg_(null), true);
+      });
+
+      test('Nullable return, changed to non-null', () {
+        expect(nullableChild.nullableReturn_(false), isA<NSObject>());
+      });
+
+      test('Non-null return, changed to nullable', () {
+        expect(nullableChild.nonNullReturn(), null);
+      });
+    });
+  });
+}

test/native_objc_test/nullable_inheritance_test.m

@@ -0,0 +1,67 @@
+#import <Foundation/NSObject.h>
+
+@interface NullableBase : NSObject {}
+
+-(BOOL) nullableArg:(nullable NSObject *)x;
+-(BOOL) nonNullArg:(NSObject *)x;
+-(nullable NSObject *) nullableReturn:(BOOL)r;
+-(NSObject*) nonNullReturn;
+
+@end
+
+@implementation NullableBase
+
+-(BOOL) nullableArg:(nullable NSObject *)x {
+  return x == NULL;
+}
+
+-(BOOL) nonNullArg:(NSObject *)x {
+  return x == NULL;
+}
+
+-(nullable NSObject *) nullableReturn:(BOOL)r {
+  if (r) {
+    return nil;
+  } else {
+    return [NSObject new];
+  }
+}
+
+-(NSObject *) nonNullReturn {
+  return [NSObject new];
+}
+
+@end
+
+@interface NullableIntermediate : NullableBase {}
+@end
+
+@interface NullableChild : NullableIntermediate {}
+
+// Redeclare the same methods with different nullability.
+-(BOOL) nullableArg:(NSObject *)x;
+-(BOOL) nonNullArg:(nullable NSObject *)x;
+-(NSObject *) nullableReturn:(BOOL)r;
+-(nullable NSObject *) nonNullReturn;
+
+@end
+
+@implementation NullableChild
+
+-(BOOL) nullableArg:(NSObject *)x {
+  return x == NULL;
+}
+
+-(BOOL) nonNullArg:(nullable NSObject *)x {
+  return x == NULL;
+}
+
+-(NSObject *) nullableReturn:(BOOL)r {
+  return [NSObject new];
+}
+
+-(nullable NSObject *) nonNullReturn {
+  return nil;
+}
+
+@end

test/native_objc_test/nullable_test.dart

@@ -15,9 +15,9 @@ import 'util.dart';
 
 void main() {
   late NullableTestObjCLibrary lib;
-  NullableInterface? nullableInterface;
-  NSObject? obj;
-  group('method calls', () {
+  late NullableInterface nullableInterface;
+  late NSObject obj;
+  group('Nullability', () {
     setUpAll(() {
       logWarnings();
       final dylib = File('test/native_objc_test/nullable_test.dylib');
@@ -30,12 +30,12 @@ void main() {
 
     group('Nullable property', () {
       test('Not null', () {
-        nullableInterface!.nullableObjectProperty = obj!;
-        expect(nullableInterface!.nullableObjectProperty, obj!);
+        nullableInterface.nullableObjectProperty = obj;
+        expect(nullableInterface.nullableObjectProperty, obj);
       });
       test('Null', () {
-        nullableInterface!.nullableObjectProperty = null;
-        expect(nullableInterface!.nullableObjectProperty, null);
+        nullableInterface.nullableObjectProperty = null;
+        expect(nullableInterface.nullableObjectProperty, null);
       });
     });
 
@@ -51,7 +51,7 @@ void main() {
     group('Nullable arguments', () {
       test('Not null', () {
         expect(
-            NullableInterface.isNullWithNullableNSObjectArg_(lib, obj!), false);
+            NullableInterface.isNullWithNullableNSObjectArg_(lib, obj), false);
       });
       test('Null', () {
         expect(
@@ -62,7 +62,7 @@ void main() {
     group('Not-nullable arguments', () {
       test('Not null', () {
         expect(
-            NullableInterface.isNullWithNotNullableNSObjectPtrArg_(lib, obj!),
+            NullableInterface.isNullWithNotNullableNSObjectPtrArg_(lib, obj),
             false);
       });
     });