From 5f947938ee7a983efaf1249f8607c69bfe4ef713 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 19 Oct 2026 14:31:25 -0400 Subject: [PATCH 1/2] Warn at build when an argument is bound to a context parameter --- .../kickstart/tools/SchemaClassScanner.kt | 28 ++++++++++ .../tools/resolver/FieldResolverScanner.kt | 2 +- .../tools/MethodFieldResolverTest.kt | 56 +++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 3d383130..3a4e144d 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -9,6 +9,7 @@ import graphql.kickstart.tools.util.* import graphql.language.* import graphql.schema.GraphQLScalarType import graphql.schema.idl.ScalarInfo +import graphql.schema.idl.TypeUtil import org.apache.commons.lang3.ClassUtils import org.apache.commons.lang3.reflect.TypeUtils import org.slf4j.LoggerFactory @@ -528,6 +529,7 @@ internal class SchemaClassScanner( fieldResolver.scanForMatches().forEach { potentialMatch -> handleFoundType(typeClassMatcher.match(potentialMatch)) } + warnIfArgumentPassedToContext(type, fieldResolver) } } @@ -552,6 +554,32 @@ internal class SchemaClassScanner( ) } + // TODO throw a FieldResolverError instead in 15.0.0 (#889) + private fun warnIfArgumentPassedToContext(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { + if (fieldResolver !is MethodFieldResolver) { + return + } + + fieldResolver.field.inputValueDefinitions.forEachIndexed { index, argument -> + val parameter = fieldResolver.getMethodParameterType(index)?.let(fieldResolver.typeResolver::getRawClass) ?: return@forEachIndexed + if (!isContextParameter(parameter, argument)) { + return@forEachIndexed + } + + val method = fieldResolver.method + log.warn( + "Field '${type.name}.${fieldResolver.field.name}' passes its argument '${argument.name}: ${TypeUtil.simplePrint(argument.type)}' " + + "to the ${parameter.name} parameter of ${method.declaringClass.name}.${method.name}, so the field fails or gets null for " + + "that parameter. Declare one parameter per argument, in order, before the ${parameter.simpleName} parameter." + ) + } + } + + // the context class can also be an argument's type (e.g. a Map for an input object), so only spec scalars count for it + private fun isContextParameter(parameter: Class<*>, argument: InputValueDefinition): Boolean = + parameter in fieldResolverScanner.allowedTrailingParameterTypes && + (parameter != options.contextClass || argument.type.unwrap().name in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS) + private fun handleFoundType(match: TypeClassMatcher.Match) = when (match) { is TypeClassMatcher.ScalarMatch -> handleFoundScalarType(match.type) is TypeClassMatcher.ValidMatch -> handleFoundType(match.type, match.javaType, match.reference) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index 0c54fee2..d2e11815 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -25,7 +25,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private val log = LoggerFactory.getLogger(javaClass) - private val allowedTrailingParameterTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass) + val allowedTrailingParameterTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass) private val methodsByNameCache = mutableMapOf, Boolean>, Map>>() diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index b630d0da..4aa2538e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -10,6 +10,7 @@ import graphql.language.Value import graphql.schema.Coercing import graphql.schema.CoercingParseLiteralException import graphql.schema.CoercingSerializeException +import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLScalarType import org.junit.Test import java.lang.reflect.InvocationHandler @@ -283,6 +284,61 @@ class MethodFieldResolverTest { )) } + @Test + fun `should warn when an argument is passed to a context parameter`() { + val (_, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + context(id: ID!, persist: Boolean!): String + graphQLContext(persist: Boolean, id: ID): String + image: Image + input(input: Input): String + extra(persist: Boolean): String + } + + type Image { + thumbnail(size: Int): String + } + + input Input { + value: String + } + """) + .resolvers(ContextParameterQuery(), ImageResolver()) + .options(SchemaParserOptions.newOptions().contextClass(Map::class).build()) + .build() + .makeExecutableSchema() + } + + val query = ContextParameterQuery::class.java.name + assertEquals(warnings, listOf( + "Field 'Query.context' passes its argument 'persist: Boolean!' to the java.util.Map parameter of $query.context, so the field " + + "fails or gets null for that parameter. Declare one parameter per argument, in order, before the Map parameter.", + "Field 'Query.graphQLContext' passes its argument 'persist: Boolean' to the graphql.GraphQLContext parameter of " + + "$query.graphQLContext, so the field fails or gets null for that parameter. Declare one parameter per argument, in order, " + + "before the GraphQLContext parameter.", + "Field 'Image.thumbnail' passes its argument 'size: Int' to the graphql.schema.DataFetchingEnvironment parameter of " + + "${ImageResolver::class.java.name}.thumbnail, so the field fails or gets null for that parameter. Declare one parameter per " + + "argument, in order, before the DataFetchingEnvironment parameter." + )) + } + + class ContextParameterQuery : GraphQLQueryResolver { + fun context(id: String, context: Map) = "$id $context" + fun graphQLContext(context: GraphQLContext?, id: String?) = "$context $id" + fun image() = Image() + fun input(input: Map?) = input.toString() + fun extra(persist: Boolean?, context: Map) = "$persist $context" + } + + class Image + + class ImageResolver : GraphQLResolver { + fun thumbnail(image: Image, env: DataFetchingEnvironment?) = env.toString() + } + sealed class Maybe { data class Some(val value: T) : Maybe() object None : Maybe() From 57151da4d186164336b6f7be71ecb243024e12b0 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 19 Oct 2026 18:19:43 -0400 Subject: [PATCH 2/2] Word the warning around the missing parameter --- .../kickstart/tools/SchemaClassScanner.kt | 10 ++--- .../tools/MethodFieldResolverTest.kt | 43 +++++++++---------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 3a4e144d..eec6d8fa 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -529,7 +529,7 @@ internal class SchemaClassScanner( fieldResolver.scanForMatches().forEach { potentialMatch -> handleFoundType(typeClassMatcher.match(potentialMatch)) } - warnIfArgumentPassedToContext(type, fieldResolver) + warnIfParameterMissing(type, fieldResolver) } } @@ -555,7 +555,7 @@ internal class SchemaClassScanner( } // TODO throw a FieldResolverError instead in 15.0.0 (#889) - private fun warnIfArgumentPassedToContext(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { + private fun warnIfParameterMissing(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { if (fieldResolver !is MethodFieldResolver) { return } @@ -568,9 +568,9 @@ internal class SchemaClassScanner( val method = fieldResolver.method log.warn( - "Field '${type.name}.${fieldResolver.field.name}' passes its argument '${argument.name}: ${TypeUtil.simplePrint(argument.type)}' " + - "to the ${parameter.name} parameter of ${method.declaringClass.name}.${method.name}, so the field fails or gets null for " + - "that parameter. Declare one parameter per argument, in order, before the ${parameter.simpleName} parameter." + "Field '${type.name}.${fieldResolver.field.name}' has no parameter for its argument '${argument.name}: " + + "${TypeUtil.simplePrint(argument.type)}' in ${method.declaringClass.name}.${method.name}, so the ${parameter.simpleName} " + + "parameter gets the argument instead. Add a parameter for each argument, in order, before the ${parameter.simpleName} parameter." ) } } diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index 4aa2538e..39130950 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -285,52 +285,51 @@ class MethodFieldResolverTest { } @Test - fun `should warn when an argument is passed to a context parameter`() { + fun `should warn when a resolver method is missing a parameter for an argument`() { val (_, warnings) = withWarnings { SchemaParser.newParser() .schemaString( """ type Query { - context(id: ID!, persist: Boolean!): String - graphQLContext(persist: Boolean, id: ID): String + user(id: ID!, active: Boolean!): String + users(active: Boolean, first: Int): String image: Image - input(input: Input): String - extra(persist: Boolean): String + search(filter: Filter): String + count(active: Boolean): String } type Image { thumbnail(size: Int): String } - input Input { - value: String + input Filter { + name: String } """) - .resolvers(ContextParameterQuery(), ImageResolver()) + .resolvers(MissingParameterQuery(), ImageResolver()) .options(SchemaParserOptions.newOptions().contextClass(Map::class).build()) .build() .makeExecutableSchema() } - val query = ContextParameterQuery::class.java.name + val query = MissingParameterQuery::class.java.name assertEquals(warnings, listOf( - "Field 'Query.context' passes its argument 'persist: Boolean!' to the java.util.Map parameter of $query.context, so the field " + - "fails or gets null for that parameter. Declare one parameter per argument, in order, before the Map parameter.", - "Field 'Query.graphQLContext' passes its argument 'persist: Boolean' to the graphql.GraphQLContext parameter of " + - "$query.graphQLContext, so the field fails or gets null for that parameter. Declare one parameter per argument, in order, " + - "before the GraphQLContext parameter.", - "Field 'Image.thumbnail' passes its argument 'size: Int' to the graphql.schema.DataFetchingEnvironment parameter of " + - "${ImageResolver::class.java.name}.thumbnail, so the field fails or gets null for that parameter. Declare one parameter per " + - "argument, in order, before the DataFetchingEnvironment parameter." + "Field 'Query.user' has no parameter for its argument 'active: Boolean!' in $query.user, so the Map parameter gets the " + + "argument instead. Add a parameter for each argument, in order, before the Map parameter.", + "Field 'Query.users' has no parameter for its argument 'active: Boolean' in $query.users, so the GraphQLContext parameter " + + "gets the argument instead. Add a parameter for each argument, in order, before the GraphQLContext parameter.", + "Field 'Image.thumbnail' has no parameter for its argument 'size: Int' in ${ImageResolver::class.java.name}.thumbnail, so the " + + "DataFetchingEnvironment parameter gets the argument instead. Add a parameter for each argument, in order, before the " + + "DataFetchingEnvironment parameter." )) } - class ContextParameterQuery : GraphQLQueryResolver { - fun context(id: String, context: Map) = "$id $context" - fun graphQLContext(context: GraphQLContext?, id: String?) = "$context $id" + class MissingParameterQuery : GraphQLQueryResolver { + fun user(id: String, context: Map) = "$id $context" + fun users(context: GraphQLContext?, first: Int?) = "$context $first" fun image() = Image() - fun input(input: Map?) = input.toString() - fun extra(persist: Boolean?, context: Map) = "$persist $context" + fun search(filter: Map?) = filter.toString() + fun count(active: Boolean?, context: Map) = "$active $context" } class Image