diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index 963097449a..2db1169bb2 100644 --- 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 index 980b9cedad..75cf813b1b 100644 --- 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 index 7d3268f80e..711162f64b 100644 --- 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 index 882e68c3f0..8a7601b2ed 100644 --- a/pkgs/ffigen/lib/src/header_parser/parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/parser.dart @@ -175,8 +175,9 @@ List _transformBindings(Config config, List 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)) @@ -187,9 +188,11 @@ List _transformBindings(Config config, List 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 index 6903bf2eda..56b1f60403 100644 --- 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 = {}; - final _superTypes = {}; + final directlyIncluded = {}; + final indirectlyIncluded = {}; ApplyConfigFiltersVisitation(this.config); - Set 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 index e9bc98e5d5..d28bb37ee6 100644 --- 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 includes; + final Set directIncludes; final directTransitives = {}; - 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 index c278f68917..8eb0fb0efc 100644 --- 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 directTransitives; final bindings = {}; - ListBindingsVisitation( - this.config, this.includes, this.transitives, this.directTransitives); + ListBindingsVisitation(this.config, this.includes, + Set 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 index 2263a9d747..467b082a6d 100644 --- 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 index dbde5a95bb..4d296b4b74 100644 --- 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.toString(), '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 index 000694f9ed..82df2fde29 100644 --- 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 #import +#import @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 index 290ddcfe76..3a777043e6 100644 --- 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 index 03ecc2628a..8c6de011ce 100644 --- 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 index 5959743862..e73dcbfd69 100644 --- 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 yamlStructs; late List yamlEnums; late List yamlProtocols; + late List yamlCategories; setUpAll(() { final yaml = @@ -47,6 +48,11 @@ void main() { .map((dynamic i) => i as String) .toList() ..sort(); + yamlCategories = ((yaml['objc-categories'] as YamlMap)['include'] + as YamlList) + .map((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 = []; @@ -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 = []; + 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)); + }); }); }