From 2f494f9ccb81d35ef7862d12648e46a5ef901f85 Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Fri, 19 Jul 2024 00:25:09 -0700 Subject: [PATCH] Remove unnecessary javadoc helper methods Remove unneeded sanitizeJavaDoc helper methods, and fix the call-site to simply pass in the correct format specifiers to addJavaDoc. Fixes #694. Other cleanup: - Remove trivial deprecatedAnnotation helper method - Use ClassName constants where possible in typeClassBestGuess --- .../generators/java/ClientApiGenerator.kt | 42 +++++-------- .../generators/java/DataTypeGenerator.kt | 6 +- .../generators/java/EnumTypeGenerator.kt | 10 +-- .../generators/java/InterfaceGenerator.kt | 13 ++-- .../codegen/generators/java/JavaPoetUtils.kt | 61 ++++++------------- .../generators/shared/DirectivesUtils.kt | 7 ++- .../graphql/dgs/codegen/CodeGenTest.kt | 14 ++--- 7 files changed, 61 insertions(+), 92 deletions(-) diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/ClientApiGenerator.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/ClientApiGenerator.kt index f42c80b38..4eeec31ee 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/ClientApiGenerator.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/ClientApiGenerator.kt @@ -21,7 +21,6 @@ package com.netflix.graphql.dgs.codegen.generators.java import com.netflix.graphql.dgs.client.codegen.BaseSubProjectionNode import com.netflix.graphql.dgs.client.codegen.GraphQLQuery import com.netflix.graphql.dgs.codegen.* -import com.netflix.graphql.dgs.codegen.generators.shared.ClassnameShortener import com.netflix.graphql.dgs.codegen.generators.shared.CodeGeneratorUtils.capitalized import com.squareup.javapoet.* import graphql.introspection.Introspection.TypeNameMetaFieldDef @@ -65,7 +64,7 @@ class ClientApiGenerator(private val config: CodeGenConfig, private val document .addModifiers(Modifier.PUBLIC).superclass(ClassName.get(GraphQLQuery::class.java)) if (it.description != null) { - javaType.addJavadoc(it.description.sanitizeJavaDoc()) + javaType.addJavadoc("\$L", it.description.content) } val deprecatedClassDirective = getDeprecateDirective(it) @@ -73,7 +72,7 @@ class ClientApiGenerator(private val config: CodeGenConfig, private val document javaType.addAnnotation(java.lang.Deprecated::class.java) val deprecationReason = getDeprecatedReason(deprecatedClassDirective) if (deprecationReason != null) { - javaType.addJavadoc("@deprecated " + deprecationReason.sanitizeJavaDoc()) + javaType.addJavadoc("@deprecated \$L", deprecationReason) } } @@ -82,12 +81,8 @@ class ClientApiGenerator(private val config: CodeGenConfig, private val document .addModifiers(Modifier.PUBLIC) .returns(String::class.java) .addAnnotation(Override::class.java) - .addCode( - """ - | return "${it.name}"; - | - """.trimMargin() - ).build() + .addStatement("return \$S", it.name) + .build() ) val setType = ClassName.get(Set::class.java) @@ -116,12 +111,7 @@ class ClientApiGenerator(private val config: CodeGenConfig, private val document val constructorBuilder = MethodSpec.constructorBuilder() .addModifiers(Modifier.PUBLIC) - constructorBuilder.addCode( - """ - |super("${operation.lowercase()}", queryName); - | - """.trimMargin() - ) + constructorBuilder.addStatement("super(\$S, queryName)", operation.lowercase()) it.inputValueDefinitions.forEach { inputValue -> val findReturnType = TypeUtils(getDatatypesPackageName(), config, document).findReturnType(inputValue.type) @@ -146,19 +136,21 @@ class ClientApiGenerator(private val config: CodeGenConfig, private val document } // Build Javadoc, separate multiple blocks by empty line - val javaDocCodeBlocks = mutableListOf() + val javaDoc = CodeBlock.builder() if (inputValue.description != null) { - javaDocCodeBlocks.add(inputValue.description.sanitizeJavaDoc()) + javaDoc.add("\$L", inputValue.description.content) } if (deprecationReason != null) { - javaDocCodeBlocks.add("@deprecated " + deprecationReason.sanitizeJavaDoc()) + if (!javaDoc.isEmpty) { + javaDoc.add("\n\n") + } + javaDoc.add("@deprecated \$L", deprecationReason) } - javaDocCodeBlocks - .takeIf { it.isNotEmpty() } - ?.joinToString("\n\n") - ?.also { methodBuilder.addJavadoc(it) } + if (!javaDoc.isEmpty) { + methodBuilder.addJavadoc(javaDoc.build()) + } builderClass.addMethod(methodBuilder.build()) .addField(findReturnType, ReservedKeywordSanitizer.sanitize(inputValue.name), Modifier.PRIVATE) @@ -675,14 +667,10 @@ class ClientApiGenerator(private val config: CodeGenConfig, private val document } private fun getDeprecatedReason(directive: Directive): String? { - return directive - ?.getArgument("reason") + return directive.getArgument("reason") ?.let { it.value as? StringValue } ?.value } - private fun truncatePrefix(prefix: String): String { - return if (config.shortProjectionNames) ClassnameShortener.shorten(prefix) else prefix - } private fun getPackageName(): String { return config.packageNameClient diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/DataTypeGenerator.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/DataTypeGenerator.kt index ad5b0fc00..32391f822 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/DataTypeGenerator.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/DataTypeGenerator.kt @@ -288,7 +288,7 @@ abstract class BaseDataTypeGenerator( } if (description != null) { - javaType.addJavadoc(description.sanitizeJavaDoc()) + javaType.addJavadoc("\$L", description.content) } if (directives.isNotEmpty()) { @@ -468,7 +468,7 @@ abstract class BaseDataTypeGenerator( } if (fieldDefinition.description != null) { - fieldBuilder.addJavadoc(fieldDefinition.description.sanitizeJavaDoc()) + fieldBuilder.addJavadoc("\$L", fieldDefinition.description.content) } val getterPrefix = if (returnType == com.squareup.javapoet.TypeName.BOOLEAN && config.generateIsGetterForPrimitiveBooleanFields) "is" else "get" @@ -480,7 +480,7 @@ abstract class BaseDataTypeGenerator( } if (fieldDefinition.description != null) { - getterMethodBuilder.addJavadoc(fieldDefinition.description.sanitizeJavaDoc()) + getterMethodBuilder.addJavadoc("\$L", fieldDefinition.description.content) } val setterName = typeUtils.transformIfDefaultClassMethodExists("set${fieldDefinition.name[0].uppercase()}${fieldDefinition.name.substring(1)}", TypeUtils.setClass) diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/EnumTypeGenerator.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/EnumTypeGenerator.kt index 235bafbb7..e5653741e 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/EnumTypeGenerator.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/EnumTypeGenerator.kt @@ -31,14 +31,16 @@ import org.slf4j.LoggerFactory import javax.lang.model.element.Modifier class EnumTypeGenerator(private val config: CodeGenConfig) { - private val logger: Logger = LoggerFactory.getLogger(EnumTypeGenerator::class.java) + companion object { + private val logger: Logger = LoggerFactory.getLogger(EnumTypeGenerator::class.java) + } fun generate(definition: EnumTypeDefinition, extensions: List): CodeGenResult { if (definition.shouldSkip(config)) { return CodeGenResult() } - logger.info("Generating enum type ${definition.name}") + logger.info("Generating enum type {}", definition.name) val javaType = TypeSpec @@ -47,13 +49,13 @@ class EnumTypeGenerator(private val config: CodeGenConfig) { .addOptionalGeneratedAnnotation(config) if (definition.description != null) { - javaType.addJavadoc(definition.description.sanitizeJavaDoc()) + javaType.addJavadoc("\$L", definition.description.content) } val mergedEnumDefinitions = definition.enumValueDefinitions + extensions.flatMap { it.enumValueDefinitions } mergedEnumDefinitions.forEach { - var typeSpec = TypeSpec.anonymousClassBuilder("") + val typeSpec = TypeSpec.anonymousClassBuilder("") if (it.directives.isNotEmpty()) { val (annotations, comments) = applyDirectivesJava(it.directives, config) if (!comments.isNullOrBlank()) { diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/InterfaceGenerator.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/InterfaceGenerator.kt index e0590d17e..bfadc4944 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/InterfaceGenerator.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/InterfaceGenerator.kt @@ -34,10 +34,13 @@ import javax.lang.model.element.Modifier class InterfaceGenerator(private val config: CodeGenConfig, private val document: Document) { + companion object { + private val logger: Logger = LoggerFactory.getLogger(InterfaceGenerator::class.java) + } + private val packageName = config.packageNameTypes private val typeUtils = TypeUtils(packageName, config, document) private val useInterfaceType = config.generateInterfaces - private val logger: Logger = LoggerFactory.getLogger(InterfaceGenerator::class.java) fun generate( definition: InterfaceTypeDefinition, @@ -47,13 +50,13 @@ class InterfaceGenerator(private val config: CodeGenConfig, private val document return CodeGenResult() } - logger.info("Generating type ${definition.name}") + logger.info("Generating type {}", definition.name) val javaType = TypeSpec.interfaceBuilder(definition.name) .addOptionalGeneratedAnnotation(config) .addModifiers(Modifier.PUBLIC) if (definition.description != null) { - javaType.addJavadoc(definition.description.sanitizeJavaDoc()) + javaType.addJavadoc("\$L", definition.description.content) } definition.implements @@ -119,7 +122,7 @@ class InterfaceGenerator(private val config: CodeGenConfig, private val document .addModifiers(Modifier.ABSTRACT, Modifier.PUBLIC) .returns(returnType) if (fieldDefinition.description != null) { - getterBuilder.addJavadoc(fieldDefinition.description.sanitizeJavaDoc()) + getterBuilder.addJavadoc("\$L", fieldDefinition.description.content) } javaType.addMethod(getterBuilder.build()) @@ -129,7 +132,7 @@ class InterfaceGenerator(private val config: CodeGenConfig, private val document .addParameter(returnType, ReservedKeywordSanitizer.sanitize(fieldName)) if (fieldDefinition.description != null) { - setterBuilder.addJavadoc(fieldDefinition.description.content.lines().joinToString("\n")) + setterBuilder.addJavadoc("\$L", fieldDefinition.description.content) } javaType.addMethod(setterBuilder.build()) } diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/JavaPoetUtils.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/JavaPoetUtils.kt index 1489af08a..7cc4658ed 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/JavaPoetUtils.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/java/JavaPoetUtils.kt @@ -35,7 +35,6 @@ import com.squareup.javapoet.WildcardTypeName import graphql.introspection.Introspection.TypeNameMetaFieldDef import graphql.language.ArrayValue import graphql.language.BooleanValue -import graphql.language.Description import graphql.language.EnumValue import graphql.language.FloatValue import graphql.language.IntValue @@ -44,7 +43,6 @@ import graphql.language.ObjectField import graphql.language.ObjectValue import graphql.language.StringValue import graphql.language.Value -import java.lang.IllegalArgumentException /** * Generate a [JsonTypeInfo] annotation, which allows for Jackson @@ -58,14 +56,6 @@ import java.lang.IllegalArgumentException * property = "__typename") * ``` */ - -/** - * Adds @Deprecated annotation - */ -fun deprecatedAnnotation(): AnnotationSpec { - return AnnotationSpec.builder(java.lang.Deprecated::class.java).build() -} - fun jsonTypeInfoAnnotation(): AnnotationSpec { return AnnotationSpec.builder(JsonTypeInfo::class.java) .addMember("use", "\$T.\$L", JsonTypeInfo.Id::class.java, JsonTypeInfo.Id.NAME.name) @@ -122,22 +112,6 @@ fun jsonSubTypeAnnotation(subTypes: Collection): AnnotationSpec { return annotationSpec.build() } -/** - * Javapoet treats $ as a reference - * https://github.com/square/javapoet/issues/670 - */ -fun Description.sanitizeJavaDoc(): String { - return this.content.lines().joinToString("\n").sanitizeJavaDoc() -} - -/** - * Javapoet treats $ as a reference - * https://github.com/square/javapoet/issues/670 - */ -fun String.sanitizeJavaDoc(): String { - return replace("$", "$$") -} - fun String.toTypeName(isGenericParam: Boolean = false): TypeName { val normalizedClassName = this.trim() @@ -219,11 +193,11 @@ fun customAnnotation(annotationArgumentMap: MutableMap>>, */ private fun generateCode(config: CodeGenConfig, value: Value>, annotationName: String, prefix: String = ""): CodeBlock = when (value) { - is BooleanValue -> CodeBlock.of("\$L", (value as BooleanValue).isValue) - is IntValue -> CodeBlock.of("\$L", (value as IntValue).value) + is BooleanValue -> CodeBlock.of("\$L", value.isValue) + is IntValue -> CodeBlock.of("\$L", value.value) is StringValue -> { // If string value ends with .class and classImports mapping is provided, treat as Java Class - val string = (value as StringValue).value + val string = value.value if (string.endsWith(ParserConstants.CLASS_STRING)) { val className = string.dropLast(ParserConstants.CLASS_LENGTH) // Use annotationName and className in the PackagerParserUtil to get Class Package name. @@ -232,32 +206,35 @@ private fun generateCode(config: CodeGenConfig, value: Value>, annotati else CodeBlock.of("\$S", string) } else CodeBlock.of("\$S", string) } - is FloatValue -> CodeBlock.of("\$L", (value as FloatValue).value) + is FloatValue -> CodeBlock.of("\$L", value.value) // In an enum value the prefix (key in the parameters map for the enum) is used to get the package name from the config // Limitation: Since it uses the enum key to lookup the package from the configs. 2 enums using different packages cannot have the same keys. is EnumValue -> CodeBlock.of( "\$T", - ClassName.get(PackageParserUtil.getEnumPackage(config, annotationName, prefix), (value as EnumValue).name) + ClassName.get(PackageParserUtil.getEnumPackage(config, annotationName, prefix), value.name) ) is ArrayValue -> - if ((value as ArrayValue).values.isEmpty()) CodeBlock.of("{}") - else CodeBlock.of("{\$L}", (value as ArrayValue).values.joinToString { v -> generateCode(config = config, value = v, annotationName = annotationName, prefix = if (v is EnumValue) prefix else "").toString() }) + if (value.values.isEmpty()) { + CodeBlock.of("{}") + } else { + CodeBlock.of("{\$L}", value.values.joinToString { v -> generateCode(config = config, value = v, annotationName = annotationName, prefix = if (v is EnumValue) prefix else "").toString() }) + } else -> CodeBlock.of("\$L", value) } private fun typeClassBestGuess(name: String): TypeName { return when (name) { "String" -> ClassName.get("java.lang", "String") - "Integer" -> ClassName.get("java.lang", "Integer") - "Long" -> ClassName.get("java.lang", "Long") - "Float" -> ClassName.get("java.lang", "Float") - "Double" -> ClassName.get("java.lang", "Double") - "Character" -> ClassName.get("java.lang", "Character") - "Short" -> ClassName.get("java.lang", "Short") - "Byte" -> ClassName.get("java.lang", "Byte") + "Integer" -> ClassName.INT.box() + "Long" -> ClassName.LONG.box() + "Float" -> ClassName.FLOAT.box() + "Double" -> ClassName.DOUBLE.box() + "Character" -> ClassName.CHAR.box() + "Short" -> ClassName.SHORT.box() + "Byte" -> ClassName.BYTE.box() "Number" -> ClassName.get("java.lang", "Number") - "Boolean" -> ClassName.get("java.lang", "Boolean") - "Object" -> ClassName.get("java.lang", "Object") + "Boolean" -> ClassName.BOOLEAN.box() + "Object" -> ClassName.OBJECT "BigDecimal" -> ClassName.get("java.math", "BigDecimal") "List" -> ClassName.get("java.util", "List") "ArrayList" -> ClassName.get("java.util", "ArrayList") diff --git a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt index 0b06f1e39..ff92bd0e0 100644 --- a/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt +++ b/graphql-dgs-codegen-core/src/main/kotlin/com/netflix/graphql/dgs/codegen/generators/shared/DirectivesUtils.kt @@ -25,6 +25,7 @@ import com.squareup.kotlinpoet.AnnotationSpec import graphql.language.Directive import graphql.language.StringValue import graphql.language.Value +import com.squareup.javapoet.AnnotationSpec as JavaAnnotationSpec fun createArgumentMap(directive: Directive): MutableMap>> { return directive.arguments.fold(mutableMapOf()) { argMap, argument -> @@ -60,7 +61,7 @@ fun applyDirectivesKotlin(directives: List, config: CodeGenConfig): M * @input config: code generator config * @return Pair of (map of target site and corresponding annotations) and comments */ -fun applyDirectivesJava(directives: List, config: CodeGenConfig): Pair>, String?> { +fun applyDirectivesJava(directives: List, config: CodeGenConfig): Pair>, String?> { var commentFormat: String? = null return Pair( directives.fold(mutableMapOf()) { annotations, directive -> @@ -68,7 +69,7 @@ fun applyDirectivesJava(directives: List, config: CodeGenConfig): Pai val siteTarget = if (argumentMap.containsKey(ParserConstants.SITE_TARGET)) (argumentMap[ParserConstants.SITE_TARGET] as StringValue).value.uppercase() else SiteTarget.DEFAULT.name if (directive.name == ParserConstants.CUSTOM_ANNOTATION && config.generateCustomAnnotations) { annotations[siteTarget] = if (annotations.containsKey(siteTarget)) { - var annotationList: MutableList = annotations[siteTarget]!! + var annotationList: MutableList = annotations[siteTarget]!! annotationList.add( com.netflix.graphql.dgs.codegen.generators.java.customAnnotation( argumentMap, @@ -81,7 +82,7 @@ fun applyDirectivesJava(directives: List, config: CodeGenConfig): Pai } } if (directive.name == ParserConstants.DEPRECATED && config.addDeprecatedAnnotation) { - annotations[siteTarget] = mutableListOf(com.netflix.graphql.dgs.codegen.generators.java.deprecatedAnnotation()) + annotations[siteTarget] = mutableListOf(JavaAnnotationSpec.builder(java.lang.Deprecated::class.java).build()) if (argumentMap.containsKey(ParserConstants.REASON)) { val reason: String = (argumentMap[ParserConstants.REASON] as StringValue).value val replace = reason.substringAfter(ParserConstants.REPLACE_WITH_STR, "") diff --git a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt index 0703fb515..f9d39132a 100644 --- a/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt +++ b/graphql-dgs-codegen-core/src/test/kotlin/com/netflix/graphql/dgs/codegen/CodeGenTest.kt @@ -3504,6 +3504,9 @@ It takes a title and such. Anything with a title! ""${'"'} interface Titled { + ""${'"'} + The title field; must not contain '$' + ""${'"'} title: String } """.trimIndent() @@ -3515,10 +3518,8 @@ It takes a title and such. ) ).generate() - assertThat(result.javaInterfaces[0].typeSpec.javadoc.toString()).isEqualTo( - """Anything with a title! - """.trimIndent() - ) + assertThat(result.javaInterfaces[0].typeSpec.javadoc.toString()).isEqualTo("Anything with a title!") + assertThat(result.javaInterfaces[0].typeSpec.methodSpecs[0].javadoc.toString()).isEqualTo("The title field; must not contain '\$'") } @Test @@ -3539,10 +3540,7 @@ It takes a title and such. ) ).generate() - assertThat(result.javaInterfaces[0].typeSpec.methodSpecs[0].javadoc.toString()).isEqualTo( - """The original, non localized title. - """.trimIndent() - ) + assertThat(result.javaInterfaces[0].typeSpec.methodSpecs[0].javadoc.toString()).isEqualTo("The original, non localized title.") } @Test