Skip to content

Commit 910756f

Browse files
oryan-blockclaude
andcommitted
Simplify the input field scan and the type error messages
- Fix the two compiler warnings for the java.util.Map and Object class literals in isPropertyMap - Flatten handleInputObjectFields with early returns - Give the three enum-without-class messages the same fix hint, so only the reason differs - Pick the type kind in unexpectedTypeError without a triple negation - Write the source name as "in <file>", like the syntax errors from #838 - Drop "javax." from the library class prefixes, nothing there is an input class - Shorten the missing property warning - Fold the ObjectNode method name test and the Map subclass enum error test into their neighbours Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b4cd4f0 commit 910756f

4 files changed

Lines changed: 81 additions & 132 deletions

File tree

‎src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,9 @@ internal class SchemaClassScanner(
220220
}
221221

222222
private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects {
223-
enumsWithoutClass
224-
.filter { dictionary[it.key]?.javaType == null }
225-
.values.firstOrNull()
226-
?.let { throw SchemaClassScannerError(it) }
223+
enumsWithoutClass.entries
224+
.find { dictionary[it.key]?.javaType == null }
225+
?.let { throw SchemaClassScannerError(it.value) }
227226

228227
initialDictionary
229228
.filter { !it.value.accessed }
@@ -471,27 +470,30 @@ internal class SchemaClassScanner(
471470
* has no known class, e.g. because it's only reachable through a Map.
472471
*/
473472
private fun handleInputObjectFields(graphQLType: InputObjectTypeDefinition, javaType: JavaType?) {
473+
val clazz = javaType?.unwrap()
474474
val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name }
475475
inputObjectTypes
476476
.flatMap { it.inputValueDefinitions }
477477
.forEach { inputValueDefinition ->
478478
val inputGraphQLType = inputValueDefinition.type.unwrap()
479-
if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) {
480-
val propertyType = javaType?.unwrap()?.let { findInputValueTypeInType(inputValueDefinition.name, it) }
481-
val inputValueJavaType = propertyType ?: initialDictionary[inputGraphQLType.name]?.get()
482-
if (inputValueJavaType != null) {
483-
val genericContext = javaType ?: inputValueJavaType
484-
handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType(
485-
// a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper
486-
if (propertyType != null) inputValueDefinition.type else inputGraphQLType,
487-
inputValueJavaType,
488-
GenericTypeResolver(genericContext, options),
489-
InputObjectReference(inputValueDefinition)
490-
)))
491-
} else {
492-
handleInputValueWithoutJavaType(graphQLType, inputValueDefinition, inputGraphQLType, javaType?.unwrap())
493-
}
479+
if (inputGraphQLType !is TypeName || ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) {
480+
return@forEach
481+
}
482+
483+
val propertyType = clazz?.let { findInputValueTypeInType(inputValueDefinition.name, it) }
484+
val inputValueJavaType = propertyType ?: initialDictionary[inputGraphQLType.name]?.get()
485+
if (inputValueJavaType == null) {
486+
handleInputValueWithoutJavaType(graphQLType, inputValueDefinition, inputGraphQLType, clazz)
487+
return@forEach
494488
}
489+
490+
handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType(
491+
// a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper
492+
if (propertyType != null) inputValueDefinition.type else inputGraphQLType,
493+
inputValueJavaType,
494+
GenericTypeResolver(javaType ?: inputValueJavaType, options),
495+
InputObjectReference(inputValueDefinition)
496+
)))
495497
}
496498
}
497499

