Skip to content

Add an option to fallback to comments if a description is not provided #564

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

Merged
merged 2 commits into from
Aug 13, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ internal class SchemaClassScanner(
?: throw SchemaClassScannerError("Expected a user-defined GraphQL scalar type with name '${definition.name}' but found none!")
GraphQLScalarType.newScalar()
.name(provided.name)
.description(definition.description?.content ?: getDocumentation(definition) ?: provided.description)
.description(getDocumentation(definition, options) ?: provided.description)
.coercing(provided.coercing)
.definition(definition)
.build()
Expand Down
19 changes: 9 additions & 10 deletions src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class SchemaParser internal constructor(
val builder = GraphQLObjectType.newObject()
.name(name)
.definition(objectDefinition)
.description(if (objectDefinition.description != null) objectDefinition.description.content else getDocumentation(objectDefinition))
.description(getDocumentation(objectDefinition, options))

builder.withDirectives(*buildDirectives(objectDefinition.directives, Introspection.DirectiveLocation.OBJECT))

Expand All @@ -133,7 +133,6 @@ class SchemaParser internal constructor(
}

objectDefinition.getExtendedFieldDefinitions(extensionDefinitions).forEach { fieldDefinition ->
fieldDefinition.description
builder.field { field ->
createField(field, fieldDefinition, inputObjects)
codeRegistryBuilder.dataFetcher(
Expand Down Expand Up @@ -163,7 +162,7 @@ class SchemaParser internal constructor(
.name(definition.name)
.definition(definition)
.extensionDefinitions(extensionDefinitions)
.description(if (definition.description != null) definition.description.content else getDocumentation(definition))
.description(getDocumentation(definition, options))

builder.withDirectives(*buildDirectives(definition.directives, Introspection.DirectiveLocation.INPUT_OBJECT))

Expand All @@ -174,7 +173,7 @@ class SchemaParser internal constructor(
val fieldBuilder = GraphQLInputObjectField.newInputObjectField()
.name(inputDefinition.name)
.definition(inputDefinition)
.description(if (inputDefinition.description != null) inputDefinition.description.content else getDocumentation(inputDefinition))
.description(getDocumentation(inputDefinition, options))
.defaultValue(buildDefaultValue(inputDefinition.defaultValue))
.type(determineInputType(inputDefinition.type, inputObjects, referencingInputObjects))
.withDirectives(*buildDirectives(inputDefinition.directives, Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
Expand All @@ -194,7 +193,7 @@ class SchemaParser internal constructor(
val builder = GraphQLEnumType.newEnum()
.name(name)
.definition(definition)
.description(if (definition.description != null) definition.description.content else getDocumentation(definition))
.description(getDocumentation(definition, options))

builder.withDirectives(*buildDirectives(definition.directives, Introspection.DirectiveLocation.ENUM))

Expand All @@ -207,7 +206,7 @@ class SchemaParser internal constructor(
getDeprecated(enumDefinition.directives).let {
val enumValueDefinition = GraphQLEnumValueDefinition.newEnumValueDefinition()
.name(enumName)
.description(if (enumDefinition.description != null) enumDefinition.description.content else getDocumentation(enumDefinition))
.description(getDocumentation(enumDefinition, options))
.value(enumValue)
.deprecationReason(it)
.withDirectives(*enumValueDirectives)
Expand All @@ -226,7 +225,7 @@ class SchemaParser internal constructor(
val builder = GraphQLInterfaceType.newInterface()
.name(name)
.definition(interfaceDefinition)
.description(if (interfaceDefinition.description != null) interfaceDefinition.description.content else getDocumentation(interfaceDefinition))
.description(getDocumentation(interfaceDefinition, options))

builder.withDirectives(*buildDirectives(interfaceDefinition.directives, Introspection.DirectiveLocation.INTERFACE))

Expand All @@ -247,7 +246,7 @@ class SchemaParser internal constructor(
val builder = GraphQLUnionType.newUnionType()
.name(name)
.definition(definition)
.description(if (definition.description != null) definition.description.content else getDocumentation(definition))
.description(getDocumentation(definition, options))

builder.withDirectives(*buildDirectives(definition.directives, Introspection.DirectiveLocation.UNION))

Expand Down Expand Up @@ -278,7 +277,7 @@ class SchemaParser internal constructor(
private fun createField(field: GraphQLFieldDefinition.Builder, fieldDefinition: FieldDefinition, inputObjects: List<GraphQLInputObjectType>): GraphQLFieldDefinition.Builder {
field
.name(fieldDefinition.name)
.description(fieldDefinition.description?.content ?: getDocumentation(fieldDefinition))
.description(getDocumentation(fieldDefinition, options))
.definition(fieldDefinition)
.apply { getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } }
.type(determineOutputType(fieldDefinition.type, inputObjects))
Expand All @@ -287,7 +286,7 @@ class SchemaParser internal constructor(
val argumentBuilder = GraphQLArgument.newArgument()
.name(argumentDefinition.name)
.definition(argumentDefinition)
.description(if (argumentDefinition.description != null) argumentDefinition.description.content else getDocumentation(argumentDefinition))
.description(getDocumentation(argumentDefinition, options))
.type(determineInputType(argumentDefinition.type, inputObjects, setOf()))
.apply { buildDefaultValue(argumentDefinition.defaultValue)?.let { defaultValue(it) } }
.withDirectives(*buildDirectives(argumentDefinition.directives, Introspection.DirectiveLocation.ARGUMENT_DEFINITION))
Expand Down
11 changes: 9 additions & 2 deletions src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ data class SchemaParserOptions internal constructor(
val coroutineContextProvider: CoroutineContextProvider,
val typeDefinitionFactories: List<TypeDefinitionFactory>,
val fieldVisibility: GraphqlFieldVisibility?,
val includeUnusedTypes: Boolean
val includeUnusedTypes: Boolean,
val useCommentsForDescriptions: Boolean
) {
companion object {
@JvmStatic
Expand Down Expand Up @@ -61,6 +62,7 @@ data class SchemaParserOptions internal constructor(
private var typeDefinitionFactories: MutableList<TypeDefinitionFactory> = mutableListOf(RelayConnectionFactory())
private var fieldVisibility: GraphqlFieldVisibility? = null
private var includeUnusedTypes = false
private var useCommentsForDescriptions = true

fun contextClass(contextClass: Class<*>) = this.apply {
this.contextClass = contextClass
Expand Down Expand Up @@ -138,6 +140,10 @@ data class SchemaParserOptions internal constructor(
this.includeUnusedTypes = includeUnusedTypes
}

fun useCommentsForDescriptions(useCommentsForDescriptions: Boolean) = this.apply {
this.useCommentsForDescriptions = useCommentsForDescriptions
}

@ExperimentalCoroutinesApi
fun build(): SchemaParserOptions {
val coroutineContextProvider = coroutineContextProvider
Expand Down Expand Up @@ -177,7 +183,8 @@ data class SchemaParserOptions internal constructor(
coroutineContextProvider,
typeDefinitionFactories,
fieldVisibility,
includeUnusedTypes
includeUnusedTypes,
useCommentsForDescriptions
)
}
}
Expand Down
15 changes: 11 additions & 4 deletions src/main/kotlin/graphql/kickstart/tools/util/Utils.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.kickstart.tools.util

import graphql.kickstart.tools.GraphQLResolver
import graphql.kickstart.tools.SchemaParserOptions
import graphql.language.*
import graphql.schema.DataFetchingEnvironment
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -49,10 +50,16 @@ internal val Class<*>.declaredNonProxyMethods: List<JavaMethod>
}
}

internal fun getDocumentation(node: AbstractNode<*>): String? = node.comments?.asSequence()
?.filter { !it.content.startsWith("#") }
?.joinToString("\n") { it.content.trimEnd() }
?.trimIndent()
internal fun getDocumentation(node: AbstractDescribedNode<*>, options: SchemaParserOptions): String? =
when {
node.description != null -> node.description.content
!options.useCommentsForDescriptions -> null
node.comments.isNullOrEmpty() -> null
else -> node.comments.asSequence()
.filter { !it.content.startsWith("#") }
.joinToString("\n") { it.content.trimEnd() }
.trimIndent()
}

/**
* Simple heuristic to check is a method is a trivial data fetcher.
Expand Down
61 changes: 61 additions & 0 deletions src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,67 @@ class SchemaParserTest {
assert(testNullableArgument.type is GraphQLInputObjectType)
}

@Test
fun `parser should use comments for descriptions`() {
val schema = SchemaParser.newParser()
.schemaString(
"""
type Query {
"description"
description: String
#comment
comment: String
omitted: String
"description"
#comment
both: String
""
empty: String
}
""")
.resolvers(object : GraphQLQueryResolver {})
.options(SchemaParserOptions.newOptions().allowUnimplementedResolvers(true).build())
.build()
.makeExecutableSchema()

val queryType = schema.getObjectType("Query")
assertEquals(queryType.getFieldDefinition("description").description, "description")
assertEquals(queryType.getFieldDefinition("comment").description, "comment")
assertNull(queryType.getFieldDefinition("omitted").description)
assertEquals(queryType.getFieldDefinition("both").description, "description")
assertEquals(queryType.getFieldDefinition("empty").description, "")
}

@Test
fun `parser should not use comments for descriptions`() {
val schema = SchemaParser.newParser()
.schemaString(
"""
type Query {
"description"
description: String
#comment
comment: String
omitted: String
"description"
#comment
both: String
""
empty: String
}
""")
.resolvers(object : GraphQLQueryResolver {})
.options(SchemaParserOptions.newOptions().useCommentsForDescriptions(false).allowUnimplementedResolvers(true).build())
.build()
.makeExecutableSchema()

assertEquals(schema.queryType.getFieldDefinition("description").description, "description")
assertNull(schema.queryType.getFieldDefinition("comment").description)
assertNull(schema.queryType.getFieldDefinition("omitted").description)
assertEquals(schema.queryType.getFieldDefinition("both").description, "description")
assertEquals(schema.queryType.getFieldDefinition("empty").description, "")
}

enum class EnumType {
TEST
}
Expand Down