circe icon indicating copy to clipboard operation
circe copied to clipboard

Support semiauto derivation without recursive derivation

Open mrdziuban opened this issue 2 years ago • 6 comments

In scala 2, semiauto derivation requires that any nested case classes also have instances defined explicitly, while in scala 3 derivation automatically recurses to perform derivation for the nested case classes (the Mirror.Of branch here), e.g.

case class Foo()
case class Bar(foo: Foo)
// scala 2 - doesn't compile because there's no Decoder[Foo]
io.circe.generic.semiauto.deriveDecoder[Bar]
// scala 3 - compiles
io.circe.Decoder.derived[Bar]

I understand this is convenient in many cases, but I feel like there are a number of reasons it's less then ideal -- for example, I may define a Decoder[Foo] with custom behavior, and then someone later on without context could delete it, see that all code still compiles, and assume that everything is fine when it's not.

Is this behavior by design? If so, would you at least consider alternate derivation methods that skip the recursion?

mrdziuban avatar Apr 18 '23 16:04 mrdziuban

Gentle nudge to @zarthross and @zmccoy, would you consider a change in this behavior? I would be happy to work on it if so

mrdziuban avatar Jun 29 '23 17:06 mrdziuban

This is a very nice idea! I would like to know why the #2173 PR didn't work. Seems like both branchs of the if are being executed, and that is surprising to me.

Lasering avatar Oct 23 '23 13:10 Lasering

@Lasering it didn't work because it would require users to define their Configuration values as inline givens, and would silently do the wrong thing if they didn't -- see my comment here: https://github.com/circe/circe/pull/2173#issuecomment-1658900145

I opened another PR, #2183, that would also resolve this

mrdziuban avatar Oct 23 '23 13:10 mrdziuban

Unfortunately I was not aware of this detail when I implemented #1800. My goal was to have the typeclass derivation in Scala 3 to be a drop-in replacement of the generic (and generic extras) modules. Or in other words by default the typeclass derivation should not recurse, and then the auto derivation (see #2206) would do so. To make things worse, fixing this properly implies a new major version (which I am of the opinion that should be done).

Also have a look at https://github.com/circe/circe/issues/2111#issuecomment-1775592042 for some context.

@zmccoy @zarthross @armanbilge

Lasering avatar Oct 23 '23 14:10 Lasering

@Lasering I think it should be possible to change the default semantics to NO auto-derivation by default in a way thats binary compatible, but certainly semantically breaking. We can just announce the change, and while it may break source, it should be possible to be bin-safe.

Is there something that I'm missing there?

zarthross avatar Oct 25 '23 22:10 zarthross

In my head this semantic breaking change is big enough to merit a major version (I guess a 0.15.x). Also if you look at https://github.com/circe/circe/issues/1965 the proper fix would be to move the derived method from Encoder.AsObject to Encoder, and from Codec.AsObject to Codec which also feels like a breaking change to me.

In terms of binary compatibility/MiMa I have very little experience/feel for it. I guess sometimes the are ways of doing things that don't break the binary compatibility, but some times these ways just make things more confusing both for users (because they now see multiple ways of doing things) and for maintainers (because they need to keep track of what is the proper way of doing things and what is part of the bin-safe compatibility shim)

Lasering avatar Oct 26 '23 09:10 Lasering