Skip to content

fix: allow Kotlin generators to emit fields named '_' #880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: allow Kotlin generators to emit fields named '_' #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f02de02
fix: sanitize Kotlin identifiers for underscore fields
cnaples79 Sep 30, 2025
b67b6fa
refactor: reuse keyword sanitizer for kotlin identifiers
cnaples79 Oct 14, 2025
5f400fb
format
paulbakker Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions ...s-codegen-core/src/integTest/kotlin/com/netflix/graphql/dgs/codegen/Kotlin2CodeGenTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,48 @@ class Kotlin2CodeGenTest {
assertThat(updateExpected).isFalse()
}

@Test
fun generateKotlin2TypesWithUnderscoreField() {
val schema =
"""
interface MyInterface {
_: ID
}

type MyInterfaceImpl implements MyInterface {
_: ID
}

type Query {
impl: MyInterfaceImpl
}
""".trimIndent()

val packageName = "com.netflix.graphql.dgs.codegen.kotlin2.underscore"

val codeGenResult =
CodeGen(
CodeGenConfig(
schemas = setOf(schema),
packageName = packageName,
language = Language.KOTLIN,
generateKotlinNullableClasses = true,
generateKotlinClosureProjections = true,
generateClientApi = true,
),
).generate()

val interfaceSpec = codeGenResult.kotlinInterfaces.first { it.name == "MyInterface" }
val interfaceType = interfaceSpec.members.filterIsInstance<com.squareup.kotlinpoet.TypeSpec>().first { it.name == "MyInterface" }
assertThat(interfaceType.propertySpecs.map { it.name }).contains("underscoreField_")

val dataSpec = codeGenResult.kotlinDataTypes.first { it.name == "MyInterfaceImpl" }
val dataType = dataSpec.members.filterIsInstance<com.squareup.kotlinpoet.TypeSpec>().first { it.name == "MyInterfaceImpl" }
assertThat(dataType.propertySpecs.map { it.name }).contains("underscoreField_")

assertCompilesKotlin(codeGenResult)
}

