[ffigen] Fix a bug where duplicate methods were added to some classes. (#354)
* Fix #353
* Fix analysis errors
* Fix spurious method mismatch errors
diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart
index 70fa7f2..5c53bda 100644
--- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart
+++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:ffigen/src/code_generator.dart';
+import 'package:logging/logging.dart';
import 'binding_string.dart';
import 'utils.dart';
@@ -33,18 +34,13 @@
'version',
};
+final _logger = Logger('ffigen.code_generator.objc_interface');
+
class ObjCInterface extends BindingType {
ObjCInterface? superType;
- final methods = <ObjCMethod>[];
+ final methods = <String, ObjCMethod>{};
bool filled = false;
- // Objective C supports overriding class methods, but Dart doesn't support
- // overriding static methods. So in our generated Dart code, child classes
- // must explicitly implement all the class methods of their super type. To
- // help with this, we store the class methods in this map, as well as in the
- // methods list.
- final classMethods = <String, ObjCMethod>{};
-
final ObjCBuiltInFunctions builtInFunctions;
late final ObjCInternalGlobal _classObject;
@@ -111,7 +107,7 @@
}
// Methods.
- for (final m in methods) {
+ for (final m in methods.values) {
final methodName = m._getDartMethodName(uniqueNamer);
final isStatic = m.isClass;
final returnType = m.returnType!;
@@ -145,7 +141,7 @@
}
s.write(paramsToString(m.params, isStatic: true));
} else {
- if (superType?.hasMethod(m) ?? false) {
+ if (superType?.methods[m.originalName]?.sameAs(m) ?? false) {
s.write('@override\n ');
}
switch (m.kind) {
@@ -221,59 +217,55 @@
_addNSStringMethods();
}
- _filterPropertyMethods();
-
if (superType != null) {
superType!.addDependencies(dependencies);
_copyClassMethodsFromSuperType();
}
- for (final m in methods) {
+ for (final m in methods.values) {
m.addDependencies(dependencies, builtInFunctions);
}
}
- void _filterPropertyMethods() {
- // Properties setters and getters are duplicated in the AST. One copy is
- // marked it with ObjCMethodKind.propertyGetter/Setter. The other copy is
- // missing important information, and is a plain old instanceMethod. So we
- // need to discard the second copy.
- final properties = Set<String>.from(
- methods.where((m) => m.isProperty).map((m) => m.originalName));
- methods.removeWhere(
- (m) => !m.isProperty && properties.contains(m.originalName));
- }
-
void _copyClassMethodsFromSuperType() {
// Copy class methods from the super type, because Dart classes don't
// inherit static methods.
- for (final m in superType!.classMethods.values) {
- if (!_excludedNSObjectClassMethods.contains(m.originalName)) {
+ for (final m in superType!.methods.values) {
+ if (m.isClass &&
+ !_excludedNSObjectClassMethods.contains(m.originalName)) {
addMethod(m);
}
}
}
void addMethod(ObjCMethod method) {
- methods.add(method);
- if (method.kind == ObjCMethodKind.method && method.isClass) {
- classMethods[method.originalName] ??= method;
+ final oldMethod = methods[method.originalName];
+ if (oldMethod != null) {
+ // Typically we ignore duplicate methods. However, property setters and
+ // getters are duplicated in the AST. One copy is marked with
+ // ObjCMethodKind.propertyGetter/Setter. The other copy is missing
+ // important information, and is a plain old instanceMethod. So if the
+ // existing method is an instanceMethod, and the new one is a property,
+ // override it.
+ if (method.isProperty && !oldMethod.isProperty) {
+ // Fallthrough.
+ } else if (!method.isProperty && oldMethod.isProperty) {
+ // Don't override, but also skip the same method check below.
+ return;
+ } else {
+ // Check duplicate is the same method.
+ if (!method.sameAs(oldMethod)) {
+ _logger.severe('Duplicate methods with different signatures: '
+ '$originalName.${method.originalName}');
+ }
+ return;
+ }
}
- }
-
- bool hasMethod(ObjCMethod method) {
- return methods.any(
- (m) => m.originalName == method.originalName && m.kind == method.kind);
- }
-
- void addMethodIfMissing(ObjCMethod method) {
- if (!hasMethod(method)) {
- addMethod(method);
- }
+ methods[method.originalName] = method;
}
void _addNSStringMethods() {
- addMethodIfMissing(ObjCMethod(
+ addMethod(ObjCMethod(
originalName: 'stringWithCString:encoding:',
kind: ObjCMethodKind.method,
isClass: true,
@@ -283,7 +275,7 @@
ObjCMethodParam(unsignedIntType, 'enc'),
],
));
- addMethodIfMissing(ObjCMethod(
+ addMethod(ObjCMethod(
originalName: 'UTF8String',
kind: ObjCMethodKind.propertyGetter,
isClass: false,
@@ -440,6 +432,15 @@
originalName.replaceAll(RegExp(r":$"), "").replaceAll(":", "_");
return uniqueNamer.makeUnique(name);
}
+
+ bool sameAs(ObjCMethod other) {
+ if (originalName != other.originalName) return false;
+ if (isNullableReturn != other.isNullableReturn) return false;
+ if (kind != other.kind) return false;
+ if (isClass != other.isClass) return false;
+ // msgSend is deduped by signature, so this check covers the signature.
+ return msgSend == other.msgSend;
+ }
}
class ObjCMethodParam {
diff --git a/pkgs/ffigen/test/native_objc_test/category_config.yaml b/pkgs/ffigen/test/native_objc_test/category_config.yaml
index 24c6c2e..92afb06 100644
--- a/pkgs/ffigen/test/native_objc_test/category_config.yaml
+++ b/pkgs/ffigen/test/native_objc_test/category_config.yaml
@@ -8,5 +8,7 @@
headers:
entry-points:
- 'test/native_objc_test/category_test.m'
+ # Include it twice, as a regression test for #353
+ - 'test/native_objc_test/category_test.m'
preamble: |
// ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field
diff --git a/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart b/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart
index 29d1652..6e0328a 100644
--- a/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart
@@ -8718,6 +8718,12 @@
return NSMutableString._(_ret, _lib);
}
+ static ffi.Pointer<NSStringEncoding> getAvailableStringEncodings(
+ NativeObjCLibrary _lib) {
+ return _lib._objc_msgSend_45(
+ _lib._class_NSMutableString1, _lib._sel_availableStringEncodings1);
+ }
+
static NSString localizedNameOfStringEncoding(
NativeObjCLibrary _lib, int encoding) {
final _ret = _lib._objc_msgSend_14(_lib._class_NSMutableString1,
@@ -8725,6 +8731,11 @@
return NSString._(_ret, _lib);
}
+ static int getDefaultCStringEncoding(NativeObjCLibrary _lib) {
+ return _lib._objc_msgSend_11(
+ _lib._class_NSMutableString1, _lib._sel_defaultCStringEncoding1);
+ }
+
static NSMutableString string(NativeObjCLibrary _lib) {
final _ret =
_lib._objc_msgSend_1(_lib._class_NSMutableString1, _lib._sel_string1);
@@ -8829,16 +8840,6 @@
return NSMutableString._(_ret, _lib);
}
- static void availableStringEncodings(NativeObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSMutableString1, _lib._sel_availableStringEncodings1);
- }
-
- static void defaultCStringEncoding(NativeObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSMutableString1, _lib._sel_defaultCStringEncoding1);
- }
-
static int
stringEncodingForData_encodingOptions_convertedString_usedLossyConversion(
NativeObjCLibrary _lib,
diff --git a/pkgs/ffigen/test/native_objc_test/string_bindings.dart b/pkgs/ffigen/test/native_objc_test/string_bindings.dart
index 4614d8e..e742ce0 100644
--- a/pkgs/ffigen/test/native_objc_test/string_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/string_bindings.dart
@@ -9088,6 +9088,12 @@
return NSMutableString._(_ret, _lib);
}
+ static ffi.Pointer<NSStringEncoding> getAvailableStringEncodings(
+ StringTestObjCLibrary _lib) {
+ return _lib._objc_msgSend_45(
+ _lib._class_NSMutableString1, _lib._sel_availableStringEncodings1);
+ }
+
static NSString localizedNameOfStringEncoding(
StringTestObjCLibrary _lib, int encoding) {
final _ret = _lib._objc_msgSend_14(_lib._class_NSMutableString1,
@@ -9095,6 +9101,11 @@
return NSString._(_ret, _lib);
}
+ static int getDefaultCStringEncoding(StringTestObjCLibrary _lib) {
+ return _lib._objc_msgSend_11(
+ _lib._class_NSMutableString1, _lib._sel_defaultCStringEncoding1);
+ }
+
static NSMutableString string(StringTestObjCLibrary _lib) {
final _ret =
_lib._objc_msgSend_1(_lib._class_NSMutableString1, _lib._sel_string1);
@@ -9199,16 +9210,6 @@
return NSMutableString._(_ret, _lib);
}
- static void availableStringEncodings(StringTestObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSMutableString1, _lib._sel_availableStringEncodings1);
- }
-
- static void defaultCStringEncoding(StringTestObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSMutableString1, _lib._sel_defaultCStringEncoding1);
- }
-
static int
stringEncodingForData_encodingOptions_convertedString_usedLossyConversion(
StringTestObjCLibrary _lib,
@@ -9281,6 +9282,12 @@
return NSSimpleCString._(other, lib);
}
+ static ffi.Pointer<NSStringEncoding> getAvailableStringEncodings(
+ StringTestObjCLibrary _lib) {
+ return _lib._objc_msgSend_45(
+ _lib._class_NSSimpleCString1, _lib._sel_availableStringEncodings1);
+ }
+
static NSString localizedNameOfStringEncoding(
StringTestObjCLibrary _lib, int encoding) {
final _ret = _lib._objc_msgSend_14(_lib._class_NSSimpleCString1,
@@ -9288,6 +9295,11 @@
return NSString._(_ret, _lib);
}
+ static int getDefaultCStringEncoding(StringTestObjCLibrary _lib) {
+ return _lib._objc_msgSend_11(
+ _lib._class_NSSimpleCString1, _lib._sel_defaultCStringEncoding1);
+ }
+
static NSSimpleCString string(StringTestObjCLibrary _lib) {
final _ret =
_lib._objc_msgSend_1(_lib._class_NSSimpleCString1, _lib._sel_string1);
@@ -9392,16 +9404,6 @@
return NSSimpleCString._(_ret, _lib);
}
- static void availableStringEncodings(StringTestObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSSimpleCString1, _lib._sel_availableStringEncodings1);
- }
-
- static void defaultCStringEncoding(StringTestObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSSimpleCString1, _lib._sel_defaultCStringEncoding1);
- }
-
static int
stringEncodingForData_encodingOptions_convertedString_usedLossyConversion(
StringTestObjCLibrary _lib,
@@ -9472,6 +9474,12 @@
return NSConstantString._(other, lib);
}
+ static ffi.Pointer<NSStringEncoding> getAvailableStringEncodings(
+ StringTestObjCLibrary _lib) {
+ return _lib._objc_msgSend_45(
+ _lib._class_NSConstantString1, _lib._sel_availableStringEncodings1);
+ }
+
static NSString localizedNameOfStringEncoding(
StringTestObjCLibrary _lib, int encoding) {
final _ret = _lib._objc_msgSend_14(_lib._class_NSConstantString1,
@@ -9479,6 +9487,11 @@
return NSString._(_ret, _lib);
}
+ static int getDefaultCStringEncoding(StringTestObjCLibrary _lib) {
+ return _lib._objc_msgSend_11(
+ _lib._class_NSConstantString1, _lib._sel_defaultCStringEncoding1);
+ }
+
static NSConstantString string(StringTestObjCLibrary _lib) {
final _ret =
_lib._objc_msgSend_1(_lib._class_NSConstantString1, _lib._sel_string1);
@@ -9583,16 +9596,6 @@
return NSConstantString._(_ret, _lib);
}
- static void availableStringEncodings(StringTestObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSConstantString1, _lib._sel_availableStringEncodings1);
- }
-
- static void defaultCStringEncoding(StringTestObjCLibrary _lib) {
- _lib._objc_msgSend_0(
- _lib._class_NSConstantString1, _lib._sel_defaultCStringEncoding1);
- }
-
static int
stringEncodingForData_encodingOptions_convertedString_usedLossyConversion(
StringTestObjCLibrary _lib,
