streem / pbandk

Kotlin Code Generator and Runtime for Protocol Buffers
MIT License
265 stars 38 forks source link

Generated code requires `unknownFields` named param when called from Swift #225

Open darronschall opened 2 years ago

darronschall commented 2 years ago

I'm experimenting with a Kotlin/Native KMM project that uses pbandk to generate code from a .proto file. I'm using KMM to share the network integration layer between iOS and Android apps.

Currently, the generated code provides a default value for unknownFields allowing the parameter to be omitted from the constructor in Kotlin code. This does not bridge into Swift properly, forcing the call site to always specify a value for the parameter.

For example, this message in the .proto file:

// Generic request with just an ID
message IdRequest {
  string id = 1;
}

Will generate this data class via pbandk (0.14.1):

@pbandk.Export
public data class IdRequest(
    val id: String = "",
    override val unknownFields: Map<Int, pbandk.UnknownField> = emptyMap()
) : pbandk.Message {
// ...

When I use this class from Android, I'm able to properly omit unknownFields from the call site:

val request = IdRequest("example")

... but, when I call this from iOS via Swift, I have to explicitly pass the argument, otherwise I'm met with a Swift compiler error: Missing argument for parameter 'unknownFields' in call:

let request = IdRequest(id: "example", unknownFields: [:])

Ideally, the pbandk generated code would allow the Swift call-site to omit unknownFields as well.

darronschall commented 2 years ago

This issue is a result of Kotlin/Native's interop happening through Objective-C. Objective-C does not support default argument values, whereas Swift does. Kotlin/Native may eventually support Swift directly but work is not currently happening on it.

The way to solve this via Objective-C is to declare multiple methods that pass default values as necessary. We can achieve this result via Kotlin/Native by using a secondary constructor that provides the appropriate default value for unknownFields.

For the example above, the generator should produce the additional constructor line as follows:

@pbandk.Export
public data class IdRequest(
    val id: String = "",
    override val unknownFields: Map<Int, pbandk.UnknownField> = emptyMap()
) : pbandk.Message {
    constructor(id: String) : this(id, emptyMap<Int, pbandk.UnknownField>())
    // ...

This allows the Swift code to omit unknownFields from the call-site.

To be clear, this is a Kotlin/Native issue. But, for the time being, it would be nice if pbandk can provide the secondary constructor workaround to make Swift interop more pleasant.

garyp commented 2 years ago

Thanks for the investigation @darronschall! A couple thoughts on this:

  1. As part of https://github.com/streem/pbandk/pull/217, instantiation of proto objects will be done using builders instead of constructors. For example:

    // old code
    val request = IdRequest("example")
    // or
    val request = IdRequest(id = "example")
    
    // new code
    val request = IdRequest {
        id = "example"
    }

    Which will indirectly address this issue. Though I have not yet looked at how these new builder methods look in Swift. That pull request is still a work in progress with some less-common functionality not fully implemented, but you should be able to run the code from that PR if you want to preview the upcoming changes. It would be helpful to get feedback on how the new APIs function in both Kotlin and Swift.

  2. The primary goal for pbandk is to generate code with an idiomatic interface in Kotlin for use by multi-platform libraries written in Kotlin. The expectation is that the KMP libraries would use pbandk-generated code under the hood and expose a higher-level API to Swift and Javascript/Typescript.

    We do try to support idiomatic use directly from Swift, TypeScript, and Java on a best-effort basis. We want the code to be usable from those languages, even if it's more cumbersome or not idiomatic. We'll gladly take bug reports and pull requests to improve the usability of generated code from non-Kotlin languages. But given the current maturity of Kotlin/Native and Kotlin/JS and the gaps in both of those platforms today, it is too difficult to maintain support for a great experience from all of those languages. And we also just don't have enough people working on pbandk to be able to do it.

    I've been meaning to add this to the project README so that it's clearer what pbandk's goals and limitations are.