companion object {
@Suppress("unused")
@JvmStatic
Expand Down
3 changes: 2 additions & 1 deletion ...main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinConstantsGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,10 @@ class KotlinConstantsGenerator(
constantsType: TypeSpec.Builder,
name: String,
) {
val kotlinName = sanitizeKotlinIdentifier(name)
constantsType.addProperty(
PropertySpec
.builder(name.capitalized(), String::class)
.builder(kotlinName.capitalized(), String::class)
.addModifiers(KModifier.CONST)
.initializer(""""$name"""")
.build(),
Expand Down
45 changes: 25 additions & 20 deletions .../main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinDataTypeGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,27 @@ class KotlinDataTypeGenerator(
.filter(ReservedKeywordFilter.filterInvalidNames)
.map {
Field(
it.name,
typeUtils.findReturnType(it.type),
typeUtils.isNullable(it.type),
null,
it.description,
it.directives,
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = typeUtils.findReturnType(it.type),
nullable = typeUtils.isNullable(it.type),
default = null,
description = it.description,
directives = it.directives,
)
} +
extensions
.flatMap { it.fieldDefinitions }
.filterSkipped()
.map {
Field(
it.name,
typeUtils.findReturnType(it.type),
typeUtils.isNullable(it.type),
null,
it.description,
it.directives,
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = typeUtils.findReturnType(it.type),
nullable = typeUtils.isNullable(it.type),
default = null,
description = it.description,
directives = it.directives,
)
}
val interfaces = definition.implements + extensions.flatMap { it.implements }
Expand Down Expand Up @@ -128,7 +130,8 @@ class KotlinInputTypeGenerator(
)
}
Field(
name = it.name,
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = typeUtils.findReturnType(it.type),
nullable = it.type !is NonNullType,
default = defaultValue,
Expand All @@ -138,7 +141,8 @@ class KotlinInputTypeGenerator(
}.plus(
extensions.flatMap { it.inputValueDefinitions }.map {
Field(
name = it.name,
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = typeUtils.findReturnType(it.type),
nullable = it.type !is NonNullType,
default = null,
Expand All @@ -153,7 +157,8 @@ class KotlinInputTypeGenerator(
}

internal data class Field(
val name: String,
val graphQLName: String,
val kotlinName: String,
val type: KtTypeName,
val nullable: Boolean,
val default: CodeBlock? = null,
Expand Down Expand Up @@ -209,8 +214,8 @@ abstract class AbstractKotlinDataTypeGenerator(

val parameterSpec =
ParameterSpec
.builder(field.name, returnType)
.addAnnotation(jsonPropertyAnnotation(field.name))
.builder(field.kotlinName, returnType)
.addAnnotation(jsonPropertyAnnotation(field.graphQLName))

if (field.directives.isNotEmpty()) {
parameterSpec.addAnnotations(applyDirectivesKotlin(field.directives, config))
Expand All @@ -234,12 +239,12 @@ abstract class AbstractKotlinDataTypeGenerator(

fields.forEach { field ->
val returnType = if (field.nullable) field.type.copy(nullable = true) else field.type
val propertySpecBuilder = PropertySpec.builder(field.name, returnType)
val propertySpecBuilder = PropertySpec.builder(field.kotlinName, returnType)

if (field.description != null) {
propertySpecBuilder.addKdoc("%L", field.description.sanitizeKdoc())
}
propertySpecBuilder.initializer(field.name)
propertySpecBuilder.initializer(field.kotlinName)

val interfaceNames =
interfaces
Expand All @@ -256,7 +261,7 @@ abstract class AbstractKotlinDataTypeGenerator(
.map { it.name }
.toSet()

if (field.name in interfaceFields) {
if (field.graphQLName in interfaceFields) {
// Properties are the syntactical element that will allow us to override things, they are the spec on
// which we should add the override modifier.
propertySpecBuilder.addModifiers(KModifier.OVERRIDE)
Expand Down
30 changes: 22 additions & 8 deletions ...etflix/graphql/dgs/codegen/generators/kotlin/KotlinEntitiesRepresentationTypeGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,14 @@ class KotlinEntitiesRepresentationTypeGenerator(
}
var fieldsCodeGenAccumulator = CodeGenResult.EMPTY
// generate representations of entity types that have @key, including the __typename field, and the key fields
val typeName = Field("__typename", STRING, false, CodeBlock.of("%S", definitionName))
val typeName =
Field(
graphQLName = "__typename",
kotlinName = sanitizeKotlinIdentifier("__typename"),
type = STRING,
nullable = false,
default = CodeBlock.of("%S", definitionName),
)
val fieldDefinitions =
fields
.filter { keyFields.containsKey(it.name) }
Expand Down Expand Up @@ -104,19 +111,26 @@ class KotlinEntitiesRepresentationTypeGenerator(
}
if (fieldType is ParameterizedTypeName && fieldType.rawType.simpleName == "List") {
Field(
it.name,
LIST.parameterizedBy(ClassName(getPackageName(), fieldTypeRepresentationName)),
typeUtils.isNullable(it.type),
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = LIST.parameterizedBy(ClassName(getPackageName(), fieldTypeRepresentationName)),
nullable = typeUtils.isNullable(it.type),
)
} else {
Field(
it.name,
ClassName(getPackageName(), fieldTypeRepresentationName),
typeUtils.isNullable(it.type),
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = ClassName(getPackageName(), fieldTypeRepresentationName),
nullable = typeUtils.isNullable(it.type),
)
}
} else {
Field(it.name, typeUtils.findReturnType(it.type), typeUtils.isNullable(it.type))
Field(
graphQLName = it.name,
kotlinName = sanitizeKotlinIdentifier(it.name),
type = typeUtils.findReturnType(it.type),
nullable = typeUtils.isNullable(it.type),
)
}
}
// Generate base type representation...
Expand Down
50 changes: 27 additions & 23 deletions .../kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinInterfaceTypeGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,32 +64,36 @@ class KotlinInterfaceTypeGenerator(

val mergedFieldDefinitions = definition.fieldDefinitions + extensions.flatMap { it.fieldDefinitions }

mergedFieldDefinitions.filterSkipped().forEach { field ->
val returnType = typeUtils.findReturnType(field.type)
val nullableType = if (typeUtils.isNullable(field.type)) returnType.copy(nullable = true) else returnType
val propertySpec = PropertySpec.builder(field.name, nullableType)
if (field.description != null) {
propertySpec.addKdoc("%L", field.description.sanitizeKdoc())
}
mergedFieldDefinitions
.filterSkipped()
.filter(ReservedKeywordFilter.filterInvalidNames)
.forEach { field ->
val returnType = typeUtils.findReturnType(field.type)
val nullableType = if (typeUtils.isNullable(field.type)) returnType.copy(nullable = true) else returnType
val kotlinName = sanitizeKotlinIdentifier(field.name)
val propertySpec = PropertySpec.builder(kotlinName, nullableType)
if (field.description != null) {
propertySpec.addKdoc("%L", field.description.sanitizeKdoc())
}

if (definition.implements.isNotEmpty()) {
val superInterfaceFields =
document
.getDefinitionsOfType(InterfaceTypeDefinition::class.java)
.filter {
superInterfacesNames(definition).contains(it.name)
}.asSequence()
.flatMap { it.fieldDefinitions }
.map { it.name }
.toSet()

if (field.name in superInterfaceFields) {
propertySpec.addModifiers(KModifier.OVERRIDE)
if (definition.implements.isNotEmpty()) {
val superInterfaceFields =
document
.getDefinitionsOfType(InterfaceTypeDefinition::class.java)
.filter {
superInterfacesNames(definition).contains(it.name)
}.asSequence()
.flatMap { it.fieldDefinitions }
.map { it.name }
.toSet()

if (field.name in superInterfaceFields) {
propertySpec.addModifiers(KModifier.OVERRIDE)
}
}
}

interfaceBuilder.addProperty(propertySpec.build())
}
interfaceBuilder.addProperty(propertySpec.build())
}

val implementations =
document
Expand Down
8 changes: 8 additions & 0 deletions ...core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/KotlinPoetUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ import graphql.language.StringValue
import graphql.language.Value
import java.lang.IllegalArgumentException

private val kotlinReservedKeywordSanitizer = KotlinReservedKeywordSanitizer()

fun sanitizeKotlinIdentifier(name: String): String =
when {
name == "_" -> "underscoreField_"
else -> kotlinReservedKeywordSanitizer.sanitize(name)
}

/**
* Generate a [JsonTypeInfo] annotation, which allows for Jackson
* polymorphic type handling when deserializing from JSON.
Expand Down
2 changes: 1 addition & 1 deletion ...rc/main/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin/ReservedKeywordFilter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ package com.netflix.graphql.dgs.codegen.generators.kotlin
import graphql.language.NamedNode

object ReservedKeywordFilter {
val filterInvalidNames: (NamedNode<*>) -> Boolean = { it.name != "_" }
val filterInvalidNames: (NamedNode<*>) -> Boolean = { true }
Copy link
Collaborator

@paulbakker paulbakker Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should just remove this completely now that it's basically a no-op.

Copy link
Collaborator

@paulbakker paulbakker Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiholee17 do you want to take a look at removing this filter after we merge this?

Copy link
Contributor

@jiholee17 jiholee17 Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I can take a look

}
22 changes: 16 additions & 6 deletions ...n/kotlin/com/netflix/graphql/dgs/codegen/generators/kotlin2/GenerateKotlin2ClientTypes.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.netflix.graphql.dgs.codegen.GraphQLProjection
import com.netflix.graphql.dgs.codegen.filterSkipped
import com.netflix.graphql.dgs.codegen.generators.kotlin.ReservedKeywordFilter
import com.netflix.graphql.dgs.codegen.generators.kotlin.addOptionalGeneratedAnnotation
import com.netflix.graphql.dgs.codegen.generators.kotlin.sanitizeKotlinIdentifier
import com.netflix.graphql.dgs.codegen.generators.shared.SchemaExtensionsUtils
import com.netflix.graphql.dgs.codegen.generators.shared.SchemaExtensionsUtils.collectAllFieldDefinitions
import com.netflix.graphql.dgs.codegen.generators.shared.excludeSchemaTypeExtension
Expand Down Expand Up @@ -82,6 +83,8 @@ fun generateKotlin2ClientTypes(
.filter(ReservedKeywordFilter.filterInvalidNames)
.map { field ->

val kotlinName = sanitizeKotlinIdentifier(field.name)

val isScalar = typeLookup.isScalar(field.type)
val hasArgs = field.inputValueDefinitions.isNotEmpty()

Expand All @@ -90,7 +93,7 @@ fun generateKotlin2ClientTypes(
isScalar && !hasArgs -> {
PropertySpec
.builder(
name = field.name,
name = kotlinName,
type = typeName,
).getter(
FunSpec
Expand All @@ -104,13 +107,16 @@ fun generateKotlin2ClientTypes(
// scalars with args are functions to take the args with no projection
isScalar && hasArgs -> {
FunSpec
.builder(field.name)
.builder(kotlinName)
.addInputArgs(config, typeLookup, typeName, field.inputValueDefinitions)
.returns(typeName)
.addCode(
field.inputValueDefinitions.let { iv ->
val builder = CodeBlock.builder().add("field(%S", field.name)
iv.forEach { d -> builder.add(", %S to %N", d.name, d.name) }
iv.forEach { d ->
val argName = sanitizeKotlinIdentifier(d.name)
builder.add(", %S to %N", d.name, argName)
}
builder.add(")\nreturn this").build()
},
).build()
Expand All @@ -123,7 +129,7 @@ fun generateKotlin2ClientTypes(
val (projectionType, projection) = projectionType(config.packageNameClient, projectionTypeName)

FunSpec
.builder(field.name)
.builder(kotlinName)
.addInputArgs(config, typeLookup, typeName, field.inputValueDefinitions)
.addParameter(
ParameterSpec
Expand All @@ -142,7 +148,10 @@ fun generateKotlin2ClientTypes(
field.name,
projectionType,
)
iv.forEach { d -> builder.add(", %S to %N", d.name, d.name) }
iv.forEach { d ->
val argName = sanitizeKotlinIdentifier(d.name)
builder.add(", %S to %N", d.name, argName)
}
builder.add(")\nreturn this").build()
},
).build()
Expand Down Expand Up @@ -288,8 +297,9 @@ private fun FunSpec.Builder.addInputArgs(
.addParameters(
inputValueDefinitions.map {
val returnType = typeLookup.findReturnType(config.packageNameTypes, it.type)
val kotlinName = sanitizeKotlinIdentifier(it.name)
ParameterSpec
.builder(it.name, returnType)
.builder(kotlinName, returnType)
.apply {
if (returnType.isNullable) {
defaultValue("default<%T, %T>(%S)", typeName, returnType, it.name)
Expand Down
Loading
Loading