Support generic types

[ZacSweers]

This could be done by accepting a KotlinPoet/JavaPoet TypeName as a parameter rather than just a raw classname. Another overload could be a reified fun <reified T> buildConfigField(name: String, value: T) for simple types

[gmazzo]

Let me explore a little bit the reified function. Seems elegant :)

[gmazzo]

Can you add some use cases for this? Seems a bit complex to support it:

• Accept a TypeName will imply exposing the internals of the code generator, and we have two: javapoet and kotlinpoet. I can't just decide to expose 1 of them.
• the reified feels right, but introduces a new problem. How do I "convert" the value into a raw "source code"? I may hardcode some strategies for primitive types, but it won't accept any kind of values.

[gmazzo]

Now you can use Provider API from Gradle, but sill has to be a String at the end. It's mapped to a "%L" literal template from CodeBlock.

Not exactly what you wanted but still you can build an extension function around it (I think, I didn't explore this at the end)

[gmazzo]

I think we can draft something like this:

    companion object {
        
        inline fun <reified T> BuildConfigClassSpec.buildConfigField(name: String, value: T) =
            buildConfigField(T::class.java.name, name, value)


    }

But still the problem is how to represent T as a String or better as a CodeBlock, and we do have two versions of them: one from KotlinPoet and the one from JavaPoet.

I think I'm going to drop this. It seems to add too much overhead to the API and the use case seems not to be so relevant to me.
Feel free to provide any further input 💪

[Doomsdayrs]

This does not seem to be a massive overhead. Infact this is much better compared to the current way of declaring the variables one by one.

[gmazzo]

This does not seem to be a massive overhead. Infact this is much better compared to the current way of declaring the variables one by one.

Can you elaborate what you have in mind?

[ZacSweers]

Since KotlinPoet is exposed in the plugin API, what about just exposing an overload that accepts a TypeName directly?

[gmazzo]

Since KotlinPoet is exposed in the plugin API, what about just exposing an overload that accepts a TypeName directly?

Let me give it a try, but it will have to be compatible with Configuration Cache.

I may focus on re-desing the DSL, accepting literals or an expression wrapper.

[ZacSweers]

I don't see why accepting a TypeName would be incompatible with configuration cache?

[ZacSweers]

An alternative would just be to accept a literal. The only reason this seems to currently behave this was is because it tries to convert to a ClassName in the task. It's convenient to be able to do so, but also seems not strictly necessary.

[ZacSweers]

Or a third option - add an extra enum for indicating common generic types (collection, list, set). Gonna put together a demo PR for this

[gmazzo]

I don't see why accepting a TypeName would be incompatible with configuration cache?

Because the configuration of the plugin ends up in NamedDomainObjectContainer, which will be stored. If you provide an instance of ClassName, ClassName has to be Serializable as far I know. I'd have to try it.

Another approach could be using a ()->ClassName instead, capturing the logic in a class, but it may become more complicated.

[gmazzo]

This is going to happen with #98 and #100

[gmazzo]

This has been released at 5.0.0

[ZacSweers]

Hey @gmazzo, I see you made a number of changes to what I contributed, and I have to confess I think they've made the API more limited.

• The new BuildConfigType and BuildConfigValue APIs are not usable in BuildConfigClassSpec
• The rename from typeArgs -> typeParams is actually backwards, as arguments are materialized values and parameters are just what go in a declaration signature.
  • The T in fun interface Runnable<T> is a type parameter. The String in Runnable<String> is a type argument.
• The new API doesn't seem to expose a way to define type arguments separate from the base type. They seem to require definition together like List<String> or java.util.Map<String, Integer>.
• Requiring BuildConfigType<RefType : Serializable> to be typed with a type visible at runtime to Gradle severely limits the plugin as it can no longer generate fields that use types that don't exist at build-time. For example - if our app uses kotlinx immutable collections, we now would have to import these types to our gradle build script to use them as well.

Just sharing my 2c.

[gmazzo]

Hi @ZacSweers, I'm actually iterating this, as I found an issue with Configuration Cache and Type references. I'll be to include your feedback.

The new BuildConfigType and BuildConfigValue APIs are not usable in BuildConfigClassSpec

How is so? You mean you can't instantiate them without a buildConfigField call?

The rename from typeArgs -> typeParams is actually backwards, as arguments are materialized values and parameters are just what go in a declaration signature.
The T in fun interface Runnable is a type parameter. The String in Runnable is a type argument.

This just the name of the field right? I can change it. If there is anything else I'm missing from the comment let me know. But I can rename typeParameters to the correct one typeArguments as you stated.

The new API doesn't seem to expose a way to define type arguments separate from the base type. They seem to require definition together like List or java.util.Map<String, Integer>.

I think this is intentional, but not sure I can fully get it. Do you have a use case when having a more verbose API for this will be valuable?

What I had in mind when I design this was:

• For Kotlin: use the reified buildConfigField to set the type
  i.e. buildConfigField<Int> or buildConfigField<Map<String, List<Int>>
• For Groovy, as all type will be generics at runtime, the same cases can be written with:
  • For non generic types: the type literal:
    i.e. buildConfigField('Int',
  • For generic types, pass it as a string:
    i.e. buildConfigField('Map<String, List<Int>>',

In your PR, the "usecase" added in the test was

buildConfigField(FieldType.create('${collectionsPackage}.List', 'String')

Which with this API will be, on Kotlin

buildConfigField<List<String>>(...
// or
buildConfigField("List<String>>", ...

And on Groovy, will be:

buildConfigField('List<String>>', ...

Is possible you mean by "limiting the API" that I've replaced this

    @get:Input
    val type: Property<String>

    @get:Optional
    @get:Input
    val typeArguments: ListProperty<String>

with this?

    @get:Input
    val type: Property<BuildConfigType>

The reason why I did it was to support multiple levels of generics, like Map<String, Set<String>>, by allowing nested BuildConfigType on it. Would you foresee a better way to have both?

Requiring BuildConfigType to be typed with a type visible at runtime to Gradle severely limits the plugin as it can no longer generate fields that use types that don't exist at build-time. For example - if our app uses kotlinx immutable collections, we now would have to import these types to our gradle build script to use them as well.

RefType will be gone, as it's the root class of the Configuration Cache issue I've found. NameRef will just become buildConfigType.

it can no longer generate fields that use types that don't exist at build-time

True, but for those, there is the overloaded signature that accepts a String for type instead, like:

buildConfigField("List<String>", "VALUE", listOf("a", "b") )

Which it won't be parsed as a TypeRef but as a NameRef, very similar to your original approach.

To conclude, I'm going to release a quick fix for the CC issue with the changes:

• Dropping the TypeRef class, in favor of always collecting String class names instead
• Renaming typeParameters to typeArguments
• Adding the missing DSL marker to the inline functions of buildConfigField
• Some other minor fixes I found on the project setup after enabling Configuration Cache org.gradle.configuration-cache=true

Feel free to comment!