[ffigen] Fix category filtering bug

pkgs/ffigen/CHANGELOG.md

@@ -12,6 +12,8 @@
   - Remove `useSupportedTypedefs`, treating it as always true
 - Fix [a bug](https://github.com/dart-lang/native/issues/3504) in handling of
   small structs in ObjC on mac/iOS x64.
+- Fix [a bug](https://github.com/dart-lang/native/issues/3546) in the way that
+  ObjC category methods returning `instancetype` are filtered.
 - Minor Objective-C code generator and function type signature fixes.
 - Bump `package:code_assets` dependency to `^2.0.0`.
 

pkgs/ffigen/example/objective_c/avf_audio_bindings.dart

@@ -10,7 +10,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 
 /// WARNING: AVAudioFormat is a stub. To generate bindings for this class, include
 /// AVAudioFormat in your config's objc-interfaces list.

pkgs/ffigen/example/swift/swift_api_bindings.dart

@@ -10,7 +10,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 
 /// SwiftClass
 extension type SwiftClass._(objc.ObjCObject object$)

pkgs/ffigen/lib/src/code_generator/imports.dart

@@ -129,7 +129,7 @@ const objcPkgImport = LibraryImport(
   importPathWhenImportedByPackageObjC: '../objective_c.dart',
 );
 const objcMajorVersion = 9;
-const objcMinorVersion = 5;
+const objcMinorVersion = 6;
 const selfImport = LibraryImport('self', '');
 final builtInLibraries = {
   for (final l in [

pkgs/ffigen/lib/src/code_generator/objc_built_in_types.dart

@@ -124,8 +124,16 @@ const objCBuiltInProtocols = {
 };
 
 const objCBuiltInCategories = {
+  'NSArrayCreation',
+  'NSAttributedStringCreateFromMarkdown',
+  'NSAttributedStringFormatting',
+  'NSDataBase64Encoding',
+  'NSDataCompression',
   'NSDataCreation',
+  'NSDateCreation',
+  'NSDictionaryCreation',
   'NSExtendedArray',
+  'NSExtendedAttributedString',
   'NSExtendedData',
   'NSExtendedDate',
   'NSExtendedDictionary',
@@ -137,8 +145,19 @@ const objCBuiltInCategories = {
   'NSExtendedMutableSet',
   'NSExtendedOrderedSet',
   'NSExtendedSet',
+  'NSInputStreamExtensions',
+  'NSLocaleCreation',
+  'NSMutableArrayCreation',
+  'NSMutableDataCreation',
+  'NSMutableDictionaryCreation',
+  'NSMutableOrderedSetCreation',
+  'NSMutableSetCreation',
+  'NSNotificationCreation',
   'NSNumberCreation',
-  'NSNumberIsFloat',
   'NSNumberIsBool',
+  'NSNumberIsFloat',
+  'NSOrderedSetCreation',
+  'NSOutputStreamExtensions',
+  'NSSetCreation',
   'NSStringExtensionMethods',
 };

pkgs/ffigen/lib/src/code_generator/objc_methods.dart

@@ -13,6 +13,7 @@ import 'local_variables.dart';
 import 'native_type.dart';
 import 'objc_block.dart';
 import 'objc_built_in_functions.dart';
+import 'objc_category.dart';
 import 'objc_interface.dart';
 import 'objc_nullable.dart';
 import 'pointer.dart';
@@ -49,12 +50,12 @@ mixin ObjCMethods {
     }
   }
 
-  void copyMethod(ObjCMethod method) {
+  void copyMethod(ObjCMethod method, {ObjCCategory? originCategory}) {
     // To maintain the pairing between getters and setters after cloning,
     // instead of directly cloning the setter, we clone the setter when we clone
     // the getter. This lets us, for example, share the symbol between them.
     if (method.kind == ObjCMethodKind.propertySetter) return;
-    final cloned = method.clone();
+    final cloned = method.clone(originCategory: originCategory);
     addMethod(cloned);
     addMethod(cloned.setter);
   }
@@ -213,6 +214,7 @@ class ObjCMethod extends AstNode with HasLocalScope {
   ObjCMethods? parent;
   ObjCMethod? setter;
   bool isIncluded = true;
+  ObjCCategory? originCategory;
 
   @override
   void visitChildren(Visitor visitor, {bool omitMethodName = false}) {
@@ -332,7 +334,11 @@ class ObjCMethod extends AstNode with HasLocalScope {
   bool get isInstanceMethod => !isClassMethod;
   bool get unavailable => apiAvailability.availability == Availability.none;
 
-  ObjCMethod _cloneWithSymbol(Symbol newSymbol, {ObjCMethods? parent}) {
+  ObjCMethod _cloneWithSymbol(
+    Symbol newSymbol, {
+    ObjCMethods? parent,
+    ObjCCategory? originCategory,
+  }) {
     final clonedMethod = ObjCMethod.withSymbol(
       context: context,
       originalName: originalName,
@@ -352,19 +358,25 @@ class ObjCMethod extends AstNode with HasLocalScope {
     clonedMethod.parent = parent;
     clonedMethod.protocolMethodName = protocolMethodName?.clone();
     clonedMethod.isIncluded = isIncluded;
+    clonedMethod.originCategory = originCategory ?? this.originCategory;
     return clonedMethod;
   }
 
-  ObjCMethod clone({ObjCMethods? parent}) {
+  ObjCMethod clone({ObjCMethods? parent, ObjCCategory? originCategory}) {
     assert(kind != ObjCMethodKind.propertySetter);
     final clonedSymbol = symbol.clone();
-    final clonedMethod = _cloneWithSymbol(clonedSymbol, parent: parent);
+    final clonedMethod = _cloneWithSymbol(
+      clonedSymbol,
+      parent: parent,
+      originCategory: originCategory,
+    );
     if (setter != null) {
       assert(setter!.kind == ObjCMethodKind.propertySetter);
       assert(setter!.symbol == symbol);
       final clonedSetter = setter!._cloneWithSymbol(
         clonedSymbol,
         parent: parent,
+        originCategory: originCategory,
       );
       clonedSetter.isIncluded = clonedMethod.isIncluded;
       clonedMethod.setter = clonedSetter;

pkgs/ffigen/lib/src/visitor/apply_config_filters.dart

@@ -51,7 +51,15 @@ class ApplyConfigFiltersVisitation extends Visitation {
     if (context.config.objectiveC == null) return;
 
     if (!node.isInternal) {
-      node.filterMethods((m) => !m.unavailable && m.isIncluded);
+      node.filterMethods((m) {
+        if (m.unavailable) return false;
+        if (m.originCategory != null &&
+            m.originCategory!.originalName.isNotEmpty &&
+            !m.originCategory!.isIncluded) {
+          return false;
+        }
+        return m.isIncluded;
+      });
     }
     _visitImpl(node, node.isIncluded);
 

pkgs/ffigen/lib/src/visitor/copy_methods_from_super_type.dart

@@ -70,15 +70,10 @@ class CopyMethodsFromSuperTypesVisitation extends Visitation {
     // methods return instancetype, because the Dart inheritance rules don't
     // match the ObjC rules regarding instancetype.
     // Also copy all methods from any anonymous categories.
-    // NOTE: The methods are copied regardless of whether the category is
-    // included by the config filters, since this method copying visit happens
-    // before the filtering visit. This is technically a bug, but it's unlikely
-    // to bother anyone, and the fix would be complicated. So we'll ignore it
-    // for now.
     for (final category in node.categories) {
       for (final m in category.methods) {
         if (category.shouldCopyMethodToInterface(m)) {
-          node.copyMethod(m);
+          node.copyMethod(m, originCategory: category);
         }
       }
     }

pkgs/ffigen/pubspec.yaml

@@ -43,7 +43,8 @@ dev_dependencies:
   dart_flutter_team_lints: ^3.5.2
   json_schema: ^5.1.1
   leak_tracker: ^11.0.2
-  objective_c: ^9.5.0
+  objective_c:
+    path: ../objective_c
   test: ^1.26.2
 
 dependency_overrides:

pkgs/ffigen/test/native_objc_test/arc_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 @ffi.Native<ffi.Void Function(ffi.Pointer<ffi.Void>)>()
 external void objc_autoreleasePoolPop(ffi.Pointer<ffi.Void> pool);
 

pkgs/ffigen/test/native_objc_test/bad_method_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 
 /// BadMethodTestObject
 extension type BadMethodTestObject._(objc.ObjCObject object$)

pkgs/ffigen/test/native_objc_test/bad_override_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 
 /// BadOverrideAunt
 extension type BadOverrideAunt._(objc.ObjCObject object$)

pkgs/ffigen/test/native_objc_test/block_annotation_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 @ffi.Native<
   ffi.Void Function(
     ffi.Pointer<objc.ObjCObjectImpl>,

pkgs/ffigen/test/native_objc_test/block_inherit_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 typedef AcceptMammal = ffi.Pointer<objc.ObjCBlockImpl>;
 typedef DartAcceptMammal = objc.ObjCBlock<ffi.Bool Function(Mammal)>;
 typedef AcceptPlatypus = ffi.Pointer<objc.ObjCBlockImpl>;

pkgs/ffigen/test/native_objc_test/block_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 @ffi.Native<
   ffi.Pointer<objc.ObjCBlockImpl> Function(
     ffi.Int64,

pkgs/ffigen/test/native_objc_test/cast_test_bindings.dart

@@ -9,7 +9,7 @@ import 'dart:ffi' as ffi;
 import 'package:objective_c/objective_c.dart' as objc;
 import 'package:ffi/ffi.dart' as pkg_ffi;
 
-const _$objcVersionCheck = objc.ObjCVersionCheck(9, 5);
+const _$objcVersionCheck = objc.ObjCVersionCheck(9, 6);
 
 /// Castaway
 extension type Castaway._(objc.ObjCObject object$)

pkgs/ffigen/test/native_objc_test/category_config.yaml

@@ -20,6 +20,8 @@ objc-categories:
     - InterfaceOnBuiltInType
     - StaticAndInstanceMethodsWithSameNameCategory
     - NSString
+    - NSURLCategory
+include-transitive-objc-categories: false
 ffi-native:
   asset-id: 'package:ffigen/objc_test'
 headers:

pkgs/ffigen/test/native_objc_test/category_test.dart

@@ -111,5 +111,32 @@ extension type ChildOfNSString._(objc.ObjCObject object\$)
 '''),
       );
     });
+
+    test('Excluded category filtering and instancetype methods', () {
+      final bindings = File(
+        path.join(
+          packagePathForTests,
+          'test',
+          'native_objc_test',
+          'category_test_bindings.dart',
+        ),
+      ).readAsStringSync();
+
+      // ExcludedCategory extension and its methods should not be generated.
+      expect(bindings, isNot(contains('extension ExcludedCategory')));
+      expect(
+        bindings,
+        isNot(contains('excludedCategoryNonInstancetypeMethod')),
+      );
+      expect(
+        bindings,
+        isNot(contains('excludedCategoryStaticNonInstancetypeMethod')),
+      );
+      expect(bindings, isNot(contains('excludedCategoryInstancetypeMethod')));
+      expect(
+        bindings,
+        isNot(contains('excludedCategoryStaticInstancetypeMethod')),
+      );
+    });
   });
 }

pkgs/ffigen/test/native_objc_test/category_test.h

@@ -33,6 +33,13 @@
 -(instancetype)instancetypeMethod;
 @end
 
+@interface Thing (ExcludedCategory)
+-(instancetype)excludedCategoryInstancetypeMethod;
++(instancetype)excludedCategoryStaticInstancetypeMethod;
+-(int32_t)excludedCategoryNonInstancetypeMethod;
++(int32_t)excludedCategoryStaticNonInstancetypeMethod;
+@end
+
 @interface Thing ()
 -(int32_t)anonymousCategoryMethod;
 +(int32_t)anonymousCategoryStaticMethod;

pkgs/ffigen/test/native_objc_test/category_test.m

@@ -54,6 +54,21 @@ -(instancetype)instancetypeMethod {
 }
 @end
 
+@implementation Thing (ExcludedCategory)
+-(instancetype)excludedCategoryInstancetypeMethod {
+  return [[self class] new];
+}
++(instancetype)excludedCategoryStaticInstancetypeMethod {
+  return [[self class] new];
+}
+-(int32_t)excludedCategoryNonInstancetypeMethod {
+  return 111;
+}
++(int32_t)excludedCategoryStaticNonInstancetypeMethod {
+  return 222;
+}
+@end
+
 @implementation ChildOfThing
 @end