Added support for default array values

[paulbakker]

I found a corner case of default array values.
Since we are still Java 8 compatible, there isn't really a nice way to initialize a List of default values, so it's always just generating Collections.emptyList(), which is probably by far the most use case anyway. We could consider adding a flag later to generate Java > 8 style code, but that's a separate issue.

In Kotlin it's checking the contained type, and creates either an emptyList() or a listOf(). For the latter case it re-uses the default value logic to create the right contents in the list.

[zappolowski]

Since we are still Java 8 compatible, there isn't really a nice way to initialize a List of default values

Isn't Arrays.asList() already available in Java 8?

[zappolowski]

IIRC collection.map { body }.joinToString() could be rewritten as collection.joinToString { body }.

[paulbakker]

Would need to do a toString() on each codeblock in that case.

[zappolowski]

I'm using (in my patch for another comment)

CodeBlock.of("listOf(%L)", value.values.joinToString { v -> generateCode(v, type).toString() })

which saves an intermediary list. The call to toString() is explicit in this approach, but would happen anyhow (Iterable<T>.joinToString uses Iterable<T>.joinTo which eventually calls Appendable.appendElement which calls toString() by default :) )

[zappolowski]

It doesn't handle ArrayValue here, which would prevent having nested lists (I'm not sure if this is allowed in GraphQL at all).

That when is the same as the enclosing on (minus the nesting). Is it easily feasible to extract this? This would handling more types (like the missing ObjectValue easier).

[paulbakker]

Arrays in arrays are not possible AFAIK. I thought about extracting it, but I think the code just gets more complicated that way, also because the blocks aren't exactly the same. Feel free to give it a try though if you disagree.

[zappolowski]

From some quick testing, nested lists seem to be allowed as long as they are homogeneous.

What do you think about this?

diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinDataTypeGenerator.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinDataTypeGenerator.kt
index 616d292..cb1bf66 100644
--- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinDataTypeGenerator.kt
+++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinDataTypeGenerator.kt
@@ -23,9 +23,9 @@ import com.netflix.graphql.dgs.codegen.KotlinCodeGenResult
 import com.netflix.graphql.dgs.codegen.filterSkipped
 import com.netflix.graphql.dgs.codegen.shouldSkip
 import com.squareup.kotlinpoet.*
+import com.squareup.kotlinpoet.TypeName
 import graphql.language.*
 
-
 class KotlinDataTypeGenerator(private val config: CodeGenConfig, private val document: Document): AbstractKotlinDataTypeGenerator(config.packageName + ".types", config) {
     fun generate(definition: ObjectTypeDefinition, extensions: List<ObjectTypeExtensionDefinition>): KotlinCodeGenResult {
         if(definition.shouldSkip()) {
@@ -56,32 +56,33 @@ class KotlinInputTypeGenerator(private val config: CodeGenConfig, private val do
                 .filter(ReservedKeywordFilter.filterInvalidNames)
                 .map {
                     val type = typeUtils.findReturnType(it.type)
-                    val defaultValue = it.defaultValue?.let { defVal ->
-                        when (defVal) {
-                            is BooleanValue -> CodeBlock.of("%L", defVal.isValue)
-                            is IntValue -> CodeBlock.of("%L", defVal.value)
-                            is StringValue -> CodeBlock.of("%S", defVal.value)
-                            is FloatValue -> CodeBlock.of("%L", defVal.value)
-                            is EnumValue -> CodeBlock.of("%M", MemberName(type as ClassName, defVal.name))
-                            is ArrayValue -> if(defVal.values.isEmpty()) CodeBlock.of("emptyList()") else CodeBlock.of("listOf(%L)", defVal.values.map { v ->
-                                when(v) {
-                                    is BooleanValue -> CodeBlock.of("%L", v.isValue)
-                                    is IntValue -> CodeBlock.of("%L", v.value)
-                                    is StringValue -> CodeBlock.of("%S", v.value)
-                                    is FloatValue -> CodeBlock.of("%Lf", v.value)
-                                    is EnumValue -> CodeBlock.of("%M", MemberName((type as ParameterizedTypeName).typeArguments[0] as ClassName, v.name))
-                                    else -> ""
-                                }
-                            }.joinToString())
-                            else -> CodeBlock.of("%L", defVal)
-                        }
-                    }
+                    val defaultValue = it.defaultValue?.let { value -> generateCode(value, type) }
                     Field(it.name, type, typeUtils.isNullable(it.type), defaultValue)
                 }.plus(extensions.flatMap { it.inputValueDefinitions }.map { Field(it.name, typeUtils.findReturnType(it.type), typeUtils.isNullable(it.type)) })
         val interfaces = emptyList<Type<*>>()
         return generate(definition.name, fields, interfaces, true, document)
     }
 
+    private fun generateCode(value: Value<Value<*>>, type: TypeName): CodeBlock =
+        when (value) {
+            is BooleanValue -> CodeBlock.of("%L", value.isValue)
+            is IntValue -> CodeBlock.of("%L", value.value)
+            is StringValue -> CodeBlock.of("%S", value.value)
+            is FloatValue -> CodeBlock.of("%L", value.value)
+            is EnumValue -> CodeBlock.of("%M", MemberName(type.className, value.name))
+            is ArrayValue ->
+                if (value.values.isEmpty()) CodeBlock.of("emptyList()")
+                else CodeBlock.of("listOf(%L)", value.values.joinToString { v -> generateCode(v, type).toString() })
+            else -> CodeBlock.of("%L", value)
+        }
+
+    private val TypeName.className: ClassName
+        get() = when (this) {
+            is ClassName -> this
+            is ParameterizedTypeName -> typeArguments[0].className
+            else -> TODO()
+        }
+
     override fun getPackageName(): String {
         return config.packageName + ".types"
     }

which could (haven't tested it yet) handle nested arrays as well.

[paulbakker]

I think we're safe to ignore lists of lists, but would be fine with the refactoring you propose

[paulbakker]

Since we are still Java 8 compatible, there isn't really a nice way to initialize a List of default values

Isn't Arrays.asList() already available in Java 8?

Clearly I'm doing to much Kotlin these days. Added default array values for Java based on this now.

[paulbakker]

I suggest we merge this into the original PR and do any remaining finishing touches there?