Skip to content

Commit

Permalink
Implement the new proto3 field presence feature #34
Browse files Browse the repository at this point in the history
  • Loading branch information
@NiematojakTomasz
NiematojakTomasz committed Jan 20, 2022
1 parent 9c2d0c9 commit a4e6e74
Show file tree
Hide file tree
Showing 10 changed files with 3,115 additions and 53 deletions.
2,615 changes: 2,615 additions & 0 deletions jvm-test-types/src/main/java/pbandk/testpb/Proto3Presence.java
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion protoc-gen-pbandk/lib/src/commonMain/kotlin/pbandk/gen/CodeGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ public open class CodeGenerator(
}
}

protected val File.Field.Numbered.Standard.hasPresence: Boolean get() = (file.version == 2 && optional)
protected val File.Field.Numbered.Standard.hasPresence: Boolean get() = optional
protected fun File.Field.Numbered.Standard.mapEntry(): File.Type.Message? =
if (!map) null else (localType as? File.Type.Message)?.takeIf { it.mapEntry }

Expand Down
24 changes: 21 additions & 3 deletions protoc-gen-pbandk/lib/src/commonMain/kotlin/pbandk/gen/FileBuilder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,16 @@ internal open class FileBuilder(val namer: Namer = Namer.Standard, val supportMa
} + oneofFields.groupBy {
it.oneofIndex!!
}.mapNotNull { (oneofIndex, fields) ->
msgDesc.oneofDecl[oneofIndex].name?.let { oneofName ->
oneofFieldFromProto(ctx, oneofName, fields, usedFieldNames, usedTypeNames)
// "Every proto3 optional field is placed into a one-field oneof.
// We call this a "synthetic" oneof, as it was not present in the source .proto file."
// https://github.com/protocolbuffers/protobuf/blob/master/docs/implementing_proto3_presence.md#background
val synthetic = fields.size == 1 && (fields[0].proto3Optional ?: false)
if (synthetic) {
numberedFieldFromProto(ctx, fields[0], usedFieldNames)
} else {
msgDesc.oneofDecl[oneofIndex].name?.let { oneofName ->
oneofFieldFromProto(ctx, oneofName, fields, usedFieldNames, usedTypeNames)
}
}
}
}
Expand All @@ -131,6 +139,12 @@ internal open class FileBuilder(val namer: Namer = Namer.Standard, val supportMa
): File.Field.OneOf {
val fields = oneofFields.map {
// wrapper fields are not supposed to be used inside of oneof's
numberedFieldFromProto(
ctx,
it,
mutableSetOf(),
ctx.fileDesc.syntax.let { it == "proto2" || it == null}
) as File.Field.Numbered.Standard
numberedFieldFromProto(ctx, it, mutableSetOf(), true) as File.Field.Numbered.Standard
}
return File.Field.OneOf(
Expand Down Expand Up @@ -180,7 +194,9 @@ internal open class FileBuilder(val namer: Namer = Namer.Standard, val supportMa
localTypeName = fieldDesc.typeName,
repeated = fieldDesc.label == FieldDescriptorProto.Label.REPEATED,
jsonName = fieldDesc.jsonName,
optional = !alwaysRequired && fieldDesc.label == FieldDescriptorProto.Label.OPTIONAL,
optional = !alwaysRequired &&
((fieldDesc.label == FieldDescriptorProto.Label.OPTIONAL && ctx.fileDesc.usesProto2Syntax) ||
(fieldDesc.proto3Optional ?: false)),
packed = !type.neverPacked && (fieldDesc.options?.packed ?: (ctx.fileDesc.syntax == "proto3")),
map = supportMaps &&
fieldDesc.label == FieldDescriptorProto.Label.REPEATED &&
Expand Down Expand Up @@ -252,3 +268,5 @@ internal open class FileBuilder(val namer: Namer = Namer.Standard, val supportMa

companion object : FileBuilder()
}

private val FileDescriptorProto.usesProto2Syntax: Boolean get() = syntax == null || syntax == "proto2"
105 changes: 56 additions & 49 deletions protoc-gen-pbandk/lib/src/commonMain/kotlin/pbandk/gen/Main.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import pbandk.gen.pb.CodeGeneratorRequest
import pbandk.gen.pb.CodeGeneratorResponse

private var logDebug = false
private inline fun debug(fn: () -> String) { if (logDebug) Platform.stderrPrintln(fn()) }
private inline fun debug(fn: () -> String) {
if (logDebug) Platform.stderrPrintln(fn())
}

public fun main() {
Platform.stdoutWriteResponse(runGenerator(Platform.stdinReadRequest()))
Expand All @@ -25,61 +27,66 @@ internal fun runGenerator(request: CodeGeneratorRequest): CodeGeneratorResponse
// Convert to file model and generate the code only for ones requested
val kotlinTypeMappings = mutableMapOf<String, String>()

return CodeGeneratorResponse(file = request.protoFile.flatMap { protoFile ->
val packageName = protoFile.`package`
debug { "Reading ${protoFile.name}, package: $packageName" }
return CodeGeneratorResponse(
supportedFeatures = CodeGeneratorResponse.Feature.PROTO3_OPTIONAL.value.toLong(),
file = request.protoFile.flatMap { protoFile ->
val packageName = protoFile.`package`
debug { "Reading ${protoFile.name}, package: $packageName" }

val needToGenerate = request.fileToGenerate.contains(protoFile.name)
val needToGenerate = request.fileToGenerate.contains(protoFile.name)

// Convert the file to our model
val file = FileBuilder.buildFile(FileBuilder.Context(protoFile, params.let {
// As a special case, if we're not generating it but it's a well-known type package, change it our known one
if (needToGenerate || packageName != "google.protobuf") it
else it + ("kotlin_package" to "pbandk.wkt")
}))
// Convert the file to our model
val file = FileBuilder.buildFile(FileBuilder.Context(protoFile, params.let {
// As a special case, if we're not generating it but it's a well-known type package, change it our known one
if (needToGenerate || packageName != "google.protobuf") it
else it + ("kotlin_package" to "pbandk.wkt")
}))

// Update the type mappings
kotlinTypeMappings += file.kotlinTypeMappings()
// Update the type mappings
kotlinTypeMappings += file.kotlinTypeMappings()

// Generate if necessary
if (!needToGenerate) emptyList() else {
// Package name + file name (s/proto/kt)
val fileNameSansPath = protoFile.name!!.substringAfterLast('/')
val filePath = (file.kotlinPackageName?.replace('.', '/')?.plus('/') ?: "") +
fileNameSansPath.removeSuffix(".proto") + ".kt"
debug { "Generating $filePath" }
val code = CodeGenerator(file = file, kotlinTypeMappings = kotlinTypeMappings, params = params).generate()
// Generate if necessary
if (!needToGenerate) emptyList() else {
// Package name + file name (s/proto/kt)
val fileNameSansPath = protoFile.name!!.substringAfterLast('/')
val filePath = (file.kotlinPackageName?.replace('.', '/')?.plus('/') ?: "") +
fileNameSansPath.removeSuffix(".proto") + ".kt"
debug { "Generating $filePath" }
val code =
CodeGenerator(file = file, kotlinTypeMappings = kotlinTypeMappings, params = params).generate()

// Do service gen if generator present
var extraServiceCode = ""
val serviceFiles = if (serviceGen == null) emptyList() else protoFile.service.flatMap { protoService ->
val results = serviceGen.generate(ServiceGenerator.Service(
file = file,
filePath = filePath,
existingCode = code,
kotlinTypeMappings = kotlinTypeMappings,
raw = protoService,
rawRequest = request,
debugFn = ::debug
))
results.mapNotNull { result ->
// If the result is for this file, just append
if (result.otherFilePath == null) {
extraServiceCode += "\n" + result.code
null
} else CodeGeneratorResponse.File(
name = result.otherFilePath,
insertionPoint = result.otherFileInsertionPoint,
content = result.code
// Do service gen if generator present
var extraServiceCode = ""
val serviceFiles = if (serviceGen == null) emptyList() else protoFile.service.flatMap { protoService ->
val results = serviceGen.generate(
ServiceGenerator.Service(
file = file,
filePath = filePath,
existingCode = code,
kotlinTypeMappings = kotlinTypeMappings,
raw = protoService,
rawRequest = request,
debugFn = ::debug
)
)
results.mapNotNull { result ->
// If the result is for this file, just append
if (result.otherFilePath == null) {
extraServiceCode += "\n" + result.code
null
} else CodeGeneratorResponse.File(
name = result.otherFilePath,
insertionPoint = result.otherFileInsertionPoint,
content = result.code
)
}
}
}

val primaryFiles =
if (file.types.isEmpty() && extraServiceCode.isEmpty()) emptyList()
else listOf(CodeGeneratorResponse.File(name = filePath, content = code + extraServiceCode))
val primaryFiles =
if (file.types.isEmpty() && extraServiceCode.isEmpty()) emptyList()
else listOf(CodeGeneratorResponse.File(name = filePath, content = code + extraServiceCode))

primaryFiles + serviceFiles
}
})
primaryFiles + serviceFiles
}
})
}
18 changes: 18 additions & 0 deletions protoc-gen-pbandk/lib/src/jvmTest/kotlin/CodeGeneratorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import pbandk.wkt.FileDescriptorSet
import java.io.File
import kotlin.reflect.full.declaredMemberProperties
import kotlin.reflect.full.hasAnnotation
import kotlin.reflect.full.memberProperties
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class CodeGeneratorTest {
Expand Down Expand Up @@ -58,6 +60,22 @@ class CodeGeneratorTest {
valueClazz.classLoader.loadClass("foobar.Value\$Value")
}

@Test
fun testProto3Optional() {
val result = compileProto("proto_3_presence.proto")
assertEquals(ExitCode.OK, result.exitCode, result.messages)

val mainClazz = result.classLoader.loadClass("pbandk.testpb.Proto3PresenceMain").kotlin
assertTrue("optionalMessage should be nullable") { mainClazz.memberProperties.find { it.name == "optionalMessage" }!!.returnType.isMarkedNullable }
assertTrue("message should be nullable") { mainClazz.memberProperties.find { it.name == "message" }!!.returnType.isMarkedNullable }
assertTrue("optionalString should be nullable") { mainClazz.memberProperties.find { it.name == "optionalString" }!!.returnType.isMarkedNullable }
assertFalse("string should not be nullable") { mainClazz.memberProperties.find { it.name == "string" }!!.returnType.isMarkedNullable }
assertTrue("optionalInt should be nullable") { mainClazz.memberProperties.find { it.name == "optionalInt" }!!.returnType.isMarkedNullable }
assertFalse("int should not be nullable") { mainClazz.memberProperties.find { it.name == "int" }!!.returnType.isMarkedNullable }
assertTrue("optionalEnum should be nullable") { mainClazz.memberProperties.find { it.name == "optionalEnum" }!!.returnType.isMarkedNullable }
assertFalse("enum should not be nullable") { mainClazz.memberProperties.find { it.name == "enum" }!!.returnType.isMarkedNullable }
}

private fun compileProto(inputProto: String): KotlinCompilation.Result {
val gen = runGenerator(
CodeGeneratorRequest(
Expand Down
23 changes: 23 additions & 0 deletions protoc-gen-pbandk/lib/src/jvmTest/resources/protos/proto_3_presence.proto
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
syntax = "proto3";
package pbandk.testpb;

message Proto3PresenceMessage {}

enum Proto3PresenceEnum {
PROTO3_PRESENCE_ENUM_UNSPECIFIED = 0;
}

message Proto3PresenceMain {
optional Proto3PresenceMessage optional_message = 1;
Proto3PresenceMessage message = 2;
optional string optional_string = 3;
string string = 4;
optional int32 optional_int = 5;
int32 int = 6;
optional Proto3PresenceEnum optional_enum = 7;
Proto3PresenceEnum enum = 8;
oneof one_of {
string one_of_string = 9;
int32 one_of_int = 10;
}
}
Loading

0 comments on commit a4e6e74

Please sign in to comment.