marcoferrer / kroto-plus

gRPC Kotlin Coroutines, Protobuf DSL, Scripting for Protoc
Apache License 2.0
493 stars 28 forks source link

Add additional utility overloads for repeated fields in a proto class #5

Open lifk opened 5 years ago

lifk commented 5 years ago

I've been using a lot the generated builders with nested objects and that creates really nice to read structures but I think we can improve them even more adding a couple of overloads to the methods that handle repeated fields.

For example given a proto message like:

message Item {
uint32 id = 1;
string value = 2;
}

message ItemList {
repeated Item item = 1;
}

by default we have available addAllItem(items: Iterable<Item>) and addItem(item: Item).

What I suggest to add if the following overloads. addAllItem(vararg items: Item) --> this one would avoid to call listOf to add items that are being defined manually addItem(block: Item.Builder.() -> Unit) --> this one would be to avoid the redundant information that you get while reading addItem(Item { id = 1, value = "test"})

Let me know if you think this could be added to the base of Kroto+ or if we should make it into a custom script.

marcoferrer commented 5 years ago

So I think the vararg ext is a valid use case and probably something that could be added to the default generator. As for the nested builder, from 0.1.3 and up, all fields of type message have a nested builder automatically generated. This includes repeated fields. If they aren't then it might be a bug that I would be glad to take a look at. You can see the nested builder usage in this file from the example project.

lifk commented 5 years ago

I didn't saw the nested builder from 0.1.3. Probably I checked it before updating. Then let's consider this issue only for the vararg one as I think it can also be useful.

marcoferrer commented 5 years ago

In the mean time you can use this script to generate the exts until the next release.

Config

generator_scripts {
    script_path: "VarArgExtensionGenerator.kts"
}

Script

import com.github.marcoferrer.krotoplus.generators.Generator
import com.github.marcoferrer.krotoplus.proto.ProtoEnum
import com.github.marcoferrer.krotoplus.proto.ProtoMessage
import com.github.marcoferrer.krotoplus.utils.addFunctions
import com.github.marcoferrer.krotoplus.utils.memoize
import com.google.protobuf.DescriptorProtos
import com.google.protobuf.DescriptorProtos.FieldDescriptorProto.Label.LABEL_REPEATED
import com.google.protobuf.compiler.PluginProtos
import com.squareup.kotlinpoet.*

object VarArgExtensionGenerator : Generator {

    val basePackage = "com.my.output.package"
    val outputFilename = "VarArgBuilderExts"

    override fun invoke(): PluginProtos.CodeGeneratorResponse {

        val statementTemplate = "return this.%N(values.toList())"

        val funSpecs = context.schema.protoTypes.values.asSequence()
            .filterIsInstance<ProtoMessage>()
            .associate {
                it to it.descriptorProto.fieldList.filter { field -> field.label == LABEL_REPEATED }
            }
            .filterValues { it.isNotEmpty() }
            .map { (protoMessage, repeatedFields) ->

                repeatedFields.map { field ->
                    val fieldNameCamelCase = camelCaseFieldName(field.name)

                    FunSpec.builder("addAll$fieldNameCamelCase")
                        .addStatement(statementTemplate, "addAll$fieldNameCamelCase")
                        .receiver(protoMessage.builderClassName)
                        .addParameter("values", field.javaClassName, KModifier.VARARG)
                        .build()
                }
            }
            .flatten()

        return PluginProtos.CodeGeneratorResponse.newBuilder()
            .addFile(FileSpec
                .builder(basePackage, outputFilename)
                .addFunctions(funSpecs).build()
                .toResponseFileProto()
            )
            .build()
    }

    private val DescriptorProtos.FieldDescriptorProto.javaClassName: ClassName
        get() = when (this@javaClassName.type!!) {
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_INT64,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_FIXED64,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_SFIXED64,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_SINT64,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_UINT64 -> Long::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_INT32,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_UINT32,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_SFIXED32,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_SINT32,
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_FIXED32 -> Int::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_DOUBLE -> Double::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_FLOAT -> Float::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_BOOL -> Boolean::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_STRING -> String::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_GROUP -> TODO()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_BYTES -> ByteString::class.asClassName()
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_MESSAGE ->
                (context.schema.protoTypes[this@javaClassName.typeName] as ProtoMessage).className
            DescriptorProtos.FieldDescriptorProto.Type.TYPE_ENUM ->
                (context.schema.protoTypes[this@javaClassName.typeName] as ProtoEnum).className
        }

    val camelCaseFieldName = { it: String ->
        // We cant use CaseFormat.UPPER_CAMEL since
        // protoc is lenient with malformed field names
        if (it.contains("_"))
            it.split("_").joinToString(separator = "") { it.capitalize() } else
            it.capitalize()

    }.memoize()
}
marcoferrer commented 5 years ago

Another use case I wanted to document here. Adding vararg extensions for map fields. It would follow the familiar kotlin convention for building maps.

TestMessage {
    putAllExampleField(
        "string-key1" to value,
        "string-key2" to value
    )
}