Autogenerated swift Codable conformance

[Iskander508]

This is complementary to the CaseIterable PR: #2489

It seems that Codable could be safely autogenerated for all records, enums and errors that don't contain objects (recursively). I have tried it with various declarations, also globally with the whole test suite and didn't run into any errors. Although there might be cases where this would generate code that is either wrong (unable to compile) or undesirable (user wants to use some custom implementation instead). So I have also added a swift config flag generate_codable_conformance, with default value false (Codable conformance not generated)

• add unit-tests

[mhammond]

This seems reasonable to me. My biggest concern is this going stale - would it be possible to add a test for this? eg, the omit_argument_labels option has a test at fixtures/swift-omit-labels

[mhammond]

I guess another question I have is what happens when something can't be made codable? For example, if a record has an Arc<Object> I guess it will be unable to be Codable? And that will in-turn mean that none of the objects will be able to be Codable?

ie, I guess I'm asking whether the approach should be to explicitly name the objects you want to be Codable rather than a "global" boolean?

[Iskander508]

ie, I guess I'm asking whether the approach should be to explicitly name the objects you want to be Codable rather than a "global" boolean?

My thinking is that there are actually similar rules as to when Hashable and Equatable can be autogenerated (and those are generated based on the condition if !contains_object_references). So I just reused the same logic for Codable. Although here I might be wrong (but couldn't think of any such case).

If we implement it in a way that in the config the user has to fill all the various records, enums, ... it might be very cumbersome and hard to maintain. It could be hundreds of entries in bigger projects.

I'm thinking of 3 variants:

1. my proposed approach - just a simple flag in config and autogenerate only for those, we know are working. Meaning not containing objects (recursively). For the rest the user needs to implement the protocol manually in his own swift module.
2. list everything in config - Autogenerated conformance only for enums/records/errors/objects mentioned in config
3. a combined approach - to have both:

• simple config flag - to autogenerate for all simple cases
• additional list in config - to specify additional entries to also add conformance generation (on user's responsibility)

[mhammond]

My thinking is that there are actually similar rules as to when Hashable and Equatable can be autogenerated (and those are generated based on the condition if !contains_object_references).

Right, that's a solid argument tbh - I guess I can't see a reason to suggest Codable is going to be more problematic than those 2.

So yeah, I guess this makes sense and we can tweak it as we find it necessary to. You may have guessed by now that I don't really know much about Swift :) So cc @martinmose @Sajjon @crazytonyli, any thoughts here?

[Iskander508]

Thanks, this is great - I like that it's enabled by default rather than opt-in as it seems useful and something we should maybe do in #2490?

OK, I can change the config toggle to enabled-by-default. I was just more cautious and didn't want to break somebody's code ;)

[crazytonyli]

I don't think the configuration should be on by default, simply because most of the configurations are not on by default.

I wonder if there are some subtle implications when both the Swift binding and the underlying Rust type can ser/de into JSON. Would you expect them to produce and parse the same JSON content? For example, should a JSON produced by Codable be parsable by serde_json? I don't think this PR can guarantee that.

First, the serde attributes on the Rust type are invisible to the Swift binding type. For example, it's impossible for Swift's Codable implementation to know how serde's implementation on the Rust type works.

Even if we ignore serde attributes, for a rust type like struct Foo { bar_baz: String }, the JSON schema is {"bar_baz": "..."}, by default. The Rust type's Swift binding code would be like struct Foo { var barBaz: String }, whose JSON schema is {"barBaz": "..."}.

[mhammond]

I don't think the configuration should be on by default, simply because most of the configurations are not on by default.

That's an interesting discussion. I was leaning towards things which cause a default behaviour change should not be on by default, but things which allow new capabilities, where otherwise you'd get a compiler error, are safer to enable by default. So in this scenario and in #2490, ISTM that the new capability would only be used when code tried to pass these objects as a Codable/CaseIterable, which today would just fail - but nothing would change in any other scenario.

ie, I see this as more like the Equatable etc support - there's no real need to force people to opt in because you can effectively opt-out simply by not trying to test these objects for equality - and if you never actually do that, the support for Equatable should be invisible (and ideally even omitted from the final binary, assuming the swift compiler/linker is smart enough to do that)

Would you expect them to produce and parse the same JSON content?

I don't think it's necessary for us to have an opinion about this. IIUC, this capability probably make the most sense when serde is not involved on the Rust side, but even if it is available on both sides and the objects would serialize differently in that case, I think that's beyond the scope of what we are trying to do here. Do you disagree, or where you just commenting on the fact they may be different?

[crazytonyli]

Do you disagree, or where you just commenting on the fact they may be different?

I just wanted to highlight the fact that this configuration may cause the Rust types and their Swift bindings to handle serialization & deserialization differently. I wouldn't say I disagree with introducing this configuration. It's a configuration after all, we as users can choose to turn it on or off.

BTW, I think CaseIterable is quite different from Equatable, Hashable, and Codable. CaseIterable is just syntax sugar, and does not introduce changes to runtime. However, when you let Swift to synthesize Equatable, Hashable, and Codable, then there is the risk that Swift bindings have different implementation than their underlying Rust types. #[uniffi::export(Eq, Hash)] mitigates that risk, which is great. It'd be nice if there is one for Codable too.

[mhammond]

However, when you let Swift to synthesize Equatable, Hashable, and Codable, then there is the risk that Swift bindings have different implementation than their underlying Rust types. #[uniffi::export(Eq, Hash)] mitigates that risk, which is great. It'd be nice if there is one for Codable too.

That's only true for objects, right? For records, we do implement Equatable and Hashable, which may indeed have a different implementation than the struct would on the Rust side. Personally I think I'm OK with that to be honest - the whole point of data classes is that they are completely separate from the Rust implementation - which (I think :) is also why I'm mildly against adding methods to records/enums as that would makes everything more fuzzy and less clear.

That said though, if we do agree implementing Equatable and Hashable on data classes without an explicit opt-in was a mistake, the earlier we decide and mitigate that the better, so thanks for the discussion and I welcome all other thoughts here.

[Iskander508]

OK, so from the discussion it seems everybody is now tending towards not producing the conformance code by default. To me it also seems to be more preferred. For users that don't need Codable conformance there will be no surprise when they upgrade to a newer version of uniffi. For users that would like to use it, I guess they will spend some time to skim through documentation and will figure out that they have to switch a toggle in the config. At least I did that myself first.

Regarding the differences in serialization between rust and swift sides, I honestly didn't think about that before you mentioned that. Obviously it can (and most likely will) produce different results. So we can simply mention that in the documentation to the config toggle? Something like: Be aware that serialization in Swift may lead to different and incompatible results compared to serialization in Rust.

[mhammond]

OK, so from the discussion it seems everybody is now tending towards not producing the conformance code by default.

Sounds great.

Something like: Be aware that serialization in Swift may lead to different ...

That's true for eq etc too, right? So maybe a more general note that reinforces these are entirely "foreign" (or from the other POV, "local") concepts and these are examples. Happy to roll these into 1 PR if that's easier for you too, thanks!

[Iskander508]

Happy to roll these into 1 PR if that's easier for you too, thanks!

Feel free to make any adjustments or close my PRs and open another.