Warn at build when an argument is bound to a context parameter

Author: oryan-block

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)

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<Pair<Class<out Any>, Boolean>, Map<String, List<Method>>>()
 

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<String, Any>) = "$id $context"
+        fun graphQLContext(context: GraphQLContext?, id: String?) = "$context $id"
+        fun image() = Image()
+        fun input(input: Map<String, Any>?) = input.toString()
+        fun extra(persist: Boolean?, context: Map<String, Any>) = "$persist $context"
+    }
+
+    class Image
+
+    class ImageResolver : GraphQLResolver<Image> {
+        fun thumbnail(image: Image, env: DataFetchingEnvironment?) = env.toString()
+    }
+
     sealed class Maybe<out T> {
         data class Some<T>(val value: T) : Maybe<T>()
         object None : Maybe<Nothing>()