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.

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

Jump to bottom

Add a null-check to java enum safeValueOf #5904

Merged
merged 12 commits into from
May 28, 2024
Merged

Add a null-check to java enum safeValueOf #5904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c4e09d4
Add a null-check to java enum safeValueOf
BoD May 17, 2024
af2e53d
Make java enum as class' equals and hashcode call super
BoD May 17, 2024
4967fa4
Update expected
BoD May 17, 2024
f370c5b
Fix test
BoD May 17, 2024
4b67290
Re-generate expected
BoD May 17, 2024
8fffd40
Add a comment to checkNotNull
BoD May 17, 2024
15382be
Fix hashCode equals and toString
BoD May 17, 2024
4631d2e
Rename ApolloEnumConstructor to ApolloUnknownEnum and apply it to saf…
BoD May 20, 2024
66c969a
Update API dump
BoD May 20, 2024
bd22915
Make enums as sealed classes generate sealed interfaces
BoD May 27, 2024
c493ce7
Remove ApolloUnknownEnum
BoD May 28, 2024
7a3c146
Merge pull request #5921 from apollographql/rework-enum-sealed-classes
BoD May 28, 2024
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
3 changes: 0 additions & 3 deletions libraries/apollo-annotations/api/apollo-annotations.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ public final class com/apollographql/apollo3/annotations/ApolloDeprecatedSince$V
public static fun values ()[Lcom/apollographql/apollo3/annotations/ApolloDeprecatedSince$Version;
}

public abstract interface annotation class com/apollographql/apollo3/annotations/ApolloEnumConstructor : java/lang/annotation/Annotation {
}

public abstract interface annotation class com/apollographql/apollo3/annotations/ApolloExperimental : java/lang/annotation/Annotation {
}

Expand Down
3 changes: 0 additions & 3 deletions libraries/apollo-annotations/api/apollo-annotations.klib.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ open annotation class com.apollographql.apollo3.annotations/ApolloDeprecatedSinc
final val version // com.apollographql.apollo3.annotations/ApolloDeprecatedSince.version|{}version[0]
final fun <get-version>(): com.apollographql.apollo3.annotations/ApolloDeprecatedSince.Version // com.apollographql.apollo3.annotations/ApolloDeprecatedSince.version.<get-version>|<get-version>(){}[0]
}
open annotation class com.apollographql.apollo3.annotations/ApolloEnumConstructor : kotlin/Annotation { // com.apollographql.apollo3.annotations/ApolloEnumConstructor|null[0]
constructor <init>() // com.apollographql.apollo3.annotations/ApolloEnumConstructor.<init>|<init>(){}[0]
}
open annotation class com.apollographql.apollo3.annotations/ApolloExperimental : kotlin/Annotation { // com.apollographql.apollo3.annotations/ApolloExperimental|null[0]
constructor <init>() // com.apollographql.apollo3.annotations/ApolloExperimental.<init>|<init>(){}[0]
}
Expand Down
9 changes: 0 additions & 9 deletions ...ions/src/commonMain/kotlin/com/apollographql/apollo3/annotations/ApolloEnumConstructor.kt
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion libraries/apollo-api-java/src/main/java/com/apollographql/apollo3/api/java/Assertions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package com.apollographql.apollo3.api.java;

