Handle ObjC nullable annotations (#624)

* Fix #334

* Fix analysis

* fix swift test

* Fix tests

* Fix nullability bug

* Fix formatting

* Fix linker error

* Fix static methods bug

* fix objective_c_example_test

lib/src/code_generator.dart

@@ -19,6 +19,7 @@ export 'code_generator/native_type.dart';
 export 'code_generator/objc_block.dart';
 export 'code_generator/objc_built_in_functions.dart';
 export 'code_generator/objc_interface.dart';
+export 'code_generator/objc_nullable.dart';
 export 'code_generator/pointer.dart';
 export 'code_generator/struct.dart';
 export 'code_generator/type.dart';

lib/src/code_generator/objc_built_in_functions.dart

@@ -326,7 +326,7 @@ class $name implements ${w.ffiLibraryPrefix}.Finalizable {
   String toString() {
     final data = dataUsingEncoding_(
         0x94000100 /* NSUTF16LittleEndianStringEncoding */);
-    return data.bytes.cast<${w.ffiPkgLibraryPrefix}.Utf16>().toDartString(
+    return data!.bytes.cast<${w.ffiPkgLibraryPrefix}.Utf16>().toDartString(
         length: length);
   }
 ''');

lib/src/code_generator/objc_interface.dart

@@ -76,10 +76,8 @@ class ObjCInterface extends BindingType {
       if (isStatic) {
         stringParams.add('${w.className} _lib');
       }
-      stringParams.addAll(params.map((p) =>
-          (_getConvertedType(p.type, w, name) +
-              (p.isNullable ? "? " : " ") +
-              p.name)));
+      stringParams.addAll(
+          params.map((p) => '${_getConvertedType(p.type, w, name)} ${p.name}'));
       return '(${stringParams.join(", ")})';
     }
 
@@ -146,8 +144,7 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
       s.write('  ');
       if (isStatic) {
         s.write('static ');
-        s.write(
-            _getConvertedReturnType(returnType, w, name, m.isNullableReturn));
+        s.write(_getConvertedType(returnType, w, name));
 
         switch (m.kind) {
           case ObjCMethodKind.method:
@@ -173,14 +170,12 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
         switch (m.kind) {
           case ObjCMethodKind.method:
             // returnType methodName(...)
-            s.write(_getConvertedReturnType(
-                returnType, w, name, m.isNullableReturn));
+            s.write(_getConvertedType(returnType, w, name));
             s.write(' $methodName');
             s.write(paramsToString(params, isStatic: false));
             break;
           case ObjCMethodKind.propertyGetter:
-            s.write(_getConvertedReturnType(
-                returnType, w, name, m.isNullableReturn));
+            s.write(_getConvertedType(returnType, w, name));
             if (isStret) {
               // void getMethodName(Pointer<returnType> stret, NativeLibrary _lib)
               s.write(' get');
@@ -219,8 +214,8 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
       }
       s.write(');\n');
       if (convertReturn) {
-        final result = _doReturnConversion(returnType, '_ret', name, '_lib',
-            m.isNullableReturn, m.isOwnedReturn);
+        final result = _doReturnConversion(
+            returnType, '_ret', name, '_lib', m.isOwnedReturn);
         s.write('    return $result;');
       }
 
@@ -263,6 +258,7 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
     if (superType != null) {
       superType!.addDependencies(dependencies);
       _copyMethodsFromSuperType();
+      _fixNullabilityOfOverriddenMethods();
     }
 
     for (final m in methods.values) {
@@ -280,12 +276,48 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
       if (m.isClass &&
           !_excludedNSObjectClassMethods.contains(m.originalName)) {
         addMethod(m);
-      } else if (m.returnType is ObjCInstanceType) {
+      } else if (_isInstanceType(m.returnType)) {
         addMethod(m);
       }
     }
   }
 
+  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 && !superMethod.isClass && !method.isClass) {
+          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);
+
   void addMethod(ObjCMethod method) {
     final oldMethod = methods[method.originalName];
     if (oldMethod != null) {
@@ -365,39 +397,36 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
       type is ObjCInterface ||
       type is ObjCBlock ||
       type is ObjCObjectPointer ||
-      type is ObjCInstanceType;
+      type is ObjCInstanceType ||
+      type is ObjCNullable;
 
   String _getConvertedType(Type type, Writer w, String enclosingClass) {
     if (type is ObjCInstanceType) return enclosingClass;
-    return type.getDartType(w);
-  }
-
-  String _getConvertedReturnType(
-      Type type, Writer w, String enclosingClass, bool isNullableReturn) {
-    final result = _getConvertedType(type, w, enclosingClass);
-    if (isNullableReturn) {
-      return "$result?";
+    if (type is ObjCNullable && type.child is ObjCInstanceType) {
+      return '$enclosingClass?';
     }
-    return result;
+    return type.getDartType(w);
   }
 
   String _doArgConversion(ObjCMethodParam arg) {
-    if (arg.type is ObjCInterface ||
+    if (arg.type is ObjCNullable) {
+      return '${arg.name}?._id ?? ffi.nullptr';
+    } else if (arg.type is ObjCInterface ||
         arg.type is ObjCObjectPointer ||
         arg.type is ObjCInstanceType ||
         arg.type is ObjCBlock) {
-      if (arg.isNullable) {
-        return '${arg.name}?._id ?? ffi.nullptr';
-      } else {
-        return '${arg.name}._id';
-      }
+      return '${arg.name}._id';
     }
     return arg.name;
   }
 
   String _doReturnConversion(Type type, String value, String enclosingClass,
-      String library, bool isNullable, bool isOwnedReturn) {
-    final prefix = isNullable ? '$value.address == 0 ? null : ' : '';
+      String library, bool isOwnedReturn) {
+    var prefix = '';
+    if (type is ObjCNullable) {
+      prefix = '$value.address == 0 ? null : ';
+      type = type.child;
+    }
     final ownerFlags = 'retain: ${!isOwnedReturn}, release: true';
     if (type is ObjCInterface) {
       return '$prefix${type.name}._($value, $library, $ownerFlags)';
@@ -432,8 +461,7 @@ class ObjCMethod {
   final String? dartDoc;
   final String originalName;
   final ObjCProperty? property;
-  final Type returnType;
-  final bool isNullableReturn;
+  Type returnType;
   final List<ObjCMethodParam> params;
   final ObjCMethodKind kind;
   final bool isClass;
@@ -448,7 +476,6 @@ class ObjCMethod {
     required this.kind,
     required this.isClass,
     required this.returnType,
-    this.isNullableReturn = false,
     List<ObjCMethodParam>? params_,
   }) : params = params_ ?? [];
 
@@ -488,7 +515,6 @@ class ObjCMethod {
 
   bool sameAs(ObjCMethod other) {
     if (originalName != other.originalName) return false;
-    if (isNullableReturn != other.isNullableReturn) return false;
     if (kind != other.kind) return false;
     if (isClass != other.isClass) return false;
     // msgSend is deduped by signature, so this check covers the signature.
@@ -501,11 +527,16 @@ class ObjCMethod {
       originalName.startsWith('new') ||
       originalName.startsWith('alloc') ||
       originalName.contains(_copyRegExp);
+
+  @override
+  String toString() => '$returnType $originalName(${params.join(', ')})';
 }
 
 class ObjCMethodParam {
-  final Type type;
+  Type type;
   final String name;
-  final bool isNullable;
-  ObjCMethodParam(this.type, this.name, {this.isNullable = false});
+  ObjCMethodParam(this.type, this.name);
+
+  @override
+  String toString() => '$type $name';
 }

lib/src/code_generator/objc_nullable.dart

@@ -0,0 +1,46 @@
+// Copyright (c) 2023, 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.
+
+import 'package:ffigen/src/code_generator.dart';
+
+import 'writer.dart';
+
+/// An ObjC type annotated with nullable. Eg:
+/// +(nullable NSObject*) methodWithNullableResult;
+class ObjCNullable extends Type {
+  Type child;
+
+  ObjCNullable(this.child) {
+    assert(isSupported(child));
+  }
+
+  static bool isSupported(Type type) =>
+      type is ObjCInterface ||
+      type is ObjCBlock ||
+      type is ObjCObjectPointer ||
+      type is ObjCInstanceType;
+
+  @override
+  void addDependencies(Set<Binding> dependencies) {
+    child.addDependencies(dependencies);
+  }
+
+  @override
+  Type get baseType => child.baseType;
+
+  @override
+  String getCType(Writer w) => child.getCType(w);
+
+  @override
+  String getFfiDartType(Writer w) => child.getFfiDartType(w);
+
+  @override
+  String getDartType(Writer w) => '${child.getDartType(w)}?';
+
+  @override
+  String toString() => '$child?';
+
+  @override
+  String cacheKey() => '${child.cacheKey()}?';
+}

lib/src/header_parser/clang_bindings/clang_bindings.dart

@@ -915,6 +915,23 @@ class Clang {
   late final _clang_Type_getAlignOf =
       _clang_Type_getAlignOfPtr.asFunction<int Function(CXType)>();
 
+  /// Return the type that was modified by this attributed type.
+  ///
+  /// If the type is not an attributed type, an invalid type is returned.
+  CXType clang_Type_getModifiedType(
+    CXType T,
+  ) {
+    return _clang_Type_getModifiedType(
+      T,
+    );
+  }
+
+  late final _clang_Type_getModifiedTypePtr =
+      _lookup<ffi.NativeFunction<CXType Function(CXType)>>(
+          'clang_Type_getModifiedType');
+  late final _clang_Type_getModifiedType =
+      _clang_Type_getModifiedTypePtr.asFunction<CXType Function(CXType)>();
+
   /// Determine whether the given cursor represents an anonymous
   /// tag or namespace
   int clang_Cursor_isAnonymous(
@@ -2623,10 +2640,10 @@ abstract class CXChildVisitResult {
 ///
 /// The visitor should return one of the \c CXChildVisitResult values
 /// to direct clang_visitCursorChildren().
-typedef CXCursorVisitor = ffi.Pointer<
-    ffi.NativeFunction<
-        ffi.Int32 Function(
-            CXCursor cursor, CXCursor parent, CXClientData client_data)>>;
+typedef CXCursorVisitor
+    = ffi.Pointer<ffi.NativeFunction<CXCursorVisitor_function>>;
+typedef CXCursorVisitor_function = ffi.Int32 Function(
+    CXCursor cursor, CXCursor parent, CXClientData client_data);
 
 /// Opaque pointer representing client data that will be passed through
 /// to various callbacks and visitors.

lib/src/header_parser/parser.dart

@@ -97,7 +97,9 @@ List<Binding> parseToBindings() {
       0,
       clang_types.CXTranslationUnit_Flags.CXTranslationUnit_SkipFunctionBodies |
           clang_types.CXTranslationUnit_Flags
-              .CXTranslationUnit_DetailedPreprocessingRecord,
+              .CXTranslationUnit_DetailedPreprocessingRecord |
+          clang_types
+              .CXTranslationUnit_Flags.CXTranslationUnit_IncludeAttributedTypes,
     );
 
     if (tu == nullptr) {

lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart

@@ -175,9 +175,6 @@ void _parseProperty(clang_types.CXCursor cursor) {
   final isReadOnly = propertyAttributes &
           clang_types.CXObjCPropertyAttrKind.CXObjCPropertyAttr_readonly >
       0;
-  // TODO(#334): Use the nullable attribute to decide this.
-  final isNullable =
-      cursor.type().kind == clang_types.CXTypeKind.CXType_ObjCObjectPointer;
 
   final property = ObjCProperty(fieldName);
 
@@ -193,7 +190,6 @@ void _parseProperty(clang_types.CXCursor cursor) {
     kind: ObjCMethodKind.propertyGetter,
     isClass: isClass,
     returnType: fieldType,
-    isNullableReturn: isNullable,
   );
   itf.addMethod(getter);
 
@@ -208,8 +204,7 @@ void _parseProperty(clang_types.CXCursor cursor) {
         kind: ObjCMethodKind.propertySetter,
         isClass: isClass,
         returnType: NativeType(SupportedNativeType.Void));
-    setter.params
-        .add(ObjCMethodParam(fieldType, 'value', isNullable: isNullable));
+    setter.params.add(ObjCMethodParam(fieldType, 'value'));
     itf.addMethod(setter);
   }
 }
@@ -264,22 +259,7 @@ int _parseMethodVisitor(clang_types.CXCursor cursor,
 }
 
 void _parseMethodParam(clang_types.CXCursor cursor) {
-  /*
-  TODO(#334): Change this to use:
-  
-  clang.clang_Type_getNullability(cursor.type()) ==
-      clang_types.CXTypeNullabilityKind.CXTypeNullability_Nullable;
-
-  NOTE: This will only work with the
-
-    clang_types
-      .CXTranslationUnit_Flags.CXTranslationUnit_IncludeAttributedTypes
-
-  option set.
-  */
   final parsed = _methodStack.top;
-  final isNullable =
-      cursor.type().kind == clang_types.CXTypeKind.CXType_ObjCObjectPointer;
   final name = cursor.spelling();
   final type = cursor.type().toCodeGenType();
   if (type.isIncompleteCompound) {
@@ -291,7 +271,7 @@ void _parseMethodParam(clang_types.CXCursor cursor) {
   }
   _logger.fine(
       '           >> Parameter: $type $name ${cursor.completeStringRepr()}');
-  parsed.method.params.add(ObjCMethodParam(type, name, isNullable: isNullable));
+  parsed.method.params.add(ObjCMethodParam(type, name));
 }
 
 void _markMethodReturnsRetained(clang_types.CXCursor cursor) {