Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
20 changes: 20 additions & 0 deletions pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkgs/ffigen/lib/src/code_generator/objc_category.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
13 changes: 8 additions & 5 deletions pkgs/ffigen/lib/src/header_parser/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,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))
Expand All @@ -187,9 +188,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(
Expand Down
12 changes: 5 additions & 7 deletions pkgs/ffigen/lib/src/visitor/apply_config_filters.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion pkgs/ffigen/lib/src/visitor/find_transitive_deps.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkgs/ffigen/lib/src/visitor/list_bindings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions pkgs/ffigen/test/native_objc_test/category_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ objc-interfaces:
include:
- Thing
- ChildOfThing
- NSURL
objc-categories:
include:
- Sub
Expand Down
15 changes: 15 additions & 0 deletions pkgs/ffigen/test/native_objc_test/category_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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')));
});
});
}
9 changes: 9 additions & 0 deletions pkgs/ffigen/test/native_objc_test/category_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,3 +46,11 @@
+(NSString*)staticMethod;
-(instancetype)instancetypeMethod;
@end

@interface NSURL (NSURLCategory)
-(int32_t)extensionMethod;
@end

@interface NSString (NSStringCategory)
-(int32_t)excludedExtensionMethod;
@end
12 changes: 12 additions & 0 deletions pkgs/ffigen/test/native_objc_test/category_test.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 11 additions & 0 deletions pkgs/objective_c/lib/objective_c.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions pkgs/objective_c/test/interface_lists_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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', () {
Expand All @@ -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) {
Expand Down Expand Up @@ -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>[];
Expand Down Expand Up @@ -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));
});
});
}
Loading