[jnigen] Rename identifiers starting with underscore (#1551)

Author: HosseinYousefi

pkgs/jnigen/CHANGELOG.md

@@ -6,9 +6,11 @@
   is done to avoid name collision between methods that originally end with
   numeric suffices and the renamed overloads. Similarly names that are Dart
   keywords get a dollar sign suffix now. For more information, check out the
-  [documentation](https://github.com/dart-lang/native/tree/main/pkgs/jnigen/docs/java_differences.md#method_overloading).
+  [documentation](https://github.com/dart-lang/native/tree/main/pkgs/jnigen/docs/java_differences.md#method-overloading).
 - **Breaking Change**: Each single dollar sign is replaced with two dollar signs
   in the identifier names.
+- Generating identifiers that start with an underscore (`_`) and making them
+  public by prepending a dollar sign.
 - Fixed an issue where inheriting a generic class could generate incorrect code.
 - No longer generating constructors for abstract classes.
 - No longer generating `protected` elements.

pkgs/jnigen/docs/java_differences.md

@@ -186,3 +186,30 @@ JNIgen replaces each single dollar sign with two dollar signs. For example
 `generated$method$2` will turn into `generated$$method$$2`. This prevents name
 collision as JNIgen-renamed identifiers will end with an odd number of dollar
 signs (optionally followed by a numeric suffix).
+
+### Identifier starting with underscore (`_`)
+
+Unlike Java, Dart identifiers that start with an underscore are private. This
+means they are inaccessible outside their defining scope. Users should still be
+able to access public Java identifiers that start with an underscore from the
+generated Dart bindings. To allow this, JNIgen prepends such identifiers with a
+dollar sign to keep them public in Dart.
+
+For example, `_Example` in Java will be converted to `$_Example` in the Dart
+bindings:
+
+```java
+// Java
+public class _Example {
+  public void _printMessage() {
+    System.out.println("Hello from Java!");
+  }
+}
+```
+
+```dart
+// Dart Bindings - Boilerplate omitted for clarity.
+class $_Example extends JObject {
+  void $_printMessage() { /* ... */ }
+}
+```

pkgs/jnigen/lib/src/bindings/excluder.dart

@@ -9,7 +9,6 @@ import 'visitor.dart';
 
 extension on ClassMember {
   bool get isPrivate => !isPublic;
-  bool get hasPrivateName => name.startsWith('_');
 }
 
 class Excluder extends Visitor<Classes, void> {
@@ -21,7 +20,6 @@ class Excluder extends Visitor<Classes, void> {
   void visit(Classes node) {
     node.decls.removeWhere((_, classDecl) {
       final excluded = classDecl.isPrivate ||
-          classDecl.hasPrivateName ||
           !(config.exclude?.classes?.included(classDecl) ?? true);
       if (excluded) {
         log.fine('Excluded class ${classDecl.binaryName}');
@@ -44,25 +42,20 @@ class _ClassExcluder extends Visitor<ClassDecl, void> {
   void visit(ClassDecl node) {
     node.methods = node.methods.where((method) {
       final isPrivate = method.isPrivate;
-      final hasPrivateName = method.hasPrivateName;
       final isAbstractCtor = method.isConstructor && node.isAbstract;
       final isBridgeMethod = method.isSynthetic && method.isBridge;
       final isExcludedInConfig =
           config.exclude?.methods?.included(node, method) ?? false;
-      final excluded = isPrivate ||
-          hasPrivateName ||
-          isAbstractCtor ||
-          isBridgeMethod ||
-          isExcludedInConfig;
+      final excluded =
+          isPrivate || isAbstractCtor || isBridgeMethod || isExcludedInConfig;
       if (excluded) {
         log.fine('Excluded method ${node.binaryName}#${method.name}');
       }
       return !excluded;
     }).toList();
     node.fields = node.fields.where((field) {
-      final excluded = field.isPrivate ||
-          field.hasPrivateName &&
-              (config.exclude?.fields?.included(node, field) ?? true);
+      final excluded = field.isPrivate &&
+          (config.exclude?.fields?.included(node, field) ?? true);
       if (excluded) {
         log.fine('Excluded field ${node.binaryName}#${field.name}');
       }

pkgs/jnigen/lib/src/bindings/renamer.dart

@@ -93,20 +93,24 @@ const Map<String, int> _definedSyms = {
   'type': 1,
 };
 
-/// Replaces each dollar sign with two dollar signs in [name].
-///
-/// Examples:
-/// * `$foo$$bar$` -> `$$foo$$$$bar$$`
-/// * `foo` -> `foo`
-String _doubleDollarSigns(String name) {
-  return name.replaceAll(r'$', r'$$');
+String _preprocess(String name) {
+  // Replaces the `_` prefix with `$_` to prevent hiding public members in Dart.
+  // For example `_foo` -> `$_foo`.
+  String makePublic(String name) =>
+      name.startsWith('_') ? '\$_${name.substring(1)}' : name;
+
+  // Replaces each dollar sign with two dollar signs in [name].
+  // For example `$foo$$bar$` -> `$$foo$$$$bar$$`.
+  String doubleDollarSigns(String name) => name.replaceAll(r'$', r'$$');
+
+  return makePublic(doubleDollarSigns(name));
 }
 
 /// Appends `$` to [name] if [name] is a Dart keyword.
 ///
 /// Examples:
 /// * `yield` -> `yield$`
-/// * `i` -> `i`
+/// * `foo` -> `foo`
 String _keywordRename(String name) =>
     _keywords.contains(name) ? '$name\$' : name;
 
@@ -159,7 +163,7 @@ class _ClassRenamer implements Visitor<ClassDecl, void> {
     // should continue to use dollar sign.
     // TODO(https://github.com/dart-lang/native/issues/1544): Class names can
     // have dollar signs even if not nested.
-    final className = node.name.replaceAll(r'$', '_');
+    final className = _preprocess(node.name.replaceAll(r'$', '_'));
 
     // When generating all the classes in a single file
     // the names need to be unique.
@@ -201,7 +205,7 @@ class _MethodRenamer implements Visitor<Method, void> {
 
   @override
   void visit(Method node) {
-    final name = _doubleDollarSigns(node.isConstructor ? 'new' : node.name);
+    final name = _preprocess(node.isConstructor ? 'new' : node.name);
     final sig = node.javaSig;
     // If node is in super class, assign its number, overriding it.
     final superClass =
@@ -243,7 +247,7 @@ class _FieldRenamer implements Visitor<Field, void> {
 
   @override
   void visit(Field node) {
-    final fieldName = _doubleDollarSigns(node.name);
+    final fieldName = _preprocess(node.name);
     node.finalName = _renameConflict(nameCounts, fieldName);
     log.fine('Field ${node.classDecl.binaryName}#${node.name}'
         ' is named ${node.finalName}');

pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/JsonFactory.dart

@@ -2715,6 +2715,37 @@ class JsonFactory extends jni.JObject {
             out.reference.pointer)
         .object(const jni.JObjectType());
   }
+
+  static final _id_$_getBufferRecycler = _class.instanceMethodId(
+    r'_getBufferRecycler',
+    r'()Lcom/fasterxml/jackson/core/util/BufferRecycler;',
+  );
+
+  static final _$_getBufferRecycler = ProtectedJniExtensions.lookup<
+          ffi.NativeFunction<
+              jni.JniResult Function(
+                ffi.Pointer<ffi.Void>,
+                jni.JMethodIDPtr,
+              )>>('globalEnv_CallObjectMethod')
+      .asFunction<
+          jni.JniResult Function(
+            ffi.Pointer<ffi.Void>,
+            jni.JMethodIDPtr,
+          )>();
+
+  /// from: `public com.fasterxml.jackson.core.util.BufferRecycler _getBufferRecycler()`
+  /// The returned object must be released after use, by calling the [release] method.
+  ///
+  /// Method used by factory to create buffer recycler instances
+  /// for parsers and generators.
+  ///
+  /// Note: only public to give access for {@code ObjectMapper}
+  ///@return Buffer recycler instance to use
+  jni.JObject $_getBufferRecycler() {
+    return _$_getBufferRecycler(
+            reference.pointer, _id_$_getBufferRecycler as jni.JMethodIDPtr)
+        .object(const jni.JObjectType());
+  }
 }
 
 final class $JsonFactoryType extends jni.JObjType<JsonFactory> {

pkgs/jnigen/test/renamer_test.dart

@@ -11,17 +11,18 @@ extension on Iterable<ClassMember> {
   List<String> get finalNames => map((c) => c.finalName).toList();
 }
 
-Future<Config> testConfig() async {
+Future<void> rename(Classes classes) async {
   final config = Config(
     outputConfig: OutputConfig(
       dartConfig: DartCodeOutputConfig(
         path: Uri.file('test.dart'),
         structure: OutputStructure.singleFile,
       ),
     ),
-    classes: ['Foo'],
+    classes: [],
   );
-  return config;
+  await classes.accept(Linker(config));
+  classes.accept(Renamer(config));
 }
 
 void main() {
@@ -76,9 +77,7 @@ void main() {
         superclass: TypeUsage.object,
       ),
     });
-    final config = await testConfig();
-    await classes.accept(Linker(config));
-    classes.accept(Renamer(config));
+    await rename(classes);
 
     final renamedClasses = classes.decls.values.finalNames;
     expect(renamedClasses, [
@@ -137,9 +136,7 @@ void main() {
         ],
       ),
     });
-    final config = await testConfig();
-    await classes.accept(Linker(config));
-    classes.accept(Renamer(config));
+    await rename(classes);
 
     final renamedMethods = classes.decls['Player']!.methods.finalNames;
     expect(renamedMethods, ['duck']);
@@ -168,9 +165,7 @@ void main() {
         superclass: TypeUsage.object,
       ),
     });
-    final config = await testConfig();
-    await classes.accept(Linker(config));
-    classes.accept(Renamer(config));
+    await rename(classes);
 
     final renamedMethods = classes.decls['Foo']!.methods.finalNames;
     expect(renamedMethods, [r'yield$', r'const$1', r'const$2']);
@@ -203,9 +198,7 @@ void main() {
         ],
       ),
     });
-    final config = await testConfig();
-    await classes.accept(Linker(config));
-    classes.accept(Renamer(config));
+    await rename(classes);
 
     final renamedMethods = classes.decls['Foo']!.methods.finalNames;
     expect(renamedMethods, [
@@ -223,4 +216,53 @@ void main() {
       r'alsoAField$$1',
     ]);
   });
+
+  test('Names that start with underscores', () async {
+    final classes = Classes({
+      '_Foo': ClassDecl(
+        binaryName: '_Foo',
+        declKind: DeclKind.classKind,
+        superclass: TypeUsage.object,
+        methods: [
+          Method(name: '_foo', returnType: TypeUsage.object),
+        ],
+        fields: [
+          Field(name: '_bar', type: TypeUsage.object),
+        ],
+      ),
+    });
+    await rename(classes);
+
+    final renamedMethods = classes.decls['_Foo']!.methods.finalNames;
+    expect(renamedMethods, [r'$_foo']);
+    final renamedFields = classes.decls['_Foo']!.fields.finalNames;
+    expect(renamedFields, [r'$_bar']);
+    final renamedClasses = classes.decls.values.finalNames;
+    expect(renamedClasses, [r'$_Foo']);
+  });
+
+  test('Combination of underscore, dollar signs, and overloading', () async {
+    // TODO(https://github.com/dart-lang/native/issues/1544): Test class names
+    // containing dollar signs.
+    final classes = Classes({
+      r'_Foo$': ClassDecl(
+        binaryName: r'_Foo$',
+        declKind: DeclKind.classKind,
+        superclass: TypeUsage.object,
+        methods: [
+          Method(name: r'_foo$', returnType: TypeUsage.object),
+          Method(name: r'_foo$', returnType: TypeUsage.object),
+        ],
+        fields: [
+          Field(name: r'_foo$', type: TypeUsage.object),
+        ],
+      ),
+    });
+    await rename(classes);
+
+    final renamedMethods = classes.decls[r'_Foo$']!.methods.finalNames;
+    expect(renamedMethods, [r'$_foo$$$1', r'$_foo$$$2']);
+    final renamedFields = classes.decls[r'_Foo$']!.fields.finalNames;
+    expect(renamedFields, [r'$_foo$$']);
+  });
 }