zio / zio-json

Fast, secure JSON library with tight ZIO integration.
https://zio.dev/zio-json
Apache License 2.0
407 stars 146 forks source link

Feat: scala3 enumeration support #1068

Closed ThijsBroersen closed 3 months ago

ThijsBroersen commented 7 months ago

With this PR I have tried to implement Scala 3 enumeration support. Because the current macros already do some assumptions and are build with Magnolia I did not want to disrupt to much and tried to implement it under the following condition: If all subtypes are named values (objects without parameters) then the type is an enumeration.

This means that if an enum has mixed children, with and without parameters, then the implementations stays the same. Only for pure enumerations (all named values) the encoding/decoding will be affected.

Breaking: To my opinion the current implementation is wrong as enumerations are normally serialized as strings. But this PR will of course be breaking for the few using zio-json with Scala 3 enumerations already.

I deliberately did not try to implement this for Scala 2 as these are not native enumerations.

ThijsBroersen commented 7 months ago

Interesting, so it seems shared specs are now failing because in Scala 3 the sealed traits are now also treated as enumerations. 🤔

ThijsBroersen commented 7 months ago

Interesting, so it seems shared specs are now failing because in Scala 3 the sealed traits are now also treated as enumerations. 🤔

Fixed 🙃

fsvehla commented 6 months ago

It’d say let’s get it in, the next version will be a new 0.x. Do you have a few cycles to add some docs?

ThijsBroersen commented 6 months ago

@fsvehla I was just trying to update the docs but this docs modules is Scala 2.13 only. Can Scala 3 be mixed in?

ThijsBroersen commented 5 months ago

@fsvehla I did one more attempt:

The tests now fail with the GoldenSpec which assumes sealed families with only object members should be encoded as objects... I don't agree with the spec as many libraries, including Tapir, encode this as string-based enum. When using Tapir this would lead to an endpoint schema which renders an openapi spec saying the type is a string-based enum and a json codec which behaves differently.

What is your view on this?

fsvehla commented 3 months ago

@ThijsBroersen I agree that the proposed change makes sense for most cases (and takes care to exclude enums containing case classes). Fine to remove that test!

ThijsBroersen commented 3 months ago

Changed the golden-test SumType.Case3 from case object Case3 to case class Case3(). Now it is derived as non-enum and the test works as expected. So this PR will change sealed traits / enums where all cases are without constructors, these are now derived as string-based enums.

ThijsBroersen commented 3 months ago

Lets merge!

guersam commented 2 weeks ago

It breaks my production code where the sealed trait is explicit annotated by @jsonDiscriminator.

@jsonDiscriminator("type")
enum ResponseFormat derives JsonCodec:

  @jsonHint("text") 
  case Text

  @jsonHint("json_object")
  case JsonObject

Expected encoding

{ "type": "json_object" }

Actual encoding

"json_object"