diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart index ecbe1d6fc..ee85890a4 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart @@ -19,6 +19,7 @@ class ObjCInterface extends BindingType with ObjCMethods { late final ObjCMsgSendFunc _isKindOfClassMsgSend; final protocols = []; final categories = []; + final subtypes = []; @override final ObjCBuiltInFunctions builtInFunctions; @@ -194,5 +195,9 @@ ${generateAsStub ? '' : _generateMethods(w)} visitor.visitAll(protocols); visitor.visitAll(categories); visitMethods(visitor); + + // Note: Don't visit subtypes here, because they shouldn't affect transitive + // inclusion. Including an interface shouldn't auto-include all its + // subtypes, even as stubs. } } diff --git a/pkgs/ffigen/lib/src/code_generator/objc_methods.dart b/pkgs/ffigen/lib/src/code_generator/objc_methods.dart index ae4d8696f..21fbccc4a 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_methods.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_methods.dart @@ -188,7 +188,7 @@ class ObjCMethod extends AstNode { final ObjCProperty? property; Type returnType; final List params; - final ObjCMethodKind kind; + ObjCMethodKind kind; final bool isClassMethod; final bool isOptional; ObjCMethodOwnership? ownershipAttribute; 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 6e52996cb..db4e70f0f 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 @@ -101,6 +101,7 @@ void _parseSuperType(clang_types.CXCursor cursor, ObjCInterface itf) { '$superType ${cursor.completeStringRepr()}'); if (superType is ObjCInterface) { itf.superType = superType; + superType.subtypes.add(itf); } else { _logger.severe( 'Super type of $itf is $superType, which is not a valid interface.'); diff --git a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart index c3f3d0bd1..a0e929f9d 100644 --- a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart +++ b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart @@ -10,7 +10,11 @@ class FixOverriddenMethodsVisitation extends Visitation { @override void visitObjCInterface(ObjCInterface node) { node.visitChildren(visitor); + _fixNullability(node); + _fixMethodsVsProperties(node); + } + void _fixNullability(ObjCInterface node) { // ObjC ignores nullability when deciding if an override for an inherited // method is valid. But in Dart it's invalid to override a method and change // it's return type from non-null to nullable, or its arg type from nullable @@ -41,4 +45,43 @@ class FixOverriddenMethodsVisitation extends Visitation { } } } + + void _fixMethodsVsProperties(ObjCInterface node) { + // In ObjC, supertypes and subtypes can have a method that's an ordinary + // method in some classes of the heirarchy, and a property in others. This + // isn't allowed in Dart, so we change all such conflicts to properties. + // This change could cause more conflicts in the heirarchy, so first we walk + // up to find the root of the subtree that has this method, then we walk + // down the subtreee to change all conflicting methods to properties. + for (final method in node.methods) { + final (root, rootMethod) = _findRootWithMethod(node, method); + if (method.isProperty == rootMethod.isProperty) continue; + _convertAllSubtreeMethodsToProperties(root, rootMethod); + } + } + + (ObjCInterface, ObjCMethod) _findRootWithMethod( + ObjCInterface node, ObjCMethod method) { + var root = node; + var rootMethod = method; + for (ObjCInterface? t = node; t != null; t = t.superType) { + final tMethod = t.getMethod(method.originalName); + if (tMethod != null) { + root = t; + rootMethod = tMethod; + } + } + return (root, rootMethod); + } + + void _convertAllSubtreeMethodsToProperties( + ObjCInterface node, ObjCMethod rootMethod) { + final method = node.getMethod(rootMethod.originalName); + if (method != null && method.kind == ObjCMethodKind.method) { + method.kind = ObjCMethodKind.propertyGetter; + } + for (final t in node.subtypes) { + _convertAllSubtreeMethodsToProperties(t, rootMethod); + } + } } diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_config.yaml b/pkgs/ffigen/test/native_objc_test/bad_override_config.yaml new file mode 100644 index 000000000..83939c85e --- /dev/null +++ b/pkgs/ffigen/test/native_objc_test/bad_override_config.yaml @@ -0,0 +1,19 @@ +name: BadOverrideTestObjCLibrary +description: 'Tests various overrides that are valid in ObjC but invalid in Dart' +language: objc +output: 'bad_override_bindings.dart' +exclude-all-by-default: true +objc-interfaces: + include: + - BadOverrideGrandparent + - BadOverrideParent + - BadOverrideUncle + - BadOverrideAunt + - BadOverrideChild + - BadOverrideSibbling + - BadOverrideGrandchild +headers: + entry-points: + - 'bad_override_test.m' +preamble: | + // ignore_for_file: camel_case_types, non_constant_identifier_names, unnecessary_non_null_assertion, unused_element, unused_field diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.dart b/pkgs/ffigen/test/native_objc_test/bad_override_test.dart new file mode 100644 index 000000000..04c94f851 --- /dev/null +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.dart @@ -0,0 +1,51 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// Objective C support is only available on mac. +@TestOn('mac-os') + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:ffi/ffi.dart'; +import 'package:test/test.dart'; +import '../test_utils.dart'; +import 'bad_override_bindings.dart'; +import 'util.dart'; + +void main() { + group('bad overrides', () { + setUpAll(() { + // TODO(https://github.com/dart-lang/native/issues/1068): Remove this. + DynamicLibrary.open('../objective_c/test/objective_c.dylib'); + final dylib = File('test/native_objc_test/objc_test.dylib'); + verifySetupFile(dylib); + DynamicLibrary.open(dylib.absolute.path); + generateBindingsForCoverage('bad_override'); + }); + + test('Method vs getter', () { + // In ObjC, supertypes and subtypes can have a method that's an ordinary + // method in some classes of the heirarchy, and a property in others. This + // isn't allowed in Dart, so we change all such conflicts to properties. + // https://github.com/dart-lang/native/issues/1220 + expect(BadOverrideParent.new1().methodVsGetter, 1); + expect(BadOverrideChild.new1().methodVsGetter, 11); + expect(BadOverrideSibbling.new1().methodVsGetter, 12); + expect(BadOverrideGrandchild.new1().methodVsGetter, 111); + + var inst = BadOverrideParent.new1(); + expect(inst.methodVsGetter, 1); + inst = BadOverrideChild.new1(); + expect(inst.methodVsGetter, 11); + inst = BadOverrideSibbling.new1(); + expect(inst.methodVsGetter, 12); + inst = BadOverrideGrandchild.new1(); + expect(inst.methodVsGetter, 111); + + // Uncle isn't affected by the transform, so has an ordinary method. + expect(BadOverrideUncle.new1().methodVsGetter(), 2); + }); + }); +} diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.m b/pkgs/ffigen/test/native_objc_test/bad_override_test.m new file mode 100644 index 000000000..ceed0cecb --- /dev/null +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.m @@ -0,0 +1,51 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#import + +@interface BadOverrideGrandparent : NSObject {} +@end +@implementation BadOverrideGrandparent +@end + +@interface BadOverrideParent : BadOverrideGrandparent {} +-(int32_t)methodVsGetter; +@property (readonly) int32_t methodVsGetter; +@end +@implementation BadOverrideParent +-(int32_t)methodVsGetter { return 1; } +@end + +@interface BadOverrideUncle : BadOverrideGrandparent {} +-(int32_t)methodVsGetter; +@end +@implementation BadOverrideUncle +-(int32_t)methodVsGetter { return 2; } +@end + +@interface BadOverrideAunt : BadOverrideGrandparent {} +@end +@implementation BadOverrideAunt +@end + +@interface BadOverrideChild : BadOverrideParent {} +@property (readonly) int32_t methodVsGetter; +@end +@implementation BadOverrideChild +-(int32_t)methodVsGetter { return 11; } +@end + +@interface BadOverrideSibbling : BadOverrideParent {} +-(int32_t)methodVsGetter; +@end +@implementation BadOverrideSibbling +-(int32_t)methodVsGetter { return 12; } +@end + +@interface BadOverrideGrandchild : BadOverrideParent {} +-(int32_t)methodVsGetter; +@end +@implementation BadOverrideGrandchild +-(int32_t)methodVsGetter { return 111; } +@end