Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
darronschall opened this issue Aug 7, 2022 · 2 comments

Comments

@darronschall
Copy link

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
Copy link
Author

darronschall commented Aug 10, 2022

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
Copy link
Collaborator

garyp commented Aug 10, 2022

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

  1. As part of Add support for builders using mutable messages #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants