WIP Add support for transforming constructor names. #114
Conversation
modules/annotations/shared/src/main/scala/io/circe/derivation/annotations/Configuration.scala
Show resolved
Hide resolved
| @@ -10,6 +10,7 @@ class JsonCodec( | |||
| def macroTransform(annottees: Any*): Any = macro GenericJsonCodecMacros.jsonCodecAnnotationMacro | |||
| } | |||
|
|
|||
| // FIXME: Should these annotations also handle constructor transformations? | |||
There was a problem hiding this comment.
Biggest open question feature-wise, I think
| case ..$discriminatorCases | ||
| new _root_.io.circe.Decoder[$tpe] { | ||
| // NOTE: This wrapper might look pointless, but leaving it out causes compilation to fail with | ||
| // an assertion error from the compiler. There's probably a better way to avoid the problem. |
There was a problem hiding this comment.
This is the problem I mentioned in the ticket. I noticed that the compiler crash only happened in the Decoder case, not the Codec. The only difference I could see was that the Codec explicitly created a wrapper class around the generated case statements, so I tried doing that and it worked. This is my first time digging into Scala macros so I have no idea why the other way wouldn't work.
| @@ -10,6 +10,7 @@ class JsonCodec( | |||
| def macroTransform(annottees: Any*): Any = macro GenericJsonCodecMacros.jsonCodecAnnotationMacro | |||
| } | |||
|
|
|||
| // FIXME: Should these annotations also handle constructor transformations? | |||
modules/annotations/shared/src/main/scala/io/circe/derivation/annotations/Configuration.scala
Show resolved
Hide resolved
| @@ -592,24 +602,27 @@ class DerivationMacros(val c: blackbox.Context) extends ScalaVersionCompat { | |||
|
|
|||
| // we materialize trait | |||
| private[this] def materializeEncoderTraitImpl[T: c.WeakTypeTag]( | |||
| transformMemberNames: Option[c.Expr[String => String]], | |||
| transformConstructorNames: Option[c.Expr[String => String]], | |||
There was a problem hiding this comment.
Good catch that we didn't need the old argument here.
|
|
||
| cq"""$value => c.get($value)(_root_.io.circe.Decoder[${s.asType}]).asInstanceOf[_root_.io.circe.Decoder.Result[$tpe]]""" | ||
| cq"""nme if nme == $value => c.get(nme)(_root_.io.circe.Decoder[${s.asType}]).asInstanceOf[_root_.io.circe.Decoder.Result[$tpe]]""" |
There was a problem hiding this comment.
Out of curiosity, was this change necessary?
There was a problem hiding this comment.
It was (as far as I could tell). Matching directly on $value caused compilation errors about function 1 not having an unapply method when transformConstructorNames wasn't None. The generated code looked like:
case (((x: String) => someFunc(x))("AdtBar")) => ???
case (((x: String) => someFunc(x))("AdtFoo")) => ???Now they look like:
case nme if nme == (((x: String) => someFunc(x))("AdtBar")) => ???
case nme if nme == (((x: String) => someFunc(x))("AdtFoo")) => ???Related question I was wondering about: is there any way to "lift" the transformation function into the macro to make it directly useable, so we can pre-compute the transformed strings?
modules/derivation/shared/src/main/scala/io/circe/derivation/DerivationMacros.scala
Show resolved
Hide resolved
|
Any tips for debugging the JS errors? The problem only seems to be in |
|
@danxmoran Does this branch have the 0.6.29 update? If so you might try rolling back to 0.6.28. It's a compiler crash, so I suspect it's not our fault. I can take a closer look later. |
|
@travisbrown I hit the crash on both 0.6.28 and 0.6.29 |
|
These commits are included in #186. Thanks again, @danxmoran! |
(Eventually) resolves #110
Still WIP because there are some open questions, but the code that's there is ready for a look.