[ffigen] Split protocol building stuff into a $Builder class (#2708)

Author: liamappelbe

pkgs/ffigen/CHANGELOG.md

@@ -13,6 +13,10 @@
   With this breaking change, also some defaults changed: (1) `@Native` bindings
   are now the default, and (2) struct/unions refered to by pointer will be
   generated as `Opaque` by default.
+- __Breaking change__: Change how ObjC protocols are generated, splitting the
+  methods related to constructing instances into a separate `Foo$Builder` class.
+  The protocol's instance methods are now directly invokable from the built
+  object.
 - __Breaking change__: Minor breaking change in the way that ObjC interface
   methods are generated. Interface methods are now generated as extension
   methods instead of being part of the class. This shouldn't require any code

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

@@ -117,10 +117,10 @@ mixin ObjCMethods {
   String generateStaticMethodBindings(Writer w, ObjCInterface target) =>
       _generateMethods(w, target, true);
 
-  String generateInstanceMethodBindings(Writer w, ObjCInterface target) =>
+  String generateInstanceMethodBindings(Writer w, BindingType target) =>
       _generateMethods(w, target, false);
 
-  String _generateMethods(Writer w, ObjCInterface target, bool? staticMethods) {
+  String _generateMethods(Writer w, BindingType target, bool? staticMethods) {
     return [
       for (final m in methods)
         if (staticMethods == null || staticMethods == m.isClassMethod)
@@ -423,7 +423,10 @@ class ObjCMethod extends AstNode with HasLocalScope {
     return '${_paramToStr(context, params.first)}, {$named}';
   }
 
-  String generateBindings(Writer w, ObjCInterface target) {
+  String generateBindings(Writer w, BindingType target) {
+    // Class methods are only supported for ObjCInterface targets.
+    assert(target is ObjCInterface || !isClassMethod);
+
     final context = w.context;
     final methodName = symbol.name;
     final upperName = methodName[0].toUpperCase() + methodName.substring(1);
@@ -446,7 +449,7 @@ class ObjCMethod extends AstNode with HasLocalScope {
     s.write('\n  ${makeDartDoc(dartDoc)}  ');
     late String targetStr;
     if (isClassMethod) {
-      targetStr = target.classObject.name;
+      targetStr = (target as ObjCInterface).classObject.name;
       switch (kind) {
         case ObjCMethodKind.method:
           s.write('static $returnTypeStr $methodName($paramStr)');

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

@@ -106,10 +106,45 @@ interface class $name extends $protocolBase $impls{
   $name.castFromPointer($rawObjType other,
       {bool retain = false, bool release = false}) :
       this._(other, retain: retain, release: release);
+''');
+
+    if (!generateAsStub) {
+      final msgSendInvoke = _conformsToMsgSend.invoke(
+        context,
+        'obj.ref.pointer',
+        _conformsTo.name,
+        [_protocolPointer.name],
+      );
+
+      s.write('''
+
+  /// Returns whether [obj] is an instance of [$name].
+  static bool conformsTo($objectBase obj) {
+    return $msgSendInvoke;
+  }
+''');
+    }
+
+    s.write('''
+}
+
+''');
+
+    if (!generateAsStub) {
+      s.write('''
+extension $name\$Methods on $name {
+${generateInstanceMethodBindings(w, this)}
+}
 
 ''');
+    }
 
     if (!generateAsStub) {
+      final builder = '$name\$Builder';
+      s.write('''
+  interface class $builder {
+  ''');
+
       final buildArgs = <String>[];
       final buildImplementations = StringBuffer();
       final buildListenerImplementations = StringBuffer();
@@ -163,11 +198,11 @@ interface class $name extends $protocolBase $impls{
         }
 
         buildImplementations.write('''
-    $name.$fieldName.implement(builder, $argName);''');
+    $builder.$fieldName.implement(builder, $argName);''');
         buildListenerImplementations.write('''
-    $name.$fieldName.$maybeImplementAsListener(builder, $argName);''');
+    $builder.$fieldName.$maybeImplementAsListener(builder, $argName);''');
         buildBlockingImplementations.write('''
-    $name.$fieldName.$maybeImplementAsBlocking(builder, $argName);''');
+    $builder.$fieldName.$maybeImplementAsBlocking(builder, $argName);''');
 
         methodFields.write(makeDartDoc(method.dartDoc ?? method.originalName));
         methodFields.write('''static final $fieldName = $methodClass<$funcType>(
@@ -268,26 +303,14 @@ interface class $name extends $protocolBase $impls{
 ''';
       }
 
-      final msgSendInvoke = _conformsToMsgSend.invoke(
-        context,
-        'obj.ref.pointer',
-        _conformsTo.name,
-        [_protocolPointer.name],
-      );
       s.write('''
-  /// Returns whether [obj] is an instance of [$name].
-  static bool conformsTo($objectBase obj) {
-    return $msgSendInvoke;
-  }
 
   $builders
   $listenerBuilders
   $methodFields
-''');
-    }
-    s.write('''
 }
 ''');
+    }
 
     return BindingString(
       type: BindingStringType.objcProtocol,

pkgs/ffigen/lib/src/visitor/fill_method_dependencies.dart

@@ -31,6 +31,7 @@ class FillMethodDependenciesVisitation extends Visitation {
 
     for (final method in node.methods) {
       method.fillProtocolBlock();
+      method.fillMsgSend();
     }
   }
 }

pkgs/ffigen/test/native_objc_test/protocol_test.dart

@@ -143,7 +143,7 @@ void main() {
       test('Method implementation', () {
         final consumer = ProtocolConsumer();
 
-        final MyProtocol myProtocol = MyProtocol.implement(
+        final MyProtocol myProtocol = MyProtocol$Builder.implement(
           instanceMethod_withDouble_: (NSString s, double x) {
             return 'MyProtocol: ${s.toDartString()}: $x'.toNSString();
           },
@@ -168,7 +168,7 @@ void main() {
         final consumer = ProtocolConsumer();
 
         final protocolBuilder = ObjCProtocolBuilder();
-        MyProtocol.addToBuilder(
+        MyProtocol$Builder.addToBuilder(
           protocolBuilder,
           instanceMethod_withDouble_: (NSString s, double x) {
             return 'ProtocolBuilder: ${s.toDartString()}: $x'.toNSString();
@@ -177,7 +177,7 @@ void main() {
             return s.y - s.x;
           },
         );
-        SecondaryProtocol.addToBuilder(
+        SecondaryProtocol$Builder.addToBuilder(
           protocolBuilder,
           otherMethod_b_c_d_: (int a, int b, int c, int d) {
             return a * b * c * d;
@@ -208,20 +208,18 @@ void main() {
         final consumer = ProtocolConsumer();
 
         final protocolBuilder = ObjCProtocolBuilder();
-        MyProtocol.instanceMethod_withDouble_.implement(protocolBuilder, (
-          NSString s,
-          double x,
-        ) {
-          return 'ProtocolBuilder: ${s.toDartString()}: $x'.toNSString();
-        });
-        SecondaryProtocol.otherMethod_b_c_d_.implement(protocolBuilder, (
-          int a,
-          int b,
-          int c,
-          int d,
-        ) {
-          return a * b * c * d;
-        });
+        MyProtocol$Builder.instanceMethod_withDouble_.implement(
+          protocolBuilder,
+          (NSString s, double x) {
+            return 'ProtocolBuilder: ${s.toDartString()}: $x'.toNSString();
+          },
+        );
+        SecondaryProtocol$Builder.otherMethod_b_c_d_.implement(
+          protocolBuilder,
+          (int a, int b, int c, int d) {
+            return a * b * c * d;
+          },
+        );
         final protocolImpl = protocolBuilder.build();
         final MyProtocol asMyProtocol = MyProtocol.castFrom(protocolImpl);
         final SecondaryProtocol asSecondaryProtocol =
@@ -239,7 +237,7 @@ void main() {
       test('Unimplemented method', () {
         final consumer = ProtocolConsumer();
 
-        final MyProtocol myProtocol = MyProtocol.implement(
+        final MyProtocol myProtocol = MyProtocol$Builder.implement(
           instanceMethod_withDouble_: (NSString s, double x) {
             throw UnimplementedError();
           },
@@ -254,7 +252,7 @@ void main() {
         final consumer = ProtocolConsumer();
 
         final listenerCompleter = Completer<int>();
-        final MyProtocol myProtocol = MyProtocol.implementAsListener(
+        final MyProtocol myProtocol = MyProtocol$Builder.implementAsListener(
           instanceMethod_withDouble_: (NSString s, double x) {
             return 'MyProtocol: ${s.toDartString()}: $x'.toNSString();
           },
@@ -284,7 +282,7 @@ void main() {
 
         final listenerCompleter = Completer<int>();
         final protocolBuilder = ObjCProtocolBuilder();
-        MyProtocol.addToBuilderAsListener(
+        MyProtocol$Builder.addToBuilderAsListener(
           protocolBuilder,
           instanceMethod_withDouble_: (NSString s, double x) {
             return 'ProtocolBuilder: ${s.toDartString()}: $x'.toNSString();
@@ -293,7 +291,7 @@ void main() {
             listenerCompleter.complete(x);
           },
         );
-        SecondaryProtocol.addToBuilder(
+        SecondaryProtocol$Builder.addToBuilder(
           protocolBuilder,
           otherMethod_b_c_d_: (int a, int b, int c, int d) {
             return a * b * c * d;
@@ -329,7 +327,7 @@ void main() {
         final consumer = ProtocolConsumer();
 
         final listenerCompleter = Completer<int>();
-        final MyProtocol myProtocol = MyProtocol.implementAsBlocking(
+        final MyProtocol myProtocol = MyProtocol$Builder.implementAsBlocking(
           instanceMethod_withDouble_: (NSString s, double x) {
             throw UnimplementedError();
           },
@@ -352,7 +350,7 @@ void main() {
 
         final listenerCompleter = Completer<int>();
         final protocolBuilder = ObjCProtocolBuilder();
-        MyProtocol.addToBuilderAsBlocking(
+        MyProtocol$Builder.addToBuilderAsBlocking(
           protocolBuilder,
           instanceMethod_withDouble_: (NSString s, double x) {
             throw UnimplementedError();
@@ -365,7 +363,7 @@ void main() {
             ptr.value = 98765;
           },
         );
-        SecondaryProtocol.addToBuilder(
+        SecondaryProtocol$Builder.addToBuilder(
           protocolBuilder,
           otherMethod_b_c_d_: (int a, int b, int c, int d) {
             return a * b * c * d;
@@ -389,7 +387,7 @@ void main() {
         final consumer = ProtocolConsumer();
 
         final builder = ObjCProtocolBuilder();
-        MyProtocol.instanceMethod_withDouble_.implementWithBlock(
+        MyProtocol$Builder.instanceMethod_withDouble_.implementWithBlock(
           builder,
           ObjCBlock_NSString_ffiVoid_NSString_ffiDouble.fromFunction(
             (Pointer<Void> _, NSString s, double x) =>
@@ -440,18 +438,18 @@ void main() {
 
     test('Unused protocol', () {
       // Regression test for https://github.com/dart-lang/native/issues/1672.
-      final proto = UnusedProtocol.implement(someMethod: () => 123);
+      final proto = UnusedProtocol$Builder.implement(someMethod: () => 123);
       expect(proto, isNotNull);
     });
 
     test('Disabled method', () {
       // Regression test for https://github.com/dart-lang/native/issues/1702.
-      expect(MyProtocol.instanceMethod_withDouble_.isAvailable, isTrue);
-      expect(MyProtocol.optionalMethod_.isAvailable, isTrue);
-      expect(MyProtocol.disabledMethod.isAvailable, isFalse);
+      expect(MyProtocol$Builder.instanceMethod_withDouble_.isAvailable, isTrue);
+      expect(MyProtocol$Builder.optionalMethod_.isAvailable, isTrue);
+      expect(MyProtocol$Builder.disabledMethod.isAvailable, isFalse);
 
       expect(
-        () => MyProtocol.disabledMethod.implement(
+        () => MyProtocol$Builder.disabledMethod.implement(
           ObjCProtocolBuilder(),
           () => 123,
         ),
@@ -474,7 +472,9 @@ void main() {
       int count = 0;
 
       final protocolBuilder = ObjCProtocolBuilder();
-      MyProtocol.voidMethod_.implementAsListener(protocolBuilder, (int x) {
+      MyProtocol$Builder.voidMethod_.implementAsListener(protocolBuilder, (
+        int x,
+      ) {
         expect(x, 123);
         ++count;
         if (count == 1000) completer.complete();
@@ -497,7 +497,7 @@ void main() {
       final block = InstanceMethodBlock.fromFunction(
         (Pointer<Void> p, NSString s, double x) => 'Hello'.toNSString(),
       );
-      MyProtocol.instanceMethod_withDouble_.implementWithBlock(
+      MyProtocol$Builder.instanceMethod_withDouble_.implementWithBlock(
         protocolBuilder,
         block,
       );
@@ -655,7 +655,7 @@ void main() {
     test('adding more methods after build', () {
       final protocolBuilder = ObjCProtocolBuilder();
 
-      MyProtocol.addToBuilder(
+      MyProtocol$Builder.addToBuilder(
         protocolBuilder,
         instanceMethod_withDouble_: (NSString s, double x) {
           return 'ProtocolBuilder: ${s.toDartString()}: $x'.toNSString();
@@ -668,7 +668,7 @@ void main() {
       final protocolImpl = protocolBuilder.build();
 
       expect(
-        () => SecondaryProtocol.addToBuilder(
+        () => SecondaryProtocol$Builder.addToBuilder(
           protocolBuilder,
           otherMethod_b_c_d_: (int a, int b, int c, int d) {
             return a * b * c * d;
@@ -677,5 +677,16 @@ void main() {
         throwsA(isA<StateError>()),
       );
     });
+
+    test('calling methods on a protocol instance', () {
+      final protoImpl = ObjCProtocolImpl();
+
+      final MyProtocol myProto = protoImpl.returnsMyProtocol();
+      final result = myProto.instanceMethod(
+        "abc".toNSString(),
+        withDouble: 123,
+      );
+      expect(result.toDartString(), 'ObjCProtocolImpl: abc: 123.00');
+    });
   });
 }

pkgs/ffigen/test/native_objc_test/protocol_test.h

@@ -63,6 +63,9 @@ typedef struct {
 @optional
 - (nullable instancetype)returnsInstanceType;
 
+@optional
+- (id<MyProtocol>)returnsMyProtocol;
+
 @end
 
 @protocol EmptyProtocol

pkgs/ffigen/test/native_objc_test/protocol_test.m

@@ -83,6 +83,10 @@ + (int32_t)optionalClassMethod {
   return 5432;
 }
 
+- (id<MyProtocol>)returnsMyProtocol {
+  return self;
+}
+
 @end