[ffigen] Prevent imported types from being renamed. (#2791)

Author: liamappelbe

pkgs/ffigen/CHANGELOG.md

@@ -17,6 +17,8 @@
   NS_OPTIONS enums.
 - Fix [a bug](https://github.com/dart-lang/native/issues/2760) in the internal
   variables generated by function bindings.
+- Fix [a bug](https://github.com/dart-lang/native/issues/2762) where types
+  imported from package:objective_c could be renamed.
 
 ## 20.0.0
 

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

@@ -43,7 +43,11 @@ abstract class Binding extends AstNode implements Declaration {
     required Symbol symbol,
     this.dartDoc,
     this.isInternal = false,
-  }) : _symbol = symbol;
+  }) : _symbol = symbol {
+    // Ideally isImported would be part of the Symbol constructor, but then we
+    // wouldn't be able to use this.isObjCImport.
+    _symbol.isImported = isObjCImport;
+  }
 
   /// Converts a Binding to its actual string representation.
   ///

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

@@ -79,7 +79,9 @@ class Scope {
     _namer = namer;
     for (final symbol in _symbols) {
       if (symbol._name == null) {
-        symbol._name = namer.add(symbol.oldName, symbol.kind);
+        symbol._name = symbol.isImported
+            ? symbol.oldName
+            : namer.add(symbol.oldName, symbol.kind);
       } else {
         // Symbol already has a name. This can happen if the symbol is in
         // multiple scopes. It's fine as long as the name isn't used by a
@@ -161,6 +163,7 @@ class Namer {
 class Symbol extends AstNode {
   final String oldName;
   final SymbolKind kind;
+  bool isImported = false;
   String? _name;
 
   /// Only valid if [Scope.fillNames] has been called already.

pkgs/ffigen/test/native_objc_test/category_config.yaml

@@ -8,6 +8,7 @@ objc-interfaces:
     - Thing
     - ChildOfThing
     - NSURL
+    - ChildOfNSString
 objc-categories:
   include:
     - Sub
@@ -16,6 +17,7 @@ objc-categories:
     - InstanceTypeCategory
     - InterfaceOnBuiltInType
     - StaticAndInstanceMethodsWithSameNameCategory
+    - NSString
 headers:
   entry-points:
     - 'category_test.h'

pkgs/ffigen/test/native_objc_test/category_test.dart

@@ -7,7 +7,7 @@
 import 'dart:ffi';
 import 'dart:io';
 
-import 'package:objective_c/objective_c.dart';
+import 'package:objective_c/objective_c.dart' as objc;
 import 'package:path/path.dart' as path;
 import 'package:test/test.dart';
 
@@ -87,14 +87,14 @@ void main() {
         'GoodbyeWorld!',
       );
 
-      NSString str2 = str.instancetypeMethod();
+      objc.NSString str2 = str.instancetypeMethod();
       expect(str2.toDartString(), '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);
+      expect(objc.NSURL.alloc().extensionMethod(), 555);
 
       // Don't include transitive category of built-in type that hasn't been
       // explicitly included.
@@ -111,5 +111,29 @@ void main() {
       // This method is from an NSObject extension, which shouldn't be included.
       expect(bindings, isNot(contains('autoContentAccessingProxy')));
     });
+
+    test('Category that has the same name as an imported type that is the '
+        'supertype of another type in the same file', () {
+      // Regression test for https://github.com/dart-lang/native/issues/2762.
+      final bindings = File(
+        path.join(
+          packagePathForTests,
+          'test',
+          'native_objc_test',
+          'category_bindings.dart',
+        ),
+      ).readAsStringSync();
+
+      // Neither the NSString category, nor the NSString that's a supertype of
+      // ChildOfNSString, have been renamed to NSString$1.
+      expect(bindings, contains('extension NSString on Thing {'));
+      expect(
+        bindings,
+        contains('''
+extension type ChildOfNSString._(objc.ObjCObject object\$)
+    implements objc.ObjCObject, objc.NSString {
+'''),
+      );
+    });
   });
 }

pkgs/ffigen/test/native_objc_test/category_test.h

@@ -59,3 +59,13 @@
 -(int32_t)sameNameMethod;
 +(int32_t)sameNameMethod;
 @end
+
+// Category with the same name as a built in type.
+@interface Thing (NSString)
+-(int32_t)nsStringExtension;
+@end
+
+// Extend the object with the same-named category.
+// https://github.com/dart-lang/native/issues/2762
+@interface ChildOfNSString : NSString {}
+@end