[ffigen] Support transitive categories of built-in types (#1930)

liamappelbe · web-flow · commit cb09254c0030 · 2025-01-24T13:23:43.000+11:00
diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md
@@ -1,6 +1,8 @@
 ## 17.0.0-wip
 
 - Use package:objective_c 5.0.0
+- Support transitive categories of built-in types:
+  https://github.com/dart-lang/native/issues/1820
 
 ## 16.1.0
 
diff --git a/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart b/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart
@@ -128,6 +128,24 @@ class ObjCBuiltInFunctions {
   static const builtInProtocols = {
     'NSStreamDelegate',
   };
+  @visibleForTesting
+  static const builtInCategories = {
+    'NSDataCreation',
+    'NSExtendedArray',
+    'NSExtendedData',
+    'NSExtendedDate',
+    'NSExtendedDictionary',
+    'NSExtendedEnumerator',
+    'NSExtendedMutableArray',
+    'NSExtendedMutableData',
+    'NSExtendedMutableDictionary',
+    'NSExtendedMutableOrderedSet',
+    'NSExtendedMutableSet',
+    'NSExtendedOrderedSet',
+    'NSExtendedSet',
+    'NSNumberCreation',
+    'NSStringExtensionMethods',
+  };
 
   // TODO(https://github.com/dart-lang/native/issues/1173): Ideally this check
   // would be based on more than just the name.
@@ -139,6 +157,8 @@ class ObjCBuiltInFunctions {
       !generateForPackageObjectiveC && builtInEnums.contains(name);
   bool isBuiltInProtocol(String name) =>
       !generateForPackageObjectiveC && builtInProtocols.contains(name);
+  bool isBuiltInCategory(String name) =>
+      !generateForPackageObjectiveC && builtInCategories.contains(name);
   static bool isNSObject(String name) => name == 'NSObject';
 
   // We need to load a separate instance of objc_msgSend for each signature. If
diff --git a/pkgs/ffigen/lib/src/code_generator/objc_category.dart b/pkgs/ffigen/lib/src/code_generator/objc_category.dart
@@ -34,6 +34,9 @@ class ObjCCategory extends NoLookUpBinding with ObjCMethods {
     return method.returnsInstanceType && !parent.isObjCImport;
   }
 
+  @override
+  bool get isObjCImport => builtInFunctions.isBuiltInCategory(originalName);
+
   @override
   final ObjCBuiltInFunctions builtInFunctions;
 
diff --git a/pkgs/ffigen/lib/src/header_parser/parser.dart b/pkgs/ffigen/lib/src/header_parser/parser.dart
@@ -177,8 +177,9 @@ List<Binding> transformBindings(Config config, List<Binding> bindings) {
   visit(FixOverriddenMethodsVisitation(), bindings);
   visit(FillMethodDependenciesVisitation(), bindings);
 
-  final included =
-      visit(ApplyConfigFiltersVisitation(config), bindings).included;
+  final filterResults = visit(ApplyConfigFiltersVisitation(config), bindings);
+  final directlyIncluded = filterResults.directlyIncluded;
+  final included = directlyIncluded.union(filterResults.indirectlyIncluded);
 
   final byValueCompounds = visit(FindByValueCompoundsVisitation(),
           FindByValueCompoundsVisitation.rootNodes(included))
@@ -189,9 +190,11 @@ List<Binding> transformBindings(Config config, List<Binding> bindings) {
 
   final transitives =
       visit(FindTransitiveDepsVisitation(), included).transitives;
-  final directTransitives =
-      visit(FindDirectTransitiveDepsVisitation(config, included), included)
-          .directTransitives;
+  final directTransitives = visit(
+          FindDirectTransitiveDepsVisitation(
+              config, included, directlyIncluded),
+          included)
+      .directTransitives;
 
   final finalBindings = visit(
           ListBindingsVisitation(
diff --git a/pkgs/ffigen/lib/src/visitor/apply_config_filters.dart b/pkgs/ffigen/lib/src/visitor/apply_config_filters.dart
@@ -9,17 +9,15 @@ import 'ast.dart';
 
 class ApplyConfigFiltersVisitation extends Visitation {
   final Config config;
-  final _directlyIncluded = <Binding>{};
-  final _superTypes = <Binding>{};
+  final directlyIncluded = <Binding>{};
+  final indirectlyIncluded = <Binding>{};
   ApplyConfigFiltersVisitation(this.config);
 
-  Set<Binding> get included => _directlyIncluded.union(_superTypes);
-
   void _visitImpl(Binding node, DeclarationFilters filters) {
     node.visitChildren(visitor);
     if (node.originalName == '') return;
     if (config.usrTypeMappings.containsKey(node.usr)) return;
-    if (filters.shouldInclude(node)) _directlyIncluded.add(node);
+    if (filters.shouldInclude(node)) directlyIncluded.add(node);
   }
 
   @override
@@ -45,9 +43,9 @@ class ApplyConfigFiltersVisitation extends Visitation {
     _visitImpl(node, config.objcInterfaces);
 
     // If this node is included, include all its super types.
-    if (_directlyIncluded.contains(node)) {
+    if (directlyIncluded.contains(node)) {
       for (ObjCInterface? t = node; t != null; t = t.superType) {
-        if (!_superTypes.add(t)) break;
+        if (!indirectlyIncluded.add(t)) break;
       }
     }
   }
diff --git a/pkgs/ffigen/lib/src/visitor/find_transitive_deps.dart b/pkgs/ffigen/lib/src/visitor/find_transitive_deps.dart
@@ -21,9 +21,11 @@ class FindTransitiveDepsVisitation extends Visitation {
 class FindDirectTransitiveDepsVisitation extends Visitation {
   final Config config;
   final Set<Binding> includes;
+  final Set<Binding> directIncludes;
   final directTransitives = <Binding>{};
 
-  FindDirectTransitiveDepsVisitation(this.config, this.includes);
+  FindDirectTransitiveDepsVisitation(
+      this.config, this.includes, this.directIncludes);
 
   void _visitImpl(Binding node, bool forceVisitChildren) {
     if (node.isObjCImport) return;
@@ -41,6 +43,12 @@ class FindDirectTransitiveDepsVisitation extends Visitation {
     // included. This ensures that super types of stubs are also stubs, rather
     // than being omitted like the rest of the stub's children.
     visitor.visit(node.superType);
+
+    // Visit the categories of built-in interfaces that have been explicitly
+    // included. https://github.com/dart-lang/native/issues/1820
+    if (node.isObjCImport && directIncludes.contains(node)) {
+      visitor.visitAll(node.categories);
+    }
   }
 
   @override
diff --git a/pkgs/ffigen/lib/src/visitor/list_bindings.dart b/pkgs/ffigen/lib/src/visitor/list_bindings.dart
@@ -23,8 +23,9 @@ class ListBindingsVisitation extends Visitation {
   final Set<Binding> directTransitives;
   final bindings = <Binding>{};
 
-  ListBindingsVisitation(
-      this.config, this.includes, this.transitives, this.directTransitives);
+  ListBindingsVisitation(this.config, this.includes,
+      Set<Binding> indirectTransitives, this.directTransitives)
+      : transitives = {...indirectTransitives, ...directTransitives};
 
   void _add(Binding node) {
     node.visitChildren(visitor);
diff --git a/pkgs/ffigen/test/native_objc_test/category_config.yaml b/pkgs/ffigen/test/native_objc_test/category_config.yaml
@@ -7,6 +7,7 @@ objc-interfaces:
   include:
     - Thing
     - ChildOfThing
+    - NSURL
 objc-categories:
   include:
     - Sub
diff --git a/pkgs/ffigen/test/native_objc_test/category_test.dart b/pkgs/ffigen/test/native_objc_test/category_test.dart
@@ -73,5 +73,20 @@ void main() {
       NSString str2 = str.instancetypeMethod();
       expect(str2.toDartString(), 'Hello');
     });
+
+    test('Transitive category on built-in type', () {
+      // Regression test for https://github.com/dart-lang/native/issues/1820.
+      // Include transitive category of explicitly included buit-in type.
+      expect(NSURL.alloc().extensionMethod(), 555);
+
+      // Don't include transitive category of built-in type that hasn't been
+      // explicitly included.
+      final bindings = File('test/native_objc_test/category_bindings.dart')
+          .readAsStringSync();
+      expect(bindings, isNot(contains('excludedExtensionMethod')));
+
+      // This method is from an NSObject extension, which shouldn't be included.
+      expect(bindings, isNot(contains('autoContentAccessingProxy')));
+    });
   });
 }
diff --git a/pkgs/ffigen/test/native_objc_test/category_test.h b/pkgs/ffigen/test/native_objc_test/category_test.h
@@ -4,6 +4,7 @@
 
 #import <Foundation/NSObject.h>
 #import <Foundation/NSString.h>
+#import <Foundation/NSURL.h>
 
 @interface Thing : NSObject {}
 -(int32_t)add:(int32_t)x Y:(int32_t) y;
@@ -45,3 +46,11 @@
 +(NSString*)staticMethod;
 -(instancetype)instancetypeMethod;
 @end
+
+@interface NSURL (NSURLCategory)
+-(int32_t)extensionMethod;
+@end
+
+@interface NSString (NSStringCategory)
+-(int32_t)excludedExtensionMethod;
+@end
diff --git a/pkgs/ffigen/test/native_objc_test/category_test.m b/pkgs/ffigen/test/native_objc_test/category_test.m
@@ -70,3 +70,15 @@ -(instancetype)instancetypeMethod {
   return [self copy];
 }
 @end
+
+@implementation NSURL (NSURLCategory)
+-(int32_t)extensionMethod {
+  return 555;
+}
+@end
+
+@implementation NSString (NSStringCategory)
+-(int32_t)excludedExtensionMethod {
+  return 999;
+}
+@end
diff --git a/pkgs/objective_c/lib/objective_c.dart b/pkgs/objective_c/lib/objective_c.dart
@@ -45,7 +45,18 @@ export 'src/objective_c_bindings_generated.dart'
         NSEnumerationOptions,
         NSEnumerator,
         NSError,
+        NSExtendedArray,
+        NSExtendedData,
+        NSExtendedDate,
+        NSExtendedDictionary,
+        NSExtendedEnumerator,
+        NSExtendedMutableArray,
         NSExtendedMutableData,
+        NSExtendedMutableDictionary,
+        NSExtendedMutableOrderedSet,
+        NSExtendedMutableSet,
+        NSExtendedOrderedSet,
+        NSExtendedSet,
         NSFastEnumerationState,
         NSIndexSet,
         NSInputStream,
diff --git a/pkgs/objective_c/test/interface_lists_test.dart b/pkgs/objective_c/test/interface_lists_test.dart
@@ -20,6 +20,7 @@ void main() {
     late List<String> yamlStructs;
     late List<String> yamlEnums;
     late List<String> yamlProtocols;
+    late List<String> yamlCategories;
 
     setUpAll(() {
       final yaml =
@@ -47,6 +48,11 @@ void main() {
           .map<String>((dynamic i) => i as String)
           .toList()
         ..sort();
+      yamlCategories = ((yaml['objc-categories'] as YamlMap)['include']
+              as YamlList)
+          .map<String>((dynamic i) => i as String)
+          .toList()
+        ..sort();
     });
 
     test('ObjCBuiltInFunctions.builtInInterfaces', () {
@@ -65,6 +71,10 @@ void main() {
       expect(ObjCBuiltInFunctions.builtInProtocols, yamlProtocols);
     });
 
+    test('ObjCBuiltInFunctions.builtInCategories', () {
+      expect(ObjCBuiltInFunctions.builtInCategories, yamlCategories);
+    });
+
     test('package:objective_c exports all the interfaces', () {
       final exportFile = File('lib/objective_c.dart').readAsStringSync();
       for (final intf in yamlInterfaces) {
@@ -95,6 +105,13 @@ void main() {
       }
     });
 
+    test('package:objective_c exports all the categories', () {
+      final exportFile = File('lib/objective_c.dart').readAsStringSync();
+      for (final category in yamlCategories) {
+        expect(exportFile, contains(RegExp('\\W$category\\W')));
+      }
+    });
+
     test('All code genned interfaces are included in the list', () {
       final classNameRegExp = RegExp(r'^class (\w+) ');
       final allClassNames = <String>[];
@@ -150,5 +167,18 @@ void main() {
       }
       expect(allProtocolNames, unorderedEquals(yamlProtocols));
     });
+
+    test('All code genned categories are included in the list', () {
+      final categoryNameRegExp = RegExp(r'^extension (\w+) on \w+ {');
+      final allCategoryNames = <String>[];
+      for (final line in File('lib/src/objective_c_bindings_generated.dart')
+          .readAsLinesSync()) {
+        final match = categoryNameRegExp.firstMatch(line);
+        if (match != null) {
+          allCategoryNames.add(match[1]!);
+        }
+      }
+      expect(allCategoryNames, unorderedEquals(yamlCategories));
+    });
   });
 }

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,9 @@ class ObjCCategory extends NoLookUpBinding with ObjCMethods {`
`34`	`34`	`return method.returnsInstanceType && !parent.isObjCImport;`
`35`	`35`	`}`
`36`	`36`
	`37`	`+ @override`
	`38`	`+ bool get isObjCImport => builtInFunctions.isBuiltInCategory(originalName);`
	`39`	`+`
`37`	`40`	`@override`
`38`	`41`	`final ObjCBuiltInFunctions builtInFunctions;`
`39`	`42`