search

sbt / contraband

http://www.scala-sbt.org/contraband/
Other
68 stars 21 forks source link

Generate Shapeless Generic instances #109

Closed mberndt123 closed 10 months ago

mberndt123 commented 6 years ago

The Generic (and LabelledGeneric) typeclass in shapeless implements the concepts discussed here in the Scala language. For case classes, there is a macro in Shapeless that generates Generic instances, but of course it doesn't work for contraband's pseudo case classes. Having this would be strictly more general than the JSON stuff that contraband already supports (as JSON serializers can be generated with shapeless based on LabelledGeneric), so I think it would be a useful addition and make contraband a more complete replacement for case classes. I'd be willing to help out with the implementation, but I'd like to get some feedback on the idea before I start writing code. What do you think?

eed3si9n commented 6 years ago

sjsonnew.JsonFormat is a typeclass that's backend independent, so it's potentially possible to provide LabelledGeneric as a backend of sjson-new without modifying Contraband. If having an instance for LabelledGeneric is useful without going through sjson-new, I'd be open to let Contraband optionally generate instances in a separate file (similar to how it does sjson-new instances).

mberndt123 commented 6 years ago

LabelledGeneric is completely independent of any JSON format library. It's similar to what you do in sjson-new with the IsoLList stuff, but IMO a bit better because it doesn't carry the labels around at runtime, so it should be slightly more efficient.

There's also a bunch of libraries that allow you to do stuff with any type that has a LabelledGeneric instance (e. g. diffs between objects), and I don't think that users should be required to also depend on a JSON library to get that. Also, I believe that the best practice is to put typeclass instances into the companion object rather than a separate file. What do you think? Perhaps we can make it configurable?

mberndt123 commented 6 years ago

I've thought a bit more about this. It's a bit tricky to keep the ABI stable. LabelledGeneric.Aux[A, Repr] defines an isomorphism between some type A and a type Repr, where the latter would be some HList type, or a Coproduct type for an enum. Now, as the definition of A changes over time, so would Repr – when you add a new field for the class, you would need a new member in the HList as well. Afaics the only fix for that would be to generate multiple LabelledGeneric instances, one for each version of the schema, where only the latest is declared implicit in order to avoid ambiguous implicits. This would lead to an interesting situation where a new version of the library, while binary compatible, wouldn't necessarily be source compatible. What do you think about that?

mberndt123 commented 6 years ago

@eed3si9n , can you give some feedback?

eed3si9n commented 6 years ago

There's also a bunch of libraries that allow you to do stuff with any type that has a LabelledGeneric instance (e. g. diffs between objects), and I don't think that users should be required to also depend on a JSON library to get that.

I guess there's a pros and cons. Since I use Contraband in conjunction with sjson-new, it would be most convenient if JsonFormat[A] that I've set up works automatically for LabelledGeneric. I can also understand if people just wants to use Contraband without sjson-new.

Also, I believe that the best practice is to put typeclass instances into the companion object rather than a separate file. What do you think?

It's debatable if using the implicit scope is best practice or not. My personal preference is to push these opt-in instances into trait. It has the benefit of being able to keep the pseudo case class free or serialization/generics concerns, and for me the fact that all instances require explicit import xyzprotocols._ is a plus, because I don't always want the same JsonFormat for a given type.

I've thought a bit more about this. It's a bit tricky to keep the ABI stable.

Contraband + sjson-new combo is concerned about binary compatibility, but I think it's ok for other generated code to have different stability, as long as they don't bleed to the other parts and we are up front about it.