Repository: graphql-java-kickstart/graphql-java-tools
Pull request #564 (2021-08-13 19:33:47)

Add an option to fallback to comments if a description is not provided
## Checklist
- [x] Pull requests follows the [contribution guide](https://github.com/graphql-java-kickstart/graphql-java-tools/wiki/Contribution-guide)
- [x] New or modified functionality is covered by tests

## Description
The existing behavior is to always fallback to comments from the schema. This change would add a configuration option to `SchemaParserOptions` that would make it possible to ignore comments and exclusively use descriptions from the schema.

For example:
```graphql
# Copyright 2020
# Authors: ...

type Query {
    hello: String!
}
```
This schema with the current behavior would result in the description being `Copyright 2020\nAuthors: ...`.
With the `commentsAsFallbackDescription` option set as false, the description will be null.



Patch:
diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
index b6f7d34..945e2fc 100644
--- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
+++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
@@ -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()
diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt
index 6b92d7e..46c2be2 100644
--- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt
+++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt
@@ -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))
 
@@ -133,7 +133,6 @@ class SchemaParser internal constructor(
         }
 
         objectDefinition.getExtendedFieldDefinitions(extensionDefinitions).forEach { fieldDefinition ->
-            fieldDefinition.description
             builder.field { field ->
                 createField(field, fieldDefinition, inputObjects)
                 codeRegistryBuilder.dataFetcher(
@@ -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))
 
@@ -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))
@@ -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))
 
@@ -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)
@@ -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))
 
@@ -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))
 
@@ -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))
@@ -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))
diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt
index 0f290e5..c161504 100644
--- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt
+++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt
@@ -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
@@ -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
@@ -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
@@ -177,7 +183,8 @@ data class SchemaParserOptions internal constructor(
                 coroutineContextProvider,
                 typeDefinitionFactories,
                 fieldVisibility,
-                includeUnusedTypes
+                includeUnusedTypes,
+                useCommentsForDescriptions
             )
         }
     }
diff --git a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt
index 7ff5b50..55efc2c 100644
--- a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt
+++ b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt
@@ -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
@@ -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.


Test patch:
diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt
index 263745b..0a4b892 100644
--- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt
+++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt
@@ -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
     }