-
Notifications
You must be signed in to change notification settings - Fork 147
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
Feat: scala3 enumeration support #1068
Feat: scala3 enumeration support #1068
Conversation
@@ -490,16 +515,18 @@ object DeriveJsonDecoder extends Derivation[JsonDecoder] { self => | |||
} | |||
} | |||
|
|||
private lazy val caseObjectEncoder = new JsonEncoder[Any] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This static encoder make it possible to filter on at L625
@@ -595,13 +622,43 @@ object DeriveJsonEncoder extends Derivation[JsonEncoder] { self => | |||
} | |||
|
|||
def split[A](ctx: SealedTrait[JsonEncoder, A]): JsonEncoder[A] = { | |||
val isEnumeration = ctx.subtypes.forall(_.typeclass == caseObjectEncoder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose to use this hack because I don't think I have access to the ctx.params
of the subtypes here anymore.
Interesting, so it seems shared specs are now failing because in Scala 3 the sealed traits are now also treated as enumerations. 🤔 |
Fixed 🙃 |
It’d say let’s get it in, the next version will be a new |
1187ebf
to
eeb0df0
Compare
@fsvehla I was just trying to update the docs but this docs modules is Scala 2.13 only. Can Scala 3 be mixed in? |
@fsvehla I did one more attempt:
The tests now fail with the What is your view on this? |
@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! |
Changed the golden-test |
12ee766
to
7425dd3
Compare
Lets merge! |
7425dd3
to
7d26898
Compare
It breaks my production code where the sealed trait is explicit annotated by @jsonDiscriminator("type")
enum ResponseFormat derives JsonCodec:
@jsonHint("text")
case Text
@jsonHint("json_object")
case JsonObject Expected encoding{ "type": "json_object" } Actual encoding"json_object" |
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.