@@ -502,8 +504,7 @@ internal class SchemaClassScanner(
502504
private fun handleInputValueWithoutJavaType(graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, inputGraphQLType: TypeName, clazz: Class<*>?) {
503505
if (clazz != null && !isPropertyMap(clazz)) {
504506
log.warn("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}. " +
505-
"Unless Jackson maps it some other way, e.g. through @JsonProperty, check that the property name matches the schema, " +
506-
"otherwise the field's value is rejected or dropped when the input is deserialized.")
507+
"If Jackson doesn't map it some other way, e.g. through @JsonProperty, the field's value is rejected or dropped when the input is deserialized.")
507508
}
508509

509510
when (val definition = definitionsByName[inputGraphQLType.name]) {
@@ -523,23 +524,22 @@ internal class SchemaClassScanner(
523524
}
524525
}
525526

527+
// an input bound to Any gets a Map at runtime too
526528
private fun isPropertyMap(clazz: Class<*>) =
527-
java.util.Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Object::class.java
529+
Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Any::class.java
528530

529531
private fun isLibraryClass(clazz: Class<*>) =
530-
listOf("java.", "javax.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) }
532+
listOf("java.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) }
531533

532-
private fun enumWithoutClassMessage(enum: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String {
534+
private fun enumWithoutClassMessage(enumType: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String {
533535
val field = inputValueDefinition.name
534-
val problem = "Enum type '${enum.name}' is used by field '$field' of input type '${graphQLType.name}', but its Java enum can't be determined"
535-
return when {
536-
clazz == null -> "$problem because no class was found for input type '${graphQLType.name}'. " +
537-
"Pass the enum class for type '${enum.name}' in the parser's dictionary, or a class for '${graphQLType.name}' with a public getter or field '$field'."
538-
isPropertyMap(clazz) && isLibraryClass(clazz) -> "$problem because '${graphQLType.name}' is bound to ${clazz.name}, which has no type information for its fields. " +
539-
"Pass the enum class for type '${enum.name}' in the parser's dictionary, or use a class for '${graphQLType.name}' with a public getter or field '$field'."
540-
else -> "$problem because ${clazz.name} has no public getter or field named '$field'. " +
541-
"Add a public getter or field '$field' to the class used for '${graphQLType.name}', or pass the enum class for type '${enum.name}' in the parser's dictionary."
536+
val reason = when {
537+
clazz == null -> "no class was found for input type '${graphQLType.name}'"
538+
isPropertyMap(clazz) && isLibraryClass(clazz) -> "'${graphQLType.name}' is bound to ${clazz.name}, which has no type information for its fields"
539+
else -> "${clazz.name} has no public getter or field named '$field'"
542540
}
541+
return "Enum type '${enumType.name}' is used by field '$field' of input type '${graphQLType.name}', but its Java enum can't be determined because $reason. " +
542+
"Pass the enum class for type '${enumType.name}' in the parser's dictionary, or use a class for '${graphQLType.name}' with a public getter or field '$field'."
543543
}
544544

545545
private fun scanInterfacesOfType(graphQLType: ObjectTypeDefinition) {

‎src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -574,32 +574,33 @@ class SchemaParser internal constructor(
574574
}
575575

576576
private fun unexpectedTypeError(typeName: TypeName, expectedType: KClass<*>): SchemaError {
577-
val name = typeName.name
578-
val subject = "Type '$name'" +
579-
(findUsage(typeName)?.let { " used by $it" } ?: "") +
580-
(typeName.sourceLocation?.let { " at line ${it.line}, column ${it.column}" + (it.sourceName?.let { source -> " of $source" } ?: "") } ?: "")
581-
// unused ones first, since the extensions of an unused type are still among the definitions
582-
val definition = unusedDefinitions.find { it.name == name }
583-
?: definitions.filterIsInstance<TypeDefinition<*>>().find { it.name == name }
584577
val isInput = expectedType == GraphQLInputType::class
585-
val kind = when (definition) {
586-
is InputObjectTypeDefinition -> "an input object type"
578+
// unused ones first, since the extensions of an unused type are still among the definitions
579+
val definition = unusedDefinitions.find { it.name == typeName.name }
580+
?: definitions.filterIsInstance<TypeDefinition<*>>().find { it.name == typeName.name }
581+
val outputOnlyKind = when (definition) {
582+
is ObjectTypeDefinition -> "an object type"
587583
is InterfaceTypeDefinition -> "an interface"
588584
is UnionTypeDefinition -> "a union"
589-
else -> "an object type"
585+
else -> null
590586
}
591587

592-
return SchemaError(when {
593-
definition == null -> "$subject is not defined in the schema."
594-
isInput && definition !is InputObjectTypeDefinition && definition !is EnumTypeDefinition && definition !is ScalarTypeDefinition ->
595-
"$subject is $kind, which can't be used as an input type. Input types are input objects, enums and scalars."
588+
val problem = when {
589+
definition == null -> "is not defined in the schema."
590+
isInput && outputOnlyKind != null ->
591+
"is $outputOnlyKind, which can't be used as an input type. Input types are input objects, enums and scalars."
596592
!isInput && definition is InputObjectTypeDefinition ->
597-
"$subject is $kind, which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars."
593+
"is an input object type, which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars."
598594
definition in unusedDefinitions ->
599-
"$subject is defined in the schema, but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " +
595+
"is defined in the schema, but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " +
600596
unreachedTypeHint(definition)
601-
else -> "$subject can't be used as ${if (isInput) "an input" else "an output"} type."
602-
})
597+
else -> "can't be used as ${if (isInput) "an input" else "an output"} type."
598+
}
599+
val usage = findUsage(typeName)?.let { " used by $it" }.orEmpty()
600+
val location = typeName.sourceLocation
601+
?.let { " at line ${it.line}, column ${it.column}" + it.sourceName?.let { name -> " in $name" }.orEmpty() }
602+
.orEmpty()
603+
return SchemaError("Type '${typeName.name}'$usage$location $problem")
603604
}
604605

605606
// what makes the scanner reach the type depends on its kind, see SchemaClassScanner.canIncludeUnusedType

‎src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt‎

Lines changed: 6 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,10 @@ class NestedInputTypesTest {
172172
173173
input EntityUpdateInput {
174174
id: ID!
175-
nested: Nested
175+
# named like methods of ObjectNode
176+
values: [Nested]
177+
elements: [Nested]
178+
fields: [Nested]
176179
}
177180
178181
input Nested {
@@ -193,13 +196,13 @@ class NestedInputTypesTest {
193196
"""
194197
mutation {
195198
create(input: { name: "n", nested: { x: 1 } })
196-
update(input: { id: "2", nested: { x: 3 } })
199+
update(input: { id: "2", values: [{ x: 3 }], elements: [{ x: 4 }], fields: [{ x: 5 }] })
197200
}
198201
"""
199202
}
200203

201204
assertEquals(data["create"], """{"name":"n","nested":{"x":1}}""")
202-
assertEquals(data["update"], """{"id":"2","nested":{"x":3}}""")
205+
assertEquals(data["update"], """{"id":"2","values":[{"x":3}],"elements":[{"x":4}],"fields":[{"x":5}]}""")
203206
}
204207

205208
@Test
@@ -337,43 +340,6 @@ class NestedInputTypesTest {
337340
assertEquals(data["byClass"], "b, c")
338341
}
339342

340-
@Test
341-
fun `input fields bound to ObjectNode are not matched against its own methods`() {
342-
val schema = SchemaParser.newParser()
343-
.schemaString(
344-
"""
345-
type Query {
346-
create(input: EntityInput!): String
347-
}
348-
349-
input EntityInput {
350-
values: [Nested]
351-
elements: [Nested]
352-
fields: [Nested]
353-
}
354-
355-
input Nested {
356-
x: Int
357-
}
358-
""")
359-
.resolvers(object : GraphQLQueryResolver {
360-
fun create(input: ObjectNode): String = input.toString()
361-
})
362-
.build()
363-
.makeExecutableSchema()
364-
val gql = GraphQL.newGraphQL(schema).build()
365-
366-
val data = assertNoGraphQlErrors(gql) {
367-
"""
368-
query {
369-
create(input: { values: [{ x: 1 }], elements: [{ x: 2 }], fields: [{ x: 3 }] })
370-
}
371-
"""
372-
}
373-
374-
assertEquals(data["create"], """{"values":[{"x":1}],"elements":[{"x":2}],"fields":[{"x":3}]}""")
375-
}
376-
377343
@Test
378344
fun `input field types are found through the getters of a Map subclass`() {
379345
val schema = SchemaParser.newParser()

‎src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt‎

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ class SchemaParserTest {
283283
.makeExecutableSchema()
284284
}
285285

286-
assertEquals(error.message, "Type 'Cat' used by field 'Pet.friend' at line 6, column 11 of UnreachedType.graphqls is defined in the schema, " +
286+
assertEquals(error.message, "Type 'Cat' used by field 'Pet.friend' at line 6, column 11 in UnreachedType.graphqls is defined in the schema, " +
287287
"but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " +
288288
"Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass a class for it in the parser's dictionary.")
289289
}
@@ -572,60 +572,42 @@ class SchemaParserTest {
572572

573573
@Test
574574
fun `parser should throw descriptive exception when enum is only used in input Map without a class`() {
575-
val error = assertThrows(SchemaClassScannerError::class.java) {
576-
SchemaParser.newParser()
577-
.schemaString(
578-
"""
579-
type Query {
580-
save(input: SaveInput!): Boolean
581-
}
575+
val schema = """
576+
type Query {
577+
save(input: SaveInput!): Boolean
578+
}
582579
583-
input SaveInput {
584-
type: EnumType!
585-
}
580+
input SaveInput {
581+
type: EnumType!
582+
}
586583
587-
enum EnumType {
588-
TEST
589-
}
590-
""")
584+
enum EnumType {
585+
TEST
586+
}
587+
"""
588+
val mapError = assertThrows(SchemaClassScannerError::class.java) {
589+
SchemaParser.newParser()
590+
.schemaString(schema)
591591
.resolvers(object : GraphQLQueryResolver {
592592
fun save(input: Map<*, *>): Boolean = false
593593
})
594594
.build()
595595
}
596-
597-
assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " +
598-
"because 'SaveInput' is bound to java.util.Map, which has no type information for its fields. Pass the enum class for type " +
599-
"'EnumType' in the parser's dictionary, or use a class for 'SaveInput' with a public getter or field 'type'.")
600-
}
601-
602-
@Test
603-
fun `parser should throw descriptive exception when enum is only used in input Map subclass without a getter for it`() {
604-
val error = assertThrows(SchemaClassScannerError::class.java) {
596+
val subclassError = assertThrows(SchemaClassScannerError::class.java) {
605597
SchemaParser.newParser()
606-
.schemaString(
607-
"""
608-
type Query {
609-
save(input: SaveInput!): Boolean
610-
}
611-
612-
input SaveInput {
613-
type: EnumType!
614-
}
615-
616-
enum EnumType {
617-
TEST
618-
}
619-
""")
598+
.schemaString(schema)
620599
.resolvers(object : GraphQLQueryResolver {
621600
fun save(input: SaveInputMap): Boolean = false
622601
})
623602
.build()
624603
}
625604

626-
assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " +
627-
"because ${SaveInputMap::class.java.name} has no public getter or field named 'type'. Add a public getter or field 'type' to the class " +
628-
"used for 'SaveInput', or pass the enum class for type 'EnumType' in the parser's dictionary.")
605+
assertEquals(mapError.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " +
606+
"because 'SaveInput' is bound to java.util.Map, which has no type information for its fields. Pass the enum class for type " +
607+
"'EnumType' in the parser's dictionary, or use a class for 'SaveInput' with a public getter or field 'type'.")
608+
assertEquals(subclassError.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " +
609+
"because ${SaveInputMap::class.java.name} has no public getter or field named 'type'. Pass the enum class for type " +
610+
"'EnumType' in the parser's dictionary, or use a class for 'SaveInput' with a public getter or field 'type'.")
629611
}
630612

631613
@Test
@@ -658,7 +640,7 @@ class SchemaParserTest {
658640

659641
assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'NestedInput', but its Java enum can't be determined " +
660642
"because no class was found for input type 'NestedInput'. Pass the enum class for type 'EnumType' in the parser's dictionary, " +
661-
"or a class for 'NestedInput' with a public getter or field 'type'.")
643+
"or use a class for 'NestedInput' with a public getter or field 'type'.")
662644
}
663645

664646
@Test

0 commit comments

Comments
 (0)