public class Assertions {
// A version of Objects.requireNonNull that allows a customized message
public static <T> T checkNotNull(T value, String errorMessage) {
if (value == null) {
throw new NullPointerException(errorMessage);
}

return value;
}
}
}
1 change: 1 addition & 0 deletions ...ompiler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/java/JavaClassNames.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ internal object JavaClassNames {
val Map: ClassName = ClassName.get("java.util", "Map")
val MapOfStringToObject = ParameterizedTypeName.get(Map, String, Object)
val JavaOptional = ClassName.get("java.util", "Optional")
val Objects = ClassName.get("java.util", "Objects")

val ObjectBuilderKt = ClassName.get(apolloApiPackageName, "ObjectBuilderKt")
val ObjectMap = ClassName.get(apolloApiPackageName, "ObjectMap")
Expand Down
179 changes: 90 additions & 89 deletions ...iler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/java/helpers/DataClass.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package com.apollographql.apollo3.compiler.codegen.java.helpers

import com.apollographql.apollo3.compiler.GeneratedMethod
import com.apollographql.apollo3.compiler.GeneratedMethod.*
import com.apollographql.apollo3.compiler.internal.applyIf
import com.apollographql.apollo3.compiler.GeneratedMethod.EQUALS_HASH_CODE
import com.apollographql.apollo3.compiler.GeneratedMethod.TO_STRING
import com.apollographql.apollo3.compiler.codegen.Identifier.__h
import com.apollographql.apollo3.compiler.codegen.java.JavaClassNames
import com.apollographql.apollo3.compiler.codegen.java.L
import com.apollographql.apollo3.compiler.codegen.java.joinToCode
import com.apollographql.apollo3.compiler.internal.applyIf
import com.squareup.javapoet.ClassName
import com.squareup.javapoet.CodeBlock
import com.squareup.javapoet.FieldSpec
Expand All @@ -30,7 +31,7 @@ internal fun TypeSpec.Builder.makeClassFromParameters(
generateMethods: List<GeneratedMethod>,
parameters: List<ParameterSpec>,
className: ClassName
): TypeSpec.Builder {
): TypeSpec.Builder {
addMethod(
MethodSpec.constructorBuilder()
.addModifiers(Modifier.PUBLIC)
Expand Down Expand Up @@ -69,7 +70,7 @@ internal fun TypeSpec.Builder.makeClassFromProperties(
generateMethods: List<GeneratedMethod>,
fields: List<FieldSpec>,
className: ClassName
): TypeSpec.Builder {
): TypeSpec.Builder {
addMethod(
MethodSpec.constructorBuilder()
.addModifiers(Modifier.PUBLIC)
Expand All @@ -94,36 +95,36 @@ internal fun TypeSpec.Builder.makeClassFromProperties(

internal fun TypeSpec.Builder.withToStringImplementation(className: ClassName): TypeSpec.Builder {
fun printFieldCode(fieldIndex: Int, fieldName: String) =
CodeBlock.builder()
.let { if (fieldIndex > 0) it.add(" + \", \"\n") else it.add("\n") }
.indent()
.add("+ \$S + \$L", "$fieldName=", fieldName)
.unindent()
.build()
CodeBlock.builder()
.let { if (fieldIndex > 0) it.add(" + \", \"\n") else it.add("\n") }
.indent()
.add("+ \$S + \$L", "$fieldName=", fieldName)
.unindent()
.build()

fun methodCode() =
CodeBlock.builder()
.beginControlFlow("if (\$L == null)", MEMOIZED_TO_STRING_VAR)
.add("\$L = \$S", "\$toString", "${className.simpleName()}{")
.add(fieldSpecs
.filter { !it.hasModifier(Modifier.STATIC) }
.filter { !it.hasModifier(Modifier.TRANSIENT) }
.map { it.name }
.mapIndexed(::printFieldCode)
.fold(CodeBlock.builder(), CodeBlock.Builder::add)
.build())
.add(CodeBlock.builder()
.indent()
.add("\n+ \$S;\n", "}")
.unindent()
.build())
.endControlFlow()
.addStatement("return \$L", MEMOIZED_TO_STRING_VAR)
.build()
CodeBlock.builder()
.beginControlFlow("if (\$L == null)", MEMOIZED_TO_STRING_VAR)
.add("\$L = \$S", "\$toString", "${className.simpleName()}{")
.add(fieldSpecs
.filter { !it.hasModifier(Modifier.STATIC) }
.filter { !it.hasModifier(Modifier.TRANSIENT) }
.map { it.name }
.mapIndexed(::printFieldCode)
.fold(CodeBlock.builder(), CodeBlock.Builder::add)
.build())
.add(CodeBlock.builder()
.indent()
.add("\n+ \$S;\n", "}")
.unindent()
.build())
.endControlFlow()
.addStatement("return \$L", MEMOIZED_TO_STRING_VAR)
.build()

return addField(FieldSpec.builder(JavaClassNames.String, MEMOIZED_TO_STRING_VAR, Modifier.PRIVATE, Modifier.VOLATILE,
Modifier.TRANSIENT)
.build())
Modifier.TRANSIENT)
.build())
.addMethod(MethodSpec.methodBuilder("toString")
.addAnnotation(JavaClassNames.Override)
.addModifiers(Modifier.PUBLIC)
Expand All @@ -138,84 +139,84 @@ private fun List<FieldSpec>.equalsCode(): CodeBlock = filter { !it.hasModifier(M
.joinToCode("\n &&")

private fun FieldSpec.equalsCode() =
CodeBlock.builder()
.let {
if (type.isPrimitive) {
if (type == TypeName.DOUBLE) {
it.add("Double.doubleToLongBits(this.\$L) == Double.doubleToLongBits(that.\$L)",
name, name)
} else {
it.add("this.\$L == that.\$L", name, name)
}
CodeBlock.builder()
.let {
if (type.isPrimitive) {
if (type == TypeName.DOUBLE) {
it.add("Double.doubleToLongBits(this.\$L) == Double.doubleToLongBits(that.\$L)",
name, name)
} else {
it.add("((this.\$L == null) ? (that.\$L == null) : this.\$L.equals(that.\$L))", name, name, name, name)
it.add("this.\$L == that.\$L", name, name)
}
} else {
it.add("((this.\$L == null) ? (that.\$L == null) : this.\$L.equals(that.\$L))", name, name, name, name)
}
.build()
}
.build()

internal fun TypeSpec.Builder.withEqualsImplementation(className: ClassName): TypeSpec.Builder {
fun methodCode(typeJavaClass: ClassName) =
CodeBlock.builder()
.beginControlFlow("if (o == this)")
.addStatement("return true")
.endControlFlow()
.beginControlFlow("if (o instanceof \$T)", typeJavaClass)
.apply {
if (fieldSpecs.isEmpty()) {
add("return true;\n")
} else {
addStatement("\$T that = (\$T) o", typeJavaClass, typeJavaClass)
add("return $L;\n", if (fieldSpecs.isEmpty()) "true" else fieldSpecs.equalsCode())
}
CodeBlock.builder()
.beginControlFlow("if (o == this)")
.addStatement("return true")
.endControlFlow()
.beginControlFlow("if (o instanceof \$T)", typeJavaClass)
.apply {
if (fieldSpecs.isEmpty()) {
add("return true;\n")
} else {
addStatement("\$T that = (\$T) o", typeJavaClass, typeJavaClass)
add("return $L;\n", if (fieldSpecs.isEmpty()) "true" else fieldSpecs.equalsCode())
}
.endControlFlow()
.addStatement("return false")
.build()
}
.endControlFlow()
.addStatement("return false")
.build()

return addMethod(MethodSpec.methodBuilder("equals")
.addAnnotation(JavaClassNames.Override)
.addModifiers(Modifier.PUBLIC)
.returns(TypeName.BOOLEAN)
.addParameter(ParameterSpec.builder(TypeName.OBJECT, "o").build())
.addCode(methodCode(className))
.build())
.addAnnotation(JavaClassNames.Override)
.addModifiers(Modifier.PUBLIC)
.returns(TypeName.BOOLEAN)
.addParameter(ParameterSpec.builder(TypeName.OBJECT, "o").build())
.addCode(methodCode(className))
.build())
}

internal fun TypeSpec.Builder.withHashCodeImplementation(): TypeSpec.Builder {
fun hashFieldCode(field: FieldSpec) =
CodeBlock.builder()
.addStatement("$__h *= 1000003")
.let {
if (field.type.isPrimitive) {
when (field.type.withoutAnnotations()) {
TypeName.DOUBLE -> it.addStatement("$__h ^= Double.valueOf(\$L).hashCode()", field.name)
TypeName.BOOLEAN -> it.addStatement("$__h ^= Boolean.valueOf(\$L).hashCode()", field.name)
else -> it.addStatement("$__h ^= \$L", field.name)
}
} else {
it.addStatement("$__h ^= (\$L == null) ? 0 : \$L.hashCode()", field.name, field.name)
CodeBlock.builder()
.addStatement("$__h *= 1000003")
.let {
if (field.type.isPrimitive) {
when (field.type.withoutAnnotations()) {
TypeName.DOUBLE -> it.addStatement("$__h ^= Double.valueOf(\$L).hashCode()", field.name)
TypeName.BOOLEAN -> it.addStatement("$__h ^= Boolean.valueOf(\$L).hashCode()", field.name)
else -> it.addStatement("$__h ^= \$L", field.name)
}
} else {
it.addStatement("$__h ^= (\$L == null) ? 0 : \$L.hashCode()", field.name, field.name)
}
.build()
}
.build()

fun methodCode() =
CodeBlock.builder()
.beginControlFlow("if (!\$L)", MEMOIZED_HASH_CODE_FLAG_VAR)
.addStatement("int $__h = 1")
.add(fieldSpecs
.filter { !it.hasModifier(Modifier.STATIC) }
.filter { !it.hasModifier(Modifier.TRANSIENT) }
.map(::hashFieldCode)
.fold(CodeBlock.builder(), CodeBlock.Builder::add)
.build())
.addStatement("\$L = $__h", MEMOIZED_HASH_CODE_VAR)
.addStatement("\$L = true", MEMOIZED_HASH_CODE_FLAG_VAR)
.endControlFlow()
.addStatement("return \$L", MEMOIZED_HASH_CODE_VAR)
.build()
CodeBlock.builder()
.beginControlFlow("if (!\$L)", MEMOIZED_HASH_CODE_FLAG_VAR)
.addStatement("int $__h = 1")
.add(fieldSpecs
.filter { !it.hasModifier(Modifier.STATIC) }
.filter { !it.hasModifier(Modifier.TRANSIENT) }
.map(::hashFieldCode)
.fold(CodeBlock.builder(), CodeBlock.Builder::add)
.build())
.addStatement("\$L = $__h", MEMOIZED_HASH_CODE_VAR)
.addStatement("\$L = true", MEMOIZED_HASH_CODE_FLAG_VAR)
.endControlFlow()
.addStatement("return \$L", MEMOIZED_HASH_CODE_VAR)
.build()

return addField(FieldSpec.builder(TypeName.INT, MEMOIZED_HASH_CODE_VAR, Modifier.PRIVATE, Modifier.VOLATILE,
Modifier.TRANSIENT).build())
Modifier.TRANSIENT).build())
.addField(FieldSpec.builder(TypeName.BOOLEAN, MEMOIZED_HASH_CODE_FLAG_VAR, Modifier.PRIVATE,
Modifier.VOLATILE, Modifier.TRANSIENT).build())
.addMethod(MethodSpec.methodBuilder("hashCode")
Expand Down
44 changes: 39 additions & 5 deletions .../main/kotlin/com/apollographql/apollo3/compiler/codegen/java/schema/EnumAsClassBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import com.apollographql.apollo3.compiler.codegen.java.JavaSchemaContext
import com.apollographql.apollo3.compiler.codegen.java.L
import com.apollographql.apollo3.compiler.codegen.java.S
import com.apollographql.apollo3.compiler.codegen.java.T
import com.apollographql.apollo3.compiler.codegen.java.helpers.addGeneratedMethods
import com.apollographql.apollo3.compiler.codegen.java.helpers.maybeAddDescription
import com.apollographql.apollo3.compiler.codegen.java.helpers.maybeSuppressDeprecation
import com.apollographql.apollo3.compiler.codegen.typePackageName
Expand All @@ -22,6 +21,7 @@ import com.squareup.javapoet.CodeBlock
import com.squareup.javapoet.FieldSpec
import com.squareup.javapoet.MethodSpec
import com.squareup.javapoet.ParameterSpec
import com.squareup.javapoet.TypeName
import com.squareup.javapoet.TypeSpec
import javax.lang.model.element.Modifier

Expand Down Expand Up @@ -63,7 +63,7 @@ internal class EnumAsClassBuilder(
)
.addMethod(
MethodSpec.constructorBuilder()
.addModifiers(Modifier.PUBLIC)
.addModifiers(Modifier.PRIVATE)
.addParameter(ParameterSpec.builder(JavaClassNames.String, rawValue).build())
.addCode("this.$rawValue = $rawValue;\n")
.build()
Expand All @@ -79,14 +79,18 @@ internal class EnumAsClassBuilder(
)
.addMethod(
MethodSpec.methodBuilder(safeValueOf)
.addJavadoc(
"Returns the ${enum.name} that represents the specified rawValue.\n" +
"Note: unknown values of rawValue will return UNKNOWN__. You may want to update your schema instead of calling this method directly.\n",
)
.maybeSuppressDeprecation(enum.values)
.addParameter(JavaClassNames.String, rawValue)
.addModifiers(Modifier.PUBLIC)
.addModifiers(Modifier.STATIC)
.returns(selfClassName)
.addCode(
CodeBlock.builder()
.beginControlFlow("switch($rawValue)")
.beginControlFlow("switch ($T.requireNonNull($rawValue))", JavaClassNames.Objects)
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about Objects.requireNonNull. Should we use that instead of

apollo-kotlin/libraries/apollo-api-java/src/main/java/com/apollographql/apollo3/api/java/Assertions.java

Line 4 in 39df250

public static <T> T checkNotNull(T value, String errorMessage) {
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was almost sure we had something like that but couldn't find it anymore 😅. Ours is a bit nicer since you can pass a message - so I'd keep it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment on ours?

// A version of Objects.requireNonNull that allows a customized message

.apply {
values.forEach {
add("case $S: return $T.$L;\n", it.name, selfClassName, it.targetName.escapeTypeReservedWord()
Expand All @@ -113,12 +117,42 @@ internal class EnumAsClassBuilder(
.addJavadoc(L, "An enum value that wasn't known at compile time.\n")
.addMethod(
MethodSpec.constructorBuilder()
.addModifiers(Modifier.PUBLIC)
.addModifiers(Modifier.PRIVATE)
.addParameter(ParameterSpec.builder(JavaClassNames.String, rawValue).build())
.addCode("super($rawValue);\n")
.build()
)
.addGeneratedMethods(ClassName.get("", Identifier.UNKNOWN__))
.addMethod(
MethodSpec.methodBuilder("equals")
.addModifiers(Modifier.PUBLIC)
.addAnnotation(JavaClassNames.Override)
.addParameter(ParameterSpec.builder(JavaClassNames.Object, "other").build())
.returns(TypeName.BOOLEAN)
.addCode(
CodeBlock.builder()
.add("if (this == other) return true;\n")
.add("if (!(other instanceof $L)) return false;\n", Identifier.UNKNOWN__)
.addStatement("return rawValue.equals((($L) other).rawValue)", Identifier.UNKNOWN__)
.build()
)
.build()
)
.addMethod(
MethodSpec.methodBuilder("hashCode")
.addModifiers(Modifier.PUBLIC)
.addAnnotation(JavaClassNames.Override)
.returns(TypeName.INT)
.addCode("return rawValue.hashCode();\n")
.build()
)
.addMethod(
MethodSpec.methodBuilder("toString")
.addModifiers(Modifier.PUBLIC)
.addAnnotation(JavaClassNames.Override)
.returns(JavaClassNames.String)
.addCode("return \"$L(\" + rawValue + \")\";\n", Identifier.UNKNOWN__)
.build()
)
.build()
}
}
Loading
Loading