Skip to content
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.

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

Automatically escape type in enum values #4295

Merged
merged 3 commits into from Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion apollo-compiler/api/apollo-compiler.api
Expand Up @@ -192,8 +192,9 @@ public final class com/apollographql/apollo3/compiler/PackageNameGenerator$Flat

public final class com/apollographql/apollo3/compiler/ReservedKeywordsKt {
public static final fun escapeJavaReservedWord (Ljava/lang/String;)Ljava/lang/String;
public static final fun escapeKotlinReservedEnumValueNames (Ljava/lang/String;)Ljava/lang/String;
public static final fun escapeKotlinReservedWord (Ljava/lang/String;)Ljava/lang/String;
public static final fun escapeKotlinReservedWordInEnum (Ljava/lang/String;)Ljava/lang/String;
public static final fun escapeKotlinReservedWordInSealedClass (Ljava/lang/String;)Ljava/lang/String;
}

public final class com/apollographql/apollo3/compiler/Roots {
Expand Down
Expand Up @@ -15,14 +15,25 @@ fun String.escapeJavaReservedWord() = if (this in JAVA_RESERVED_WORDS) "${this}_
// Does nothing. KotlinPoet will add the backticks
fun String.escapeKotlinReservedWord() = this

fun String.escapeKotlinReservedEnumValueNames(): String {
internal fun String.escapeTypeReservedWord(): String? {
return when {
// https://kotlinlang.org/docs/enum-classes.html#working-with-enum-constants:~:text=properties%20for%20obtaining%20its%20name%20and%20position
// type is forbidden because we use it as a companion property to hold the CompiledType
// See https://github.com/apollographql/apollo-kotlin/issues/4293
"(?:type)_*".toRegex().matches(this) -> "${this}_"
else -> null
}
}


fun String.escapeKotlinReservedWordInEnum(): String {
return when {
// name and ordinal are forbidden because already used in Kotlin
// See https://kotlinlang.org/docs/enum-classes.html#working-with-enum-constants:~:text=properties%20for%20obtaining%20its%20name%20and%20position
"(?:name|ordinal)_*".toRegex().matches(this) -> "${this}_"
// "header" and "impl" are added to this list because of https://youtrack.jetbrains.com/issue/KT-52315
this in arrayOf("header", "impl") -> "`${this}`"
else -> this
else -> escapeTypeReservedWord() ?: escapeKotlinReservedWord()
}
}

internal fun String.isApolloReservedEnumValueName() = this == "type"
fun String.escapeKotlinReservedWordInSealedClass(): String {
return escapeTypeReservedWord() ?: escapeKotlinReservedWord()
}
Expand Up @@ -20,32 +20,16 @@ internal fun checkApolloReservedEnumValueNames(schema: Schema): List<Issue> {
val targetName = value.directives.findTargetName(schema)

val name = targetName ?: value.name
when {
name.isApolloReservedEnumValueName() -> {
val message = if (targetName == null) {
"'$name' is a reserved enum value name, please use the @targetName directive to specify a target name"
} else {
"'$name' is a reserved enum value name, please use a different name"
}
issues.add(
Issue.ReservedEnumValueName(
message = message,
sourceLocation = value.sourceLocation
)
)
}
name in enumNames -> {
issues.add(
Issue.ReservedEnumValueName(
message = "'${targetName}' is already defined in this enum, please use a different name",
sourceLocation = value.sourceLocation
)
)
}
else -> {
// all good
enumNames.add(name)
}
if (name in enumNames) {
issues.add(
Issue.ReservedEnumValueName(
message = "'${targetName}' is already defined in this enum, please use a different name",
sourceLocation = value.sourceLocation
)
)
} else {
// all good
enumNames.add(name)
}
}
}
Expand Down
Expand Up @@ -3,6 +3,7 @@ package com.apollographql.apollo3.compiler.codegen.java
import com.apollographql.apollo3.compiler.PackageNameGenerator
import com.apollographql.apollo3.compiler.codegen.CodegenLayout
import com.apollographql.apollo3.compiler.escapeJavaReservedWord
import com.apollographql.apollo3.compiler.escapeTypeReservedWord
import com.apollographql.apollo3.compiler.ir.Ir

internal class JavaCodegenLayout(
Expand All @@ -22,5 +23,5 @@ internal class JavaCodegenLayout(

// We used to write upper case enum values but the server can define different values with different cases
// See https://github.com/apollographql/apollo-android/issues/3035
internal fun enumValueName(name: String) = regularIdentifier(name)
internal fun enumValueName(name: String) = name.escapeTypeReservedWord() ?: regularIdentifier(name)
}
Expand Up @@ -2,8 +2,10 @@ package com.apollographql.apollo3.compiler.codegen.kotlin

import com.apollographql.apollo3.compiler.PackageNameGenerator
import com.apollographql.apollo3.compiler.codegen.CodegenLayout
import com.apollographql.apollo3.compiler.escapeKotlinReservedEnumValueNames
import com.apollographql.apollo3.compiler.escapeKotlinReservedWordInEnum
import com.apollographql.apollo3.compiler.escapeKotlinReservedWord
import com.apollographql.apollo3.compiler.escapeKotlinReservedWordInSealedClass
import com.apollographql.apollo3.compiler.escapeTypeReservedWord
import com.apollographql.apollo3.compiler.ir.Ir

internal class KotlinCodegenLayout(
Expand All @@ -24,10 +26,10 @@ internal class KotlinCodegenLayout(
/**
* Enum value name to use when generating enums as sealed classes
*/
internal fun enumAsSealedClassValueName(name: String) = regularIdentifier(name)
internal fun enumAsSealedClassValueName(name: String) = name.escapeKotlinReservedWordInSealedClass()

/**
* Enum value name to use when generating enums as enums
*/
internal fun enumAsEnumValueName(name: String) = name.escapeKotlinReservedEnumValueNames()
internal fun enumAsEnumValueName(name: String) = name.escapeKotlinReservedWordInEnum()
}
Expand Up @@ -13,7 +13,7 @@ enum Gravity {
is

# a name that clashes with the generated `type` constant
type @targetName(name: "type_")
type

# an enum value that clashes with the rawValue type
String
Expand Down
Expand Up @@ -8,5 +8,5 @@ enum Enum {
NORTH
SOUTH

type @targetName(name: "type_")
type
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -6,14 +6,6 @@ enum Enum {
type
}

enum Enum2 {
type @targetName(name: "type")
}

enum Enum3 {
foo @targetName(name: "type")
}

enum Enum4 {
foo
bar @targetName(name: "foo")
Expand Down
2 changes: 1 addition & 1 deletion tests/enums/build.gradle.kts
Expand Up @@ -11,5 +11,5 @@ dependencies {

apollo {
packageName.set("enums")
sealedClassesForEnumsMatching.set(listOf(".*avity"))
sealedClassesForEnumsMatching.set(listOf(".*avity", "FooSealed"))
}
2 changes: 2 additions & 0 deletions tests/enums/src/main/graphql/operation.graphql
Expand Up @@ -2,4 +2,6 @@ query GetEnums {
direction
gravity
foo
fooSealed
fooEnum
}
16 changes: 14 additions & 2 deletions tests/enums/src/main/graphql/schema.graphqls
Expand Up @@ -2,6 +2,8 @@ type Query {
direction: Direction
gravity: Gravity
foo: Foo
fooSealed: FooSealed
fooEnum: FooEnum
}

enum Direction {
Expand All @@ -11,7 +13,7 @@ enum Direction {
EAST,
WEST,

# Clashes, must be renamed in extra.graphqls
# renamed in extra.graphqls
type,

# Value names should be escaped with _ suffixes when generating enums
Expand All @@ -33,7 +35,7 @@ enum Gravity {
name,
ordinal,

# Clashes, must be renamed in extra.graphqls
# renamed in extra.graphqls
type,
}

Expand All @@ -42,3 +44,13 @@ enum Foo {
header,
footer,
}

enum FooEnum {
# not renamed in extra.graphqls, will be renamed automatically
type,
}

enum FooSealed {
# not renamed in extra.graphqls, will be renamed automatically
type,
}
16 changes: 13 additions & 3 deletions tests/enums/src/test/kotlin/test/EnumsTest.kt
Expand Up @@ -2,6 +2,8 @@ package test

import enums.type.Direction
import enums.type.Foo
import enums.type.FooEnum
import enums.type.FooSealed
import enums.type.Gravity
import org.junit.Test
import kotlin.test.assertEquals
Expand All @@ -14,7 +16,7 @@ class EnumsTest {
assertEquals(Direction.UNKNOWN__, Direction.safeValueOf("newDirection"))
assertEquals(Direction.name_, Direction.safeValueOf("name"))
assertEquals(Direction.ordinal_, Direction.safeValueOf("ordinal"))
assertEquals(Direction.type_, Direction.safeValueOf("type"))
assertEquals(Direction.type__, Direction.safeValueOf("type"))
}

@Test
Expand All @@ -24,14 +26,22 @@ class EnumsTest {
assertEquals(Gravity.UNKNOWN__("newGravity"), Gravity.safeValueOf("newGravity"))
assertEquals(Gravity.name, Gravity.safeValueOf("name"))
assertEquals(Gravity.ordinal, Gravity.safeValueOf("ordinal"))
assertEquals(Gravity.type_, Gravity.safeValueOf("type"))
assertEquals(Gravity.type__, Gravity.safeValueOf("type"))
}

@Test
fun headerAndImpl() {
assertEquals(Foo.header.rawValue, "header")
}

@Test
fun type() {
assertEquals(Direction.type__, Direction.safeValueOf("type"))
assertEquals(Gravity.type__, Gravity.safeValueOf("type"))
assertEquals(FooSealed.type_, FooSealed.safeValueOf("type"))
assertEquals(FooEnum.type_, FooEnum.safeValueOf("type"))
}

@Test
fun sealedClassesKnownValues() {
// Order is important
Expand All @@ -47,7 +57,7 @@ class EnumsTest {
Gravity.RIGHT,
Gravity.name,
Gravity.ordinal,
Gravity.type_,
Gravity.type__,
).toList(),
Gravity.knownValues().toList()
)
Expand Down