diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index 97f33fb107..e16b02ed4a 100644 --- a/pkgs/ffigen/CHANGELOG.md +++ b/pkgs/ffigen/CHANGELOG.md @@ -4,6 +4,12 @@ - ObjC objects now include the methods from the protocols they implement. Both required and optional methods are included. Optional methods will throw an exception if the method isn't implemented. +- __Breaking change__: Only generate ObjC protocol implementation bindings for + protocols that are included by the config filters. This is breaking because + previously super protocols would automatically get implementation bindings, + rather than just being incorporated into the child protocol. If you want those + implementation bindings, you may need to add the super protocol to your + `objc-protocols` filters. ## 14.0.1 diff --git a/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart b/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart index e8d0ed8d2f..06f6ac86bb 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_protocol.dart @@ -12,6 +12,7 @@ class ObjCProtocol extends NoLookUpBinding with ObjCMethods { final superProtocols = []; final String lookupName; late final ObjCInternalGlobal _protocolPointer; + final bool generateBindings; @override final ObjCBuiltInFunctions builtInFunctions; @@ -23,11 +24,17 @@ class ObjCProtocol extends NoLookUpBinding with ObjCMethods { String? lookupName, super.dartDoc, required this.builtInFunctions, + required this.generateBindings, }) : lookupName = lookupName ?? originalName, super(name: name ?? originalName); @override BindingString toBindingString(Writer w) { + if (!generateBindings) { + return const BindingString( + type: BindingStringType.objcProtocol, string: ''); + } + final protocolMethod = ObjCBuiltInFunctions.protocolMethod.gen(w); final protocolListenableMethod = ObjCBuiltInFunctions.protocolListenableMethod.gen(w); @@ -154,11 +161,13 @@ ${makeDartDoc(dartDoc ?? originalName)}abstract final class $name { if (dependencies.contains(this)) return; dependencies.add(this); - _protocolPointer = ObjCInternalGlobal( - '_protocol_$originalName', - (Writer w) => - '${ObjCBuiltInFunctions.getProtocol.gen(w)}("$lookupName")') - ..addDependencies(dependencies); + if (generateBindings) { + _protocolPointer = ObjCInternalGlobal( + '_protocol_$originalName', + (Writer w) => + '${ObjCBuiltInFunctions.getProtocol.gen(w)}("$lookupName")') + ..addDependencies(dependencies); + } for (final superProtocol in superProtocols) { superProtocol.addDependencies(dependencies); diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart index f78e021fbb..83658bebcc 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart @@ -118,7 +118,7 @@ void _parseSuperType(clang_types.CXCursor cursor, ObjCInterface itf) { void _parseProtocol(clang_types.CXCursor cursor, ObjCInterface itf) { final protoCursor = clang.clang_getCursorDefinition(cursor); - final proto = parseObjCProtocolDeclaration(protoCursor); + final proto = parseObjCProtocolDeclaration(protoCursor, ignoreFilter: true); if (proto != null) { itf.addProtocol(proto); } diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart index a1ff1107d0..3827e53ac2 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart @@ -22,18 +22,19 @@ ObjCProtocol? parseObjCProtocolDeclaration(clang_types.CXCursor cursor, } final usr = cursor.usr(); - final cachedProtocol = bindingsIndex.getSeenObjCProtocol(usr); - if (cachedProtocol != null) { - return cachedProtocol; - } - final name = cursor.spelling(); final decl = Declaration(usr: usr, originalName: name); - if (!ignoreFilter && !shouldIncludeObjCProtocol(decl)) { + final included = shouldIncludeObjCProtocol(decl); + if (!ignoreFilter && !included) { return null; } + final cachedProtocol = bindingsIndex.getSeenObjCProtocol(usr); + if (cachedProtocol != null) { + return cachedProtocol; + } + if (!isApiAvailable(cursor)) { _logger.info('Omitting deprecated protocol $name'); return null; @@ -49,6 +50,12 @@ ObjCProtocol? parseObjCProtocolDeclaration(clang_types.CXCursor cursor, lookupName: applyModulePrefix(name, config.protocolModule(decl)), dartDoc: getCursorDocComment(cursor), builtInFunctions: objCBuiltInFunctions, + + // Only generate bindings for the protocol if it is included in the user's + // filters. If this protocol was only parsed because of ignoreFilter, then + // it's being used to add methods to an interface or a child protocol, and + // shouldn't get bindings. + generateBindings: included, ); // Make sure to add the protocol to the index before parsing the AST, to break diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.dart b/pkgs/ffigen/test/native_objc_test/protocol_test.dart index 978f87e4e4..ac62d65b57 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.dart +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.dart @@ -70,6 +70,9 @@ void main() { // Required instance method from secondary protocol. final otherIntResult = protocolImpl.otherMethod_b_c_d_(2, 4, 6, 8); expect(otherIntResult, 20); + + // Method from a protocol that isn't included by the filters. + expect(protocolImpl.fooMethod(), 2468); }); test('Unimplemented method', () { @@ -373,5 +376,23 @@ void main() { expect(blockRetainCount(blockPtr), 0); }, skip: !canDoGC); }); + + test('Filters', () { + // SuperProtocol and FilteredProtocol's methods are included in the + // bindings, but there shouldn't actually be bindings for the protocols + // themselves, because they're not included by the config. + final bindings = File('test/native_objc_test/protocol_bindings.dart') + .readAsStringSync(); + + expect(bindings, contains('instanceMethod_withDouble_')); + expect(bindings, contains('fooMethod')); + + expect(bindings, contains('EmptyProtocol')); + expect(bindings, contains('MyProtocol')); + expect(bindings, contains('SecondaryProtocol')); + + expect(bindings, isNot(contains('SuperProtocol'))); + expect(bindings, isNot(contains('FilteredProtocol'))); + }); }); } diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.h b/pkgs/ffigen/test/native_objc_test/protocol_test.h index fa29239c81..0405b82f9d 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.h +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.h @@ -43,6 +43,11 @@ typedef struct { @protocol EmptyProtocol @end +@protocol FilteredProtocol +@required +- (int32_t)fooMethod; +@end + @interface ProtocolConsumer : NSObject - (NSString*)callInstanceMethod:(id)protocol; @@ -52,7 +57,8 @@ typedef struct { @end -@interface ObjCProtocolImpl : NSObject +@interface ObjCProtocolImpl : + NSObject @end diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.m b/pkgs/ffigen/test/native_objc_test/protocol_test.m index 5b80e236dc..c6cc50a5b7 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.m +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.m @@ -46,6 +46,10 @@ - (int32_t)otherMethod:(int32_t)a b:(int32_t)b c:(int32_t)c d:(int32_t)d { return a + b + c + d; } +- (int32_t)fooMethod { + return 2468; +} + @end