diff --git a/build.sbt b/build.sbt index f6cf3e3ac..2baf52182 100644 --- a/build.sbt +++ b/build.sbt @@ -3,9 +3,6 @@ import com.typesafe.tools.mima.core._ import BuildHelper._ import Dependencies._ -// Different version for compiler-plugin since >=3.8.0 is not binary -// compatible with 3.7.x. When loaded inside SBT (which has its own old -// version), the binary incompatibility surfaces. val protobufCompilerVersion = "3.12.2" val MimaPreviousVersion = "0.10.0" @@ -345,6 +342,8 @@ lazy val e2eNoJava = (project in file("e2e-nojava")) .settings(e2eCommonSettings) .settings( Compile / PB.protocVersion := "-v" + versions.protobuf, + Compile / PB.protocOptions += "--experimental_allow_proto3_optional", + Compile / PB.generate := ((Compile / PB.generate) dependsOn (protocGenScalaUnix / Compile / assembly)).value, Compile / PB.targets := Seq( ( PB.gens.plugin( diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala b/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala index e0fa492bc..088ec8c52 100644 --- a/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala +++ b/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala @@ -58,7 +58,7 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { message <- file.allMessages if message.isSealedOneofType } yield SealedOneof( message, - message.getOneofs.get(0).getFields.asScala.map(_.getMessageType).toVector + message.getRealOneofs.get(0).getFields.asScala.map(_.getMessageType).toVector ) new SealedOneofsCache(sealedOneof) } @@ -185,7 +185,7 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { implicit class FieldDescriptorPimp(val fd: FieldDescriptor) { import NameUtils._ - def containingOneOf: Option[OneofDescriptor] = Option(fd.getContainingOneof) + def containingOneOf: Option[OneofDescriptor] = Option(fd.getRealContainingOneof()) def isInOneof: Boolean = containingOneOf.isDefined @@ -230,7 +230,7 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { def oneOfTypeName: ScalaName = { assert(isInOneof) - fd.getContainingOneof.scalaType / upperScalaName + fd.getRealContainingOneof.scalaType / upperScalaName } def noBox = @@ -241,12 +241,16 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { // Is this field boxed inside an Option in Scala. Equivalent, does the Java API // support hasX methods for this field. def supportsPresence: Boolean = - fd.isOptional && !fd.isInOneof && (!fd.getFile.isProto3 || fd.isMessage) && + fd.isOptional && !fd.isInOneof && (!fd.getFile.isProto3 || fd.isMessage || fd + .toProto() + .getProto3Optional()) && !noBox && !fd.isSealedOneofType // Is the Scala representation of this field a singular type. def isSingular = - fd.isRequired || (fd.getFile.isProto3 && !fd.isInOneof && fd.isOptional && !fd.isMessage) || ( + fd.isRequired || (fd.getFile.isProto3 && !fd.isInOneof && fd.isOptional && !fd.isMessage && !fd + .toProto() + .getProto3Optional()) || ( fd.isOptional && (noBox || (fd.isSealedOneofType && !fd.isInOneof)) ) @@ -431,6 +435,7 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { } implicit class OneofDescriptorPimp(val oneof: OneofDescriptor) { + def javaEnumName = { val name = NameUtils.snakeCaseToCamelCase(oneof.getName, true) s"get${name}Case" @@ -476,8 +481,9 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { def sealedOneofStyle: SealedOneofStyle = { assert(isSealedOneofType) - if (message.getOneofs.asScala.exists(_.getName == "sealed_value")) SealedOneofStyle.Default - else if (message.getOneofs.asScala.exists(_.getName == "sealed_value_optional")) + if (message.getRealOneofs.asScala.exists(_.getName == "sealed_value")) + SealedOneofStyle.Default + else if (message.getRealOneofs.asScala.exists(_.getName == "sealed_value_optional")) SealedOneofStyle.Optional else throw new RuntimeException("Unexpected oneof style") } @@ -485,7 +491,7 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) { // every message that passes this filter must be a sealed oneof. The check that it actually // obeys the rules is done in ProtoValidation. def isSealedOneofType: Boolean = { - message.getOneofs.asScala + message.getRealOneofs.asScala .exists(o => o.getName == "sealed_value" || o.getName == "sealed_value_optional") } diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/ProtoValidation.scala b/compiler-plugin/src/main/scala/scalapb/compiler/ProtoValidation.scala index 78a0b2d5e..aae04597e 100644 --- a/compiler-plugin/src/main/scala/scalapb/compiler/ProtoValidation.scala +++ b/compiler-plugin/src/main/scala/scalapb/compiler/ProtoValidation.scala @@ -49,7 +49,7 @@ class ProtoValidation(implicits: DescriptorImplicits) { m.getNestedTypes.asScala.foreach(validateMessage) m.getFields.asScala.foreach(validateField) if (m.isSealedOneofType) { - val oneof = m.getOneofs.get(0) + val oneof = m.getRealOneofs.get(0) if (m.getContainingType != null) { throw new GeneratorException(s"${m.getFullName}: sealed oneofs must be top-level messages") } diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala index d48d5e504..88fedaf79 100644 --- a/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala +++ b/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala @@ -689,7 +689,7 @@ class ProtobufGenerator( annotations = annotations(field) ) } - val oneOfFields = message.getOneofs.asScala.map { oneOf => + val oneOfFields = message.getRealOneofs.asScala.map { oneOf => val ctorDefaultValue: Option[String] = if (message.getFile.noDefaultValuesInConstructor) None else Some(oneOf.empty.fullNameWithMaybeRoot(message)) @@ -751,7 +751,7 @@ class ProtobufGenerator( fp.add(s"var __requiredFields$index: _root_.scala.Long = $bn") } } - .print(message.getOneofs.asScala)((printer, oneof) => + .print(message.getRealOneofs.asScala)((printer, oneof) => printer.add(s"var __${oneof.scalaName.name} = `_message__`.${oneof.scalaName.nameSymbol}") ) .add(s"""var _done__ = false @@ -846,7 +846,7 @@ class ProtobufGenerator( .add(s"$myFullScalaName(") .indent .addWithDelimiter(",")( - (message.fieldsWithoutOneofs ++ message.getOneofs.asScala).map { + (message.fieldsWithoutOneofs ++ message.getRealOneofs.asScala).map { case e: FieldDescriptor if e.isRepeated => s" ${e.scalaName.asSymbol} = __${e.scalaName}.result()" case e: FieldDescriptor => @@ -895,7 +895,7 @@ class ProtobufGenerator( else javaFieldToScala("javaPbSource", field) Seq(s"${field.scalaName.asSymbol} = $conversion") } - val oneOfs = message.getOneofs.asScala.map { + val oneOfs = message.getRealOneofs.asScala.map { case oneOf => val head = s"${oneOf.scalaName.nameSymbol} = javaPbSource.${oneOf.javaEnumName}.getNumber match {" @@ -976,7 +976,7 @@ class ProtobufGenerator( enclosingType = field.enclosingType ) } - val oneOfs = message.getOneofs.asScala.map { oneOf => + val oneOfs = message.getRealOneofs.asScala.map { oneOf => val elems = oneOf.fields.map { field => val value = s"__fieldsMap.get(scalaDescriptor.findFieldByNumber(${field.getNumber}).get)" @@ -1022,7 +1022,7 @@ class ProtobufGenerator( case field if !field.isInOneof => val default = defaultValueForDefaultInstance(field) s"${field.scalaName.asSymbol} = $default" - } ++ message.getOneofs.asScala.map { oneof => + } ++ message.getRealOneofs.asScala.map { oneof => s"${oneof.scalaName.nameSymbol} = ${oneof.empty.fullName}" }) .outdent @@ -1062,7 +1062,7 @@ class ProtobufGenerator( ) } } - .print(message.getOneofs.asScala) { + .print(message.getRealOneofs.asScala) { case (printer, oneof) => val oneofName = oneof.scalaName.nameSymbol printer @@ -1353,7 +1353,7 @@ class ProtobufGenerator( .call(generateEnumCompanionForField(message)) .call(generateDefaultInstance(message)) .print(message.getEnumTypes.asScala)(printEnum) - .print(message.getOneofs.asScala)(printOneof) + .print(message.getRealOneofs.asScala)(printOneof) .print(message.nestedTypes)(printMessage) .print(message.getExtensions.asScala)(printExtension) .when(message.generateLenses)(generateMessageLens(message)) @@ -1457,7 +1457,7 @@ class ProtobufGenerator( ) } } - .print(message.getOneofs.asScala) { + .print(message.getRealOneofs.asScala) { case (printer, oneof) => printer.add( s"""def clear${oneof.scalaType.name}: ${message.scalaType.nameSymbol} = copy(${oneof.scalaName.nameSymbol} = ${oneof.empty @@ -1736,22 +1736,17 @@ object ProtobufGenerator { def handleCodeGeneratorRequest(request: CodeGenRequest): CodeGenResponse = { parseParameters(request.parameter) match { case Right(params) => - try { - val implicits = new DescriptorImplicits(params, request.allProtos) - val generator = new ProtobufGenerator(params, implicits) - val validator = new ProtoValidation(implicits) - validator.validateFiles(request.allProtos) - import implicits.FileDescriptorPimp - val files = request.filesToGenerate.flatMap { file => - if (file.scalaOptions.getSingleFile) - generator.generateSingleScalaFileForFileDescriptor(file) - else generator.generateMultipleScalaFilesForFileDescriptor(file) - } - CodeGenResponse.succeed(files) - } catch { - case e: GeneratorException => - CodeGenResponse.fail(e.message) + val implicits = new DescriptorImplicits(params, request.allProtos) + val generator = new ProtobufGenerator(params, implicits) + val validator = new ProtoValidation(implicits) + validator.validateFiles(request.allProtos) + import implicits.FileDescriptorPimp + val files = request.filesToGenerate.flatMap { file => + if (file.scalaOptions.getSingleFile) + generator.generateSingleScalaFileForFileDescriptor(file) + else generator.generateMultipleScalaFilesForFileDescriptor(file) } + CodeGenResponse.succeed(files, Set(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL)) case Left(error) => CodeGenResponse.fail(error) } diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala index 07fdcfca3..2d59f0b6b 100644 --- a/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala +++ b/compiler-plugin/src/main/scala/scalapb/compiler/SealedOneofsGenerator.scala @@ -21,7 +21,7 @@ class SealedOneofsGenerator(message: Descriptor, implicits: DescriptorImplicits) val sealedOneofType = message.sealedOneofScalaType val sealedOneofName = message.sealedOneofTraitScalaType.nameSymbol val typeMapper = s"_root_.scalapb.TypeMapper[${baseType}, ${sealedOneofType}]" - val oneof = message.getOneofs.get(0) + val oneof = message.getRealOneofs.get(0) val typeMapperName = message.sealedOneofTypeMapper.name if (message.sealedOneofStyle != SealedOneofStyle.Optional) { diff --git a/e2e-nojava/src/main/protobuf/proto3_presence.proto b/e2e-nojava/src/main/protobuf/proto3_presence.proto new file mode 100644 index 000000000..3286f5f0c --- /dev/null +++ b/e2e-nojava/src/main/protobuf/proto3_presence.proto @@ -0,0 +1,14 @@ +syntax = "proto3"; + +package com.thesamet.proto.e2e; + +enum PEnum { + PENUM_UNKNOWN = 0; + PENUM_A = 1; +} + +message Presence { + optional int32 i32 = 1; + optional PEnum penum = 2; + optional Presence presence = 3; +} diff --git a/e2e/src/main/protobuf/google/protobuf/unittest.proto b/e2e/src/main/protobuf/google/protobuf/unittest.proto index 1582ed7f0..c8c1e414b 100644 --- a/e2e/src/main/protobuf/google/protobuf/unittest.proto +++ b/e2e/src/main/protobuf/google/protobuf/unittest.proto @@ -834,3 +834,6 @@ service TestService { message BarRequest {} message BarResponse {} +message TestMap { + map int32_to_string = 1; +} \ No newline at end of file diff --git a/project/Dependencies.scala b/project/Dependencies.scala index e04f14042..04bc8ba76 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -6,14 +6,14 @@ import Keys._ object Dependencies { object versions { val grpc = "1.30.2" - val protobuf = "3.11.4" + val protobuf = "3.12.2" val utest = "0.7.4" val fastparse = "2.3.0" val silencer = "1.6.0" val collectionCompat = "2.1.6" val coursier = "2.0.0-RC6-21" val mockito = "3.2.0" - val protocGen = "0.9.0-RC2" + val protocGen = "0.9.0-RC3" val protocJar = "3.11.4" val protobufRuntimeScala = "0.8.6" val commonsCodec = "1.14" diff --git a/proptest/src/test/scala/GenTypes.scala b/proptest/src/test/scala/GenTypes.scala index 59474929c..6265a4087 100644 --- a/proptest/src/test/scala/GenTypes.scala +++ b/proptest/src/test/scala/GenTypes.scala @@ -124,7 +124,7 @@ object GenTypes { val REPEATED = Value("repeated") } - case class FieldOptions(modifier: FieldModifier.Value, isPacked: Boolean) + case class FieldOptions(modifier: FieldModifier.Value, isPacked: Boolean, proto3Presence: Boolean) def genFieldModifier(allowRequired: Boolean): Gen[FieldModifier.Value] = if (allowRequired) @@ -188,19 +188,28 @@ object GenTypes { protoSyntax: ProtoSyntax, inOneof: Boolean ): Gen[FieldOptions] = - if (inOneof) Gen.const(FieldOptions(FieldModifier.OPTIONAL, isPacked = false)) + if (inOneof) + Gen.const(FieldOptions(FieldModifier.OPTIONAL, isPacked = false, proto3Presence = false)) else fieldType match { case MessageReference(id) => - genFieldModifier(allowRequired = protoSyntax.isProto2 && id < messageId) - .map(mod => FieldOptions(mod, isPacked = false)) - case MapType(_, _) => Gen.const(FieldOptions(FieldModifier.REPEATED, isPacked = false)) + for { + mod <- genFieldModifier(allowRequired = protoSyntax.isProto2 && id < messageId) + proto3Presence <- if (mod == FieldModifier.OPTIONAL && protoSyntax.isProto3) + Gen.oneOf(true, false) + else Gen.const(false) + } yield FieldOptions(mod, isPacked = false, proto3Presence = proto3Presence) + case MapType(_, _) => + Gen.const(FieldOptions(FieldModifier.REPEATED, isPacked = false, proto3Presence = false)) case _ => for { mod <- genFieldModifier(allowRequired = protoSyntax.isProto2) packed <- if (fieldType.packable && mod == FieldModifier.REPEATED) Gen.oneOf(true, false) else Gen.const(false) - } yield FieldOptions(mod, isPacked = packed) + proto3Presence <- if (mod == FieldModifier.OPTIONAL && protoSyntax.isProto3) + Gen.oneOf(true, false) + else Gen.const(false) + } yield FieldOptions(mod, isPacked = packed, proto3Presence = proto3Presence) } } diff --git a/proptest/src/test/scala/GraphGen.scala b/proptest/src/test/scala/GraphGen.scala index 2d936cb98..f81c83108 100644 --- a/proptest/src/test/scala/GraphGen.scala +++ b/proptest/src/test/scala/GraphGen.scala @@ -285,7 +285,7 @@ object GraphGen { if (protoPackage.isEmpty) state else state.closeNamespace ) - def genRootNode: Gen[RootNode] = + def genRootNode: Gen[RootNode] = { listWithStatefulGen(State(), maxSize = 10)(genFileNode) .map { case (files, state) => @@ -293,4 +293,6 @@ object GraphGen { RootNode(files) } .suchThat(_.maxMessageId.isDefined) + + } } diff --git a/proptest/src/test/scala/Nodes.scala b/proptest/src/test/scala/Nodes.scala index 4d47b4b2b..73b0d607e 100644 --- a/proptest/src/test/scala/Nodes.scala +++ b/proptest/src/test/scala/Nodes.scala @@ -334,7 +334,8 @@ object Nodes { assert( fieldOptions.modifier == FieldModifier.OPTIONAL || fieldOptions.modifier == FieldModifier.REPEATED ) - if (fieldOptions.modifier == FieldModifier.OPTIONAL || fieldType.isMap) "" + if (fieldOptions.proto3Presence) "optional " + else if (fieldOptions.modifier == FieldModifier.OPTIONAL || fieldType.isMap) "" else if (fieldOptions.modifier == FieldModifier.REPEATED) "repeated " else throw new RuntimeException("Unexpected modifier") } diff --git a/proptest/src/test/scala/SchemaGenerators.scala b/proptest/src/test/scala/SchemaGenerators.scala index 9dadd3d6d..52420a5c1 100644 --- a/proptest/src/test/scala/SchemaGenerators.scala +++ b/proptest/src/test/scala/SchemaGenerators.scala @@ -175,6 +175,7 @@ object SchemaGenerators { file.getAbsolutePath } val args = Seq( + "--experimental_allow_proto3_optional", "--proto_path", (tmpDir.toString + ":protobuf:third_party"), "--java_out",