scalapb/ScalaPB

Help making pull request: Ability to add `derives` clause

Closed this issue · 6 comments

oscar-broman commented

I might be able to create a pull request from this, but I'd need some guidance!

This is a local diff which achieves what I'm after (circe codecs for all types):

$ git diff
diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala
index a484b7c2..7bf3d0ba 100644
--- a/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala
+++ b/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala
@@ -1353,7 +1353,7 @@ class ProtobufGenerator(
       .indent
       .indent
       .call(printConstructorFieldList(message))
-      .add(s") extends ${message.baseClasses.mkString(" with ")} {")
+      .add(s") extends ${message.baseClasses.mkString(" with ")} derives io.circe.Codec.AsObject {")
       .call(generateSerializedSizeForPackedFields(message))
       .call(generateSerializedSize(message))
       .call(generateWriteTo(message))
diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala
index 98217311..b3ff3dee 100644
--- a/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala
+++ b/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala
@@ -43,7 +43,7 @@ class SealedOneofsGenerator(message: Descriptor, implicits: DescriptorImplicits)
         val sealedOneofUniversalMark = if (message.isUniversalTrait) "Any with " else ""

         fp.add(
-          s"sealed trait $sealedOneofName $bases{"
+          s"sealed trait $sealedOneofName $bases derives io.circe.Codec.AsObject {"
         ).addIndented(
           s"type MessageType = $baseType",
           s"final def isEmpty = this.isInstanceOf[${sealedOneofType}.Empty.type]",
@@ -56,7 +56,7 @@ class SealedOneofsGenerator(message: Descriptor, implicits: DescriptorImplicits)
           .indented(
             _.add(s"case object Empty extends $sealedOneofType", "")
               .add(
-                s"sealed trait $sealedOneofNonEmptyName extends $sealedOneofUniversalMark$sealedOneofType"
+                s"sealed trait $sealedOneofNonEmptyName extends $sealedOneofUniversalMark$sealedOneofType derives io.circe.Codec.AsObject"
               )
               .add(
                 s"def defaultInstance: ${sealedOneofType} = Empty",

How do I add a setting for this that could be specified both globally and per message?

thesamet commented

Hi Oscar, thanks for suggesting this feature and contributing the PR. I had started to work on this in parallel, looked at your code and made adjustments in mine as necessary. One difference is that I refrained from introducing target_regex in aux_message_options but expanded target to accept * to indicate to apply to all messages in scope. Will this work for you?

I'd really appreciate before I cut a release if you will be able to try out the snapshot release 0.11.13+88-1d78ac52-SNAPSHOT and see if it works for? Some documentation is at https://github.com/scalapb/ScalaPB/blob/master/CHANGELOG.md#01114 , and

oscar-broman commented

Hi, thanks!

While I was unable to try the snapshot you published due to scalapb-runtime_3 missing for that version, I published it locally and tried it out.

There's one thing that's preventing me from not importing io.circe.generic.auto.* and that is the derives clause for GeneratedSealedOneof. Currently, only NonEmpty has the clause (and rightfully so!), but it's also needed for the top-level type.

However, I changed all sealed_value messages into sealed_value_optional and while it did increase boilerplate a bit, I got rid of some rather complex type-class derivation logic.

In the end, I'm left with the following diff required to make things work:

diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala
index cbfce320..2f65388d 100644
--- a/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala
+++ b/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala
@@ -31,7 +31,7 @@ class SealedOneofsGenerator(message: Descriptor, implicits: DescriptorImplicits)
       val bases =
         if (baseClasses.nonEmpty)
           s"extends ${baseClasses.mkString(" with ")} $derives"
-        else ""
+        else s"$derives"
 
       if (message.sealedOneofStyle != SealedOneofStyle.Optional) {
         val sealedOneofNonEmptyName = message.sealedOneofNonEmptyScalaType.nameSymbol
oscar-broman commented

The result is amazing! 90% decrease in incremental compilation speed in many cases, and 50% reduction in clean compile.

thesamet commented

The previous commit has the diff you posted. Can you try again? I'm not sure why you were not able to use scalapb_runtime-3. Tthe new version is published on sonatype, but also the previous version did. Are your resolvers configured correctly for the project/

oscar-broman commented

Correct! I had the Sonatype snapshots configured only for the build project, and not for the project that used ScalaPB.

It works now.

thesamet commented

Perfect. Will close the issue now and cut a release soon. Don't hesitate to start a new issue or comment here if anything left unresolved.

  • 🎉1