feat: removing private from typemapper (#1674)

* feat: removing private from typemapper * feat: public_constructor_parameters flag
scalapb · Apr 4, 2024 · d93a990 · d93a990
1 parent b8a7cd1
commit d93a990
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 66 deletions.
diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala b/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala
@@ -933,6 +933,8 @@ class DescriptorImplicits private[compiler] (
 
     def emitScala3Sources: Boolean = scalaOptions.getScala3Sources || params.scala3Sources
 
+    def generatePublicConstructorParameters: Boolean = scalaOptions.getPublicConstructorParameters
+
     def V: ScalaCompatConstants = new ScalaCompatConstants(emitScala3Sources)
 
     def javaPackageAsSymbol: String =

diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala
@@ -19,6 +19,7 @@ class ProtobufGenerator(
     params: GeneratorParams,
     implicits: DescriptorImplicits
 ) {
+
   import implicits._
   import DescriptorImplicits.AsSymbolExtension
   import ProtobufGenerator._
@@ -157,6 +158,7 @@ class ProtobufGenerator(
   def defaultValueForGet(field: FieldDescriptor, uncustomized: Boolean = false) = {
     // Needs to be 'def' and not val since for some of the cases it's invalid to call it.
     def defaultValue = field.getDefaultValue
+
     val baseDefaultValue: String = field.getJavaType match {
       case FieldDescriptor.JavaType.INT  => defaultValue.toString
       case FieldDescriptor.JavaType.LONG => defaultValue.toString + "L"
@@ -257,6 +259,7 @@ class ProtobufGenerator(
     // TODO(thesamet): if both unit conversions are NoOp, we can omit the map call.
     def unitConversion(n: String, field: FieldDescriptor) =
       javaToScalaConversion(field).apply(n, EnclosingType.None)
+
     val upperJavaName =
       if (
         field.mapType.valueField.isEnum && !field.mapType.valueField.legacyEnumFieldTreatedAsClosed
@@ -957,7 +960,8 @@ class ProtobufGenerator(
   }
 
   def generateMessageLens(message: Descriptor)(printer: FunctionalPrinter): FunctionalPrinter = {
-    val className           = message.scalaType.name
+    val className = message.scalaType.name
+
     def lensType(s: String) = s"_root_.scalapb.lenses.Lens[UpperPB, $s]"
 
     printer
@@ -1011,7 +1015,8 @@ class ProtobufGenerator(
   }
 
   def generateTypeMappers(
-      fields: Seq[FieldDescriptor]
+      fields: Seq[FieldDescriptor],
+      generatePublicConstructorParameters: Boolean
   )(printer: FunctionalPrinter): FunctionalPrinter = {
     val customizedFields: Seq[(FieldDescriptor, String)] = for {
       field  <- fields
@@ -1020,13 +1025,15 @@ class ProtobufGenerator(
 
     printer
       .print(customizedFields) { case (printer, (field, customType)) =>
-        val modifier =
-          if (field.getFile().scalaPackage.fullName.isEmpty) "private"
-          else s"private[${field.getFile().scalaPackage.fullName.split('.').last}]"
+        val modifier = {
+          if (generatePublicConstructorParameters) ""
+          else if (field.getFile().scalaPackage.fullName.isEmpty) "private "
+          else s"private[${field.getFile().scalaPackage.fullName.split('.').last}] "
+        }
         printer
           .add("@transient")
           .add(
-            s"$modifier val ${field.typeMapperValName}: _root_.scalapb.TypeMapper[${field.baseSingleScalaTypeName}, ${customType}] = implicitly[_root_.scalapb.TypeMapper[${field.baseSingleScalaTypeName}, ${customType}]]"
+            s"${modifier}val ${field.typeMapperValName}: _root_.scalapb.TypeMapper[${field.baseSingleScalaTypeName}, ${customType}] = implicitly[_root_.scalapb.TypeMapper[${field.baseSingleScalaTypeName}, ${customType}]]"
           )
       }
   }
@@ -1314,7 +1321,12 @@ class ProtobufGenerator(
       .print(message.getExtensions.asScala)(printExtension)
       .when(message.generateLenses)(generateMessageLens(message))
       .call(generateFieldNumbers(message))
-      .call(generateTypeMappers(message.fields ++ message.getExtensions.asScala))
+      .call(
+        generateTypeMappers(
+          message.fields ++ message.getExtensions.asScala,
+          message.getFile.generatePublicConstructorParameters
+        )
+      )
       .call(generateCollectionAdapters(message.fields ++ message.getExtensions.asScala))
       .when(message.isMapEntry)(generateTypeMappersForMapEntry(message))
       .call(generateNoDefaultArgsFactory(message))
@@ -1650,7 +1662,12 @@ class ProtobufGenerator(
       .call(generateMessagesCompanions(file)(_))
       .call(generateFileDescriptor(file)(_))
       .print(file.getExtensions.asScala)(printExtension(_, _))
-      .call(generateTypeMappers(file.getExtensions.asScala.toSeq))
+      .call(
+        generateTypeMappers(
+          file.getExtensions.asScala.toSeq,
+          file.generatePublicConstructorParameters
+        )
+      )
       .outdent
       .add("}")
   }

diff --git a/docs/src/main/markdown/customizations.md b/docs/src/main/markdown/customizations.md
@@ -57,6 +57,7 @@ option (scalapb.options) = {
   enum_strip_prefix: false
   bytes_type: "scodec.bits.ByteVector"
   scala3_sources: false
+  public_constructor_parameters: false
 };
 ```
 
@@ -134,6 +135,10 @@ enums to a single Scala file.
 
 - By default, ScalaPB generates Scala sources that are compatible with both Scala 2 and Scala 3. To generate sources that can be compiled error-free with `-source feature` on Scala 3 or with `-Xsource:3` on Scala 2.13, set `scala3_sources` to `true` or pass the `scala3_sources` generator parameter.
 
+- Use `public_constructor_parameters` to make constructor parameters public, including defaults and TypeMappers. 
+  This is helpful for automated schema derivation with e.g. `magnolia` when trying to also derive default fields by 
+  using the compiler flag `-Yretain-trees`. Without this flag, the companion object's `_typemapper_*` fields are private.
+
 ## Package-scoped options
 
 Note: this option is available in ScalaPB 0.8.2 and later.

diff --git a/e2e/src/main/protobuf/issue1664.proto b/e2e/src/main/protobuf/issue1664.proto
@@ -0,0 +1,29 @@
+syntax = "proto3";
+
+package com.thesamet.proto.e2e;
+
+import "scalapb/scalapb.proto";
+
+option (scalapb.options) = {
+  public_constructor_parameters: true,
+  preserve_unknown_fields: false,
+  single_file: true,
+  import: "com.thesamet.proto.e2e.TypeMappers.enumMapper",
+  aux_field_options : [
+    {
+      target: "com.thesamet.proto.e2e.NestedExampleEvent.action"
+      options: {
+        type: "String"
+      }}
+  ],
+};
+
+message NestedExampleEvent {
+  string id = 1;
+  Action action = 2;
+  enum Action {
+    Undefined = 0;
+    Allow = 1;
+    Deny = 2;
+  }
+}
diff --git a/e2e/src/main/scala/com/thesamet/proto/e2e/TypeMappers.scala b/e2e/src/main/scala/com/thesamet/proto/e2e/TypeMappers.scala
@@ -0,0 +1,7 @@
+package com.thesamet.proto.e2e
+
+import scalapb.{GeneratedEnumCompanion, TypeMapper}
+
+object TypeMappers {
+  implicit def enumMapper[A <: scalapb.GeneratedEnum](implicit ec: GeneratedEnumCompanion[A]): TypeMapper[A, String] = TypeMapper[A, String](_.name)(ec.fromName(_).get)
+}
diff --git a/e2e/src/test/scala/Issue1644Spec.scala b/e2e/src/test/scala/Issue1644Spec.scala
@@ -0,0 +1,12 @@
+import org.scalatest.funspec.AnyFunSpec
+import org.scalatest.matchers.must.Matchers
+import com.thesamet.proto.e2e.issue1664.NestedExampleEvent
+
+class Issue1664Spec extends AnyFunSpec with Matchers {
+
+  it("issue1664:public_constructor_parameters") {
+    // Without public_constructor_parameters, this is inaccessible
+    NestedExampleEvent._typemapper_action.toBase("Allow") mustBe NestedExampleEvent.Action.Allow
+  }
+
+}
diff --git a/protobuf/scalapb/scalapb.proto b/protobuf/scalapb/scalapb.proto
@@ -184,6 +184,9 @@ message ScalaPbOptions {
   // Generate sources that are compatible with Scala 3
   optional bool scala3_sources = 28;
 
+  // Makes constructor parameters public, including defaults and TypeMappers.
+  optional bool public_constructor_parameters = 29;
+
   // For use in tests only. Inhibit Java conversions even when when generator parameters
   // request for it.
   optional bool test_only_no_java_conversions = 999;

diff --git a/scalapb-runtime/src/main/scala/scalapb/options/ScalaPbOptions.scala b/scalapb-runtime/src/main/scala/scalapb/options/ScalaPbOptions.scala
@@ -79,6 +79,8 @@ package scalapb.options
   *   If true, getters will be generated.
   * @param scala3Sources
   *   Generate sources that are compatible with Scala 3
+  * @param publicConstructorParameters
+  *   Makes constructor parameters public, including defaults and TypeMappers.
   * @param testOnlyNoJavaConversions
   *   For use in tests only. Inhibit Java conversions even when when generator parameters
   *   request for it.
@@ -113,6 +115,7 @@ final case class ScalaPbOptions(
     ignoreAllTransformations: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None,
     getters: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None,
     scala3Sources: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None,
+    publicConstructorParameters: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None,
     testOnlyNoJavaConversions: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None,
     unknownFields: _root_.scalapb.UnknownFieldSet = _root_.scalapb.UnknownFieldSet.empty
     ) extends scalapb.GeneratedMessage with scalapb.lenses.Updatable[ScalaPbOptions] with _root_.scalapb.ExtendableMessage[ScalaPbOptions] {
@@ -232,6 +235,10 @@ final case class ScalaPbOptions(
         val __value = scala3Sources.get
         __size += _root_.com.google.protobuf.CodedOutputStream.computeBoolSize(28, __value)
       };
+      if (publicConstructorParameters.isDefined) {
+        val __value = publicConstructorParameters.get
+        __size += _root_.com.google.protobuf.CodedOutputStream.computeBoolSize(29, __value)
+      };
       if (testOnlyNoJavaConversions.isDefined) {
         val __value = testOnlyNoJavaConversions.get
         __size += _root_.com.google.protobuf.CodedOutputStream.computeBoolSize(999, __value)
@@ -371,6 +378,10 @@ final case class ScalaPbOptions(
         val __m = __v
         _output__.writeBool(28, __m)
       };
+      publicConstructorParameters.foreach { __v =>
+        val __m = __v
+        _output__.writeBool(29, __m)
+      };
       testOnlyNoJavaConversions.foreach { __v =>
         val __m = __v
         _output__.writeBool(999, __m)
@@ -469,6 +480,9 @@ final case class ScalaPbOptions(
     def getScala3Sources: _root_.scala.Boolean = scala3Sources.getOrElse(false)
     def clearScala3Sources: ScalaPbOptions = copy(scala3Sources = _root_.scala.None)
     def withScala3Sources(__v: _root_.scala.Boolean): ScalaPbOptions = copy(scala3Sources = Option(__v))
+    def getPublicConstructorParameters: _root_.scala.Boolean = publicConstructorParameters.getOrElse(false)
+    def clearPublicConstructorParameters: ScalaPbOptions = copy(publicConstructorParameters = _root_.scala.None)
+    def withPublicConstructorParameters(__v: _root_.scala.Boolean): ScalaPbOptions = copy(publicConstructorParameters = Option(__v))
     def getTestOnlyNoJavaConversions: _root_.scala.Boolean = testOnlyNoJavaConversions.getOrElse(false)
     def clearTestOnlyNoJavaConversions: ScalaPbOptions = copy(testOnlyNoJavaConversions = _root_.scala.None)
     def withTestOnlyNoJavaConversions(__v: _root_.scala.Boolean): ScalaPbOptions = copy(testOnlyNoJavaConversions = Option(__v))
@@ -504,6 +518,7 @@ final case class ScalaPbOptions(
         case 26 => ignoreAllTransformations.orNull
         case 27 => getters.orNull
         case 28 => scala3Sources.orNull
+        case 29 => publicConstructorParameters.orNull
         case 999 => testOnlyNoJavaConversions.orNull
       }
     }
@@ -538,6 +553,7 @@ final case class ScalaPbOptions(
         case 26 => ignoreAllTransformations.map(_root_.scalapb.descriptors.PBoolean(_)).getOrElse(_root_.scalapb.descriptors.PEmpty)
         case 27 => getters.map(_root_.scalapb.descriptors.PBoolean(_)).getOrElse(_root_.scalapb.descriptors.PEmpty)
         case 28 => scala3Sources.map(_root_.scalapb.descriptors.PBoolean(_)).getOrElse(_root_.scalapb.descriptors.PEmpty)
+        case 29 => publicConstructorParameters.map(_root_.scalapb.descriptors.PBoolean(_)).getOrElse(_root_.scalapb.descriptors.PEmpty)
         case 999 => testOnlyNoJavaConversions.map(_root_.scalapb.descriptors.PBoolean(_)).getOrElse(_root_.scalapb.descriptors.PEmpty)
       }
     }
@@ -577,6 +593,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
     var __ignoreAllTransformations: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None
     var __getters: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None
     var __scala3Sources: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None
+    var __publicConstructorParameters: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None
     var __testOnlyNoJavaConversions: _root_.scala.Option[_root_.scala.Boolean] = _root_.scala.None
     var `_unknownFields__`: _root_.scalapb.UnknownFieldSet.Builder = null
     var _done__ = false
@@ -640,6 +657,8 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
           __getters = _root_.scala.Option(_input__.readBool())
         case 224 =>
           __scala3Sources = _root_.scala.Option(_input__.readBool())
+        case 232 =>
+          __publicConstructorParameters = _root_.scala.Option(_input__.readBool())
         case 7992 =>
           __testOnlyNoJavaConversions = _root_.scala.Option(_input__.readBool())
         case tag =>
@@ -678,6 +697,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
         ignoreAllTransformations = __ignoreAllTransformations,
         getters = __getters,
         scala3Sources = __scala3Sources,
+        publicConstructorParameters = __publicConstructorParameters,
         testOnlyNoJavaConversions = __testOnlyNoJavaConversions,
         unknownFields = if (_unknownFields__ == null) _root_.scalapb.UnknownFieldSet.empty else _unknownFields__.result()
     )
@@ -714,6 +734,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
         ignoreAllTransformations = __fieldsMap.get(scalaDescriptor.findFieldByNumber(26).get).flatMap(_.as[_root_.scala.Option[_root_.scala.Boolean]]),
         getters = __fieldsMap.get(scalaDescriptor.findFieldByNumber(27).get).flatMap(_.as[_root_.scala.Option[_root_.scala.Boolean]]),
         scala3Sources = __fieldsMap.get(scalaDescriptor.findFieldByNumber(28).get).flatMap(_.as[_root_.scala.Option[_root_.scala.Boolean]]),
+        publicConstructorParameters = __fieldsMap.get(scalaDescriptor.findFieldByNumber(29).get).flatMap(_.as[_root_.scala.Option[_root_.scala.Boolean]]),
         testOnlyNoJavaConversions = __fieldsMap.get(scalaDescriptor.findFieldByNumber(999).get).flatMap(_.as[_root_.scala.Option[_root_.scala.Boolean]])
       )
     case _ => throw new RuntimeException("Expected PMessage")
@@ -773,6 +794,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
     ignoreAllTransformations = _root_.scala.None,
     getters = _root_.scala.None,
     scala3Sources = _root_.scala.None,
+    publicConstructorParameters = _root_.scala.None,
     testOnlyNoJavaConversions = _root_.scala.None
   )
   /** Whether to apply the options only to this file, or for the entire package (and its subpackages)
@@ -1512,6 +1534,8 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
     def optionalGetters: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Option[_root_.scala.Boolean]] = field(_.getters)((c_, f_) => c_.copy(getters = f_))
     def scala3Sources: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Boolean] = field(_.getScala3Sources)((c_, f_) => c_.copy(scala3Sources = _root_.scala.Option(f_)))
     def optionalScala3Sources: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Option[_root_.scala.Boolean]] = field(_.scala3Sources)((c_, f_) => c_.copy(scala3Sources = f_))
+    def publicConstructorParameters: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Boolean] = field(_.getPublicConstructorParameters)((c_, f_) => c_.copy(publicConstructorParameters = _root_.scala.Option(f_)))
+    def optionalPublicConstructorParameters: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Option[_root_.scala.Boolean]] = field(_.publicConstructorParameters)((c_, f_) => c_.copy(publicConstructorParameters = f_))
     def testOnlyNoJavaConversions: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Boolean] = field(_.getTestOnlyNoJavaConversions)((c_, f_) => c_.copy(testOnlyNoJavaConversions = _root_.scala.Option(f_)))
     def optionalTestOnlyNoJavaConversions: _root_.scalapb.lenses.Lens[UpperPB, _root_.scala.Option[_root_.scala.Boolean]] = field(_.testOnlyNoJavaConversions)((c_, f_) => c_.copy(testOnlyNoJavaConversions = f_))
   }
@@ -1543,6 +1567,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
   final val IGNORE_ALL_TRANSFORMATIONS_FIELD_NUMBER = 26
   final val GETTERS_FIELD_NUMBER = 27
   final val SCALA3_SOURCES_FIELD_NUMBER = 28
+  final val PUBLIC_CONSTRUCTOR_PARAMETERS_FIELD_NUMBER = 29
   final val TEST_ONLY_NO_JAVA_CONVERSIONS_FIELD_NUMBER = 999
   def of(
     packageName: _root_.scala.Option[_root_.scala.Predef.String],
@@ -1573,6 +1598,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
     ignoreAllTransformations: _root_.scala.Option[_root_.scala.Boolean],
     getters: _root_.scala.Option[_root_.scala.Boolean],
     scala3Sources: _root_.scala.Option[_root_.scala.Boolean],
+    publicConstructorParameters: _root_.scala.Option[_root_.scala.Boolean],
     testOnlyNoJavaConversions: _root_.scala.Option[_root_.scala.Boolean]
   ): _root_.scalapb.options.ScalaPbOptions = _root_.scalapb.options.ScalaPbOptions(
     packageName,
@@ -1603,6 +1629,7 @@ object ScalaPbOptions extends scalapb.GeneratedMessageCompanion[scalapb.options.
     ignoreAllTransformations,
     getters,
     scala3Sources,
+    publicConstructorParameters,
     testOnlyNoJavaConversions
   )
   // @@protoc_insertion_point(GeneratedMessageCompanion[scalapb.ScalaPbOptions])