Some quick fixes for ffigen (#1246)

Author: liamappelbe

pkgs/ffigen/CHANGELOG.md

@@ -25,6 +25,8 @@
   https://github.com/dart-lang/native/issues/1007.
 - Added support for implementing ObjC protocols from Dart. Use the
   `objc-protocols` config option to generate bindings for a protocol.
+- Fix some bugs where ObjC interface/protocol methods could collide with Dart
+  built-in methods, or with types declared elsewhere in the generated bindings.
 
 ## 12.0.0
 

pkgs/ffigen/lib/src/code_generator/compound.dart

@@ -58,7 +58,7 @@ abstract class Compound extends BindingType {
     int? pack,
     String? dartDoc,
     List<Member>? members,
-    required ObjCBuiltInFunctions objCBuiltInFunctions,
+    ObjCBuiltInFunctions? objCBuiltInFunctions,
   }) {
     switch (type) {
       case CompoundType.struct:
@@ -95,7 +95,7 @@ abstract class Compound extends BindingType {
   }
 
   bool get _isBuiltIn =>
-      objCBuiltInFunctions?.isBuiltInInterface(originalName) ?? false;
+      objCBuiltInFunctions?.isBuiltInCompound(originalName) ?? false;
 
   @override
   BindingString toBindingString(Writer w) {

pkgs/ffigen/lib/src/code_generator/enum_class.dart

@@ -7,6 +7,7 @@ import 'package:collection/collection.dart';
 import 'binding.dart';
 import 'binding_string.dart';
 import 'imports.dart';
+import 'objc_built_in_functions.dart';
 import 'type.dart';
 import 'utils.dart';
 import 'writer.dart';
@@ -49,13 +50,16 @@ class EnumClass extends BindingType {
   /// Generates new names for all members that don't equal [name].
   final UniqueNamer namer;
 
+  ObjCBuiltInFunctions? objCBuiltInFunctions;
+
   EnumClass({
     super.usr,
     super.originalName,
     required super.name,
     super.dartDoc,
     Type? nativeType,
     List<EnumConstant>? enumConstants,
+    this.objCBuiltInFunctions,
   })  : nativeType = nativeType ?? intType,
         enumConstants = enumConstants ?? [],
         namer = UniqueNamer({name});
@@ -209,9 +213,15 @@ class EnumClass extends BindingType {
     s.write("$depth};\n");
   }
 
+  bool get _isBuiltIn =>
+      objCBuiltInFunctions?.isBuiltInEnum(originalName) ?? false;
+
   @override
   BindingString toBindingString(Writer w) {
     final s = StringBuffer();
+    if (_isBuiltIn) {
+      return BindingString(type: BindingStringType.enum_, string: '');
+    }
     scanForDuplicates();
 
     writeDartDoc(s);
@@ -239,7 +249,7 @@ class EnumClass extends BindingType {
 
   @override
   void addDependencies(Set<Binding> dependencies) {
-    if (dependencies.contains(this)) return;
+    if (dependencies.contains(this) || _isBuiltIn) return;
     dependencies.add(this);
   }
 
@@ -253,7 +263,8 @@ class EnumClass extends BindingType {
   String getFfiDartType(Writer w) => nativeType.getFfiDartType(w);
 
   @override
-  String getDartType(Writer w) => name;
+  String getDartType(Writer w) =>
+      _isBuiltIn ? '${w.objcPkgPrefix}.$name' : name;
 
   @override
   String getNativeType({String varName = ''}) => '$originalName $varName';
@@ -282,7 +293,7 @@ class EnumClass extends BindingType {
     required bool objCRetain,
     String? objCEnclosingClass,
   }) =>
-      "$name.fromValue($value)";
+      "${getDartType(w)}.fromValue($value)";
 }
 
 /// Represents a single value in an enum.

pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart

@@ -71,13 +71,36 @@ class ObjCBuiltInFunctions {
     'NSFastEnumerationState',
     'NSRange',
   };
+  static const builtInEnums = {
+    'NSBinarySearchingOptions',
+    'NSComparisonResult',
+    'NSDataBase64DecodingOptions',
+    'NSDataBase64EncodingOptions',
+    'NSDataCompressionAlgorithm',
+    'NSDataReadingOptions',
+    'NSDataSearchOptions',
+    'NSDataWritingOptions',
+    'NSEnumerationOptions',
+    'NSItemProviderFileOptions',
+    'NSItemProviderRepresentationVisibility',
+    'NSOrderedCollectionDifferenceCalculationOptions',
+    'NSSortOptions',
+    'NSStringCompareOptions',
+    'NSStringEncodingConversionOptions',
+    'NSStringEnumerationOptions',
+    'NSURLBookmarkCreationOptions',
+    'NSURLBookmarkResolutionOptions',
+    'NSURLHandleStatus',
+  };
 
   // TODO(https://github.com/dart-lang/native/issues/1173): Ideally this check
   // would be based on more than just the name.
   bool isBuiltInInterface(String name) =>
       !generateForPackageObjectiveC && builtInInterfaces.contains(name);
   bool isBuiltInCompound(String name) =>
       !generateForPackageObjectiveC && builtInCompounds.contains(name);
+  bool isBuiltInEnum(String name) =>
+      !generateForPackageObjectiveC && builtInEnums.contains(name);
   bool isNSObject(String name) => name == 'NSObject';
 
   // We need to load a separate instance of objc_msgSend for each signature. If

pkgs/ffigen/lib/src/code_generator/objc_interface.dart

@@ -75,8 +75,7 @@ class ObjCInterface extends BindingType with ObjCMethods {
     final s = StringBuffer();
     s.write(makeDartDoc(dartDoc ?? originalName));
 
-    final uniqueNamer =
-        UniqueNamer({name, 'pointer'}, parent: w.topLevelUniqueNamer);
+    final methodNamer = createMethodRenamer(w);
 
     final rawObjType = PointerType(objCObjectType).getCType(w);
     final wrapObjType = ObjCBuiltInFunctions.objectBase.gen(w);
@@ -113,7 +112,7 @@ class ObjCInterface extends BindingType with ObjCMethods {
 
     // Methods.
     for (final m in methods) {
-      final methodName = m.getDartMethodName(uniqueNamer);
+      final methodName = m.getDartMethodName(methodNamer);
       final isStatic = m.isClassMethod;
       final isStret = m.msgSend!.isStret;
 

pkgs/ffigen/lib/src/code_generator/objc_methods.dart

@@ -6,6 +6,7 @@ import 'package:ffigen/src/code_generator.dart';
 import 'package:logging/logging.dart';
 
 import 'utils.dart';
+import 'writer.dart';
 
 final _logger = Logger('ffigen.code_generator.objc_methods');
 
@@ -16,6 +17,7 @@ mixin ObjCMethods {
   ObjCMethod? getMethod(String name) => _methods[name];
 
   String get originalName;
+  String get name;
   ObjCBuiltInFunctions get builtInFunctions;
 
   void addMethod(ObjCMethod method) {
@@ -93,6 +95,10 @@ mixin ObjCMethods {
 
         return true;
       });
+
+  UniqueNamer createMethodRenamer(Writer w) => UniqueNamer(
+      {name, 'pointer', 'toString', 'hashCode', 'runtimeType', 'noSuchMethod'},
+      parent: w.topLevelUniqueNamer);
 }
 
 enum ObjCMethodKind {
@@ -149,10 +155,10 @@ class ObjCMethod {
     for (final p in params) {
       p.type.addDependencies(dependencies);
     }
-    selObject ??= builtInFunctions.getSelObject(originalName)
+    selObject = builtInFunctions.getSelObject(originalName)
       ..addDependencies(dependencies);
     if (needMsgSend) {
-      msgSend ??= builtInFunctions.getMsgSendFunc(returnType, params)
+      msgSend = builtInFunctions.getMsgSendFunc(returnType, params)
         ..addDependencies(dependencies);
     }
     if (needProtocolBlock) {

pkgs/ffigen/lib/src/code_generator/objc_protocol.dart

@@ -39,10 +39,10 @@ class ObjCProtocol extends NoLookUpBinding with ObjCMethods {
     final buildImplementations = StringBuffer();
     final methodFields = StringBuffer();
 
-    final uniqueNamer = UniqueNamer({name, 'pointer'});
+    final methodNamer = createMethodRenamer(w);
 
     for (final method in methods) {
-      final methodName = method.getDartMethodName(uniqueNamer);
+      final methodName = method.getDartMethodName(methodNamer);
       final fieldName = methodName;
       final argName = methodName;
       final block = method.protocolBlock;

pkgs/ffigen/lib/src/header_parser/sub_parsers/enumdecl_parser.dart

@@ -55,6 +55,7 @@ final _logger = Logger('ffigen.header_parser.enumdecl_parser');
       originalName: enumName,
       name: config.enumClassDecl.renameUsingConfig(enumName),
       nativeType: nativeType,
+      objCBuiltInFunctions: objCBuiltInFunctions,
     );
     cursor.visitChildren((clang_types.CXCursor child) {
       try {

pkgs/ffigen/test/native_objc_test/rename_config.yaml

@@ -16,6 +16,9 @@ objc-interfaces:
     - _Renamed
   rename:
     '_(.*)': '$1'
+structs:
+  include:
+    - CollidingStructName
 headers:
   entry-points:
     - 'rename_test.m'

pkgs/ffigen/test/native_objc_test/rename_test.dart

@@ -27,8 +27,23 @@ void main() {
 
     test('Renamed class', () {
       final renamed = Renamed.new1();
+
       renamed.property = 123;
       expect(renamed.property, 123);
     });
+
+    test('Method with the same name as a Dart built in method', () {
+      final renamed = Renamed.new1();
+
+      renamed.property = 123;
+      expect(renamed.toString(), "Instance of 'Renamed'");
+      expect(renamed.toString1().toString(), "123");
+    });
+
+    test('Method with the same name as a type', () {
+      final renamed = Renamed.new1();
+
+      expect(renamed.CollidingStructName1(), 456);
+    });
   });
 }