Typeclasses requiring only read access, and private constructors

softwaremill / magnolia

Easy, fast, transparent generic derivation of typeclass instances

https://softwaremill.com/open-source/

Apache License 2.0

768 stars 117 forks source link

Typeclasses requiring only read access, and private constructors #222

Closed jatcwang closed 4 years ago

jatcwang commented 4 years ago

Many typeclasses require only read access to fields (e.g.Show, diffx's Diff, JSON Encoders), but due to the existence of CaseClass#construct, magnolia derivation will fail if the primary constructor is not accessible.

Proposal:

When a library author writes def combine[T](ctx: CaseClass[Typeclass, T]): Derived[MyTypeClass[T]], allow them to use def combine[T](ctx: ReadOnlyCaseClass[Typeclass, T]): Derived[MyTypeClass[T]] instead, where ReadOnlyCaseClass will not have the construct methods.
Magnolia.gen will detect that ReadOnlyCaseClass is used and not try to resolve constructors

This applies to abstract as well, as currently sealed abstract case class is quite a common pattern in < 2.12 due to https://github.com/scala/bug/issues/7884 (Should be fixed in 2.13.2?)

WDYT?

joroKr21 commented 4 years ago

I think it makes sense, but I'm not sure if the encoding you are proposing has the best UX.

propensive commented 4 years ago

I'm not sure either, but I can't think of anything that's obviously better. The change shouldn't be too difficult, with this approach.

jatcwang commented 4 years ago

Adding a super class to CaseClass should be binary compatible (even if I move some methods like parameters to the parent ReadOnlyCaseClass).

joroKr21 commented 4 years ago

We haven't promised binary compatibility yet anyway.

jatcwang commented 4 years ago

As a downstream user I'd prefer to not deal with any MethodNotFoundExceptions ;) It will probably be ok.... :crossed_fingers:

propensive commented 4 years ago

I did bump the version number from 0.12.x to 0.13.x in the last (as-yet unannounced) release on the basis that it included a small API change, but in general I haven't been careful about it...

joroKr21 commented 4 years ago

Yeah if we really want to make sure, then we should cut a 1.0 release and add MiMa.

propensive commented 4 years ago

I'd like to leave that a while. I'm a bit reluctant to give a binary compatibility guarantee while I at least expect to change a few things... my priority right now is to make it easier to publish both Magnolia and ~20 other libraries...