Skip to content

Commit 2bfca9a

Browse files
oryan-blockclaude
oryan-block
and
claude
committed
Resolve type variables against the most specific type
Type variables in field and method types were resolved against the declaring type, which only knows the arguments passed in by its direct subclass. When those were type variables too (e.g. Item extends BaseItem<Long> extends AbstractItem<T>), the fallback matched them by name against the first parameterized superclass. That recursed forever when the names repeated, failed with "No type variable found" when they differed, and silently picked the wrong argument when a subclass reordered them. Resolve them against the most specific type instead, which binds the type variables of all its supertypes, or its owner types for those of outer classes, and drop the name matching. Variables in wildcard bounds (e.g. List<? extends T>, which Kotlin emits for List<T> parameters) are resolved too. A variable that can't be resolved, such as one declared by a generic method, fails with a clear error. One bound to a type containing itself, which can only leak out of a raw type, is erased like the raw type would instead of being expanded endlessly. Generic types returned by methods of a generic superclass now keep their resolved arguments, as fields already did. So two subclasses binding them differently can't share one GraphQL type anymore, like direct references to Page<A> and Page<B>. The "Two different classes" error now prints the parameterized types to show the difference. Fixes #460 Fixes #218 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 349d56a commit 2bfca9a

5 files changed

Lines changed: 597 additions & 45 deletions

File tree

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

Lines changed: 19 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import com.fasterxml.classmate.ResolvedType
44
import graphql.kickstart.tools.util.JavaType
55
import graphql.kickstart.tools.util.ParameterizedTypeImpl
66
import graphql.kickstart.tools.util.Primitives
7-
import graphql.kickstart.tools.util.unwrap
87
import org.apache.commons.lang3.reflect.TypeUtils
98
import java.lang.reflect.ParameterizedType
109
import java.lang.reflect.TypeVariable
@@ -97,51 +96,24 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec
9796
val unwrapsTo = genericType.schemaWrapper.invoke(typeArguments[genericType.index])
9897
return unwrapGenericType(unwrapsTo)
9998
}
100-
is TypeVariable<*> -> {
101-
val parameterizedDeclaringType = parameterizedDeclaringTypeOrSuperType(declaringType)
102-
if (parameterizedDeclaringType != null) {
103-
unwrapGenericType(parameterizedDeclaringType, type)
104-
} else {
105-
error("Could not resolve type variable '${TypeUtils.toLongString(type)}' because declaring type is not parameterized: ${TypeUtils.toString(declaringType)}")
106-
}
107-
}
99+
is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' of ${TypeUtils.toString(declaringType)} relative to ${TypeUtils.toString(mostSpecificType)}")
108100
is WildcardType -> type.upperBounds.firstOrNull()
109101
?: error("Unable to unwrap type, wildcard has no upper bound: $type")
110102
is Class<*> -> if (type.isPrimitive) Primitives.wrap(type) else type
111103
else -> error("Unable to unwrap type: $type")
112104
}
113105
}
114106

115-
private fun parameterizedDeclaringTypeOrSuperType(declaringType: JavaType): ParameterizedType? =
116-
if (declaringType is ParameterizedType) {
117-
declaringType
118-
} else {
119-
val superclass = declaringType.unwrap().genericSuperclass
120-
if (superclass != null) {
121-
parameterizedDeclaringTypeOrSuperType(superclass)
122-
} else {
123-
null
124-
}
125-
}
126-
127-
private fun unwrapGenericType(declaringType: ParameterizedType, type: TypeVariable<*>): JavaType {
128-
val rawClass = getRawClass(mostSpecificType)
129-
val arguments = TypeUtils.determineTypeArguments(rawClass, declaringType)
130-
val matchingType = arguments
131-
.filter { it.key.name == type.name }
132-
.values
133-
.firstOrNull()
134-
?: error("No type variable found for: ${TypeUtils.toLongString(type)}")
135-
136-
return unwrapGenericType(matchingType)
137-
}
138-
139-
private fun replaceTypeVariable(type: JavaType): JavaType {
107+
private fun replaceTypeVariable(type: JavaType, resolving: Set<TypeVariable<*>> = emptySet()): JavaType {
140108
return when (type) {
141109
is ParameterizedType -> {
142-
val actualTypeArguments = type.actualTypeArguments.map { replaceTypeVariable(it) }.toTypedArray()
143-
ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType)
110+
val actualTypeArguments = type.actualTypeArguments.map { replaceTypeVariable(it, resolving) }.toTypedArray()
111+
ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType?.let { replaceTypeVariable(it, resolving) })
144112
}
113+
is WildcardType -> TypeUtils.wildcardType()
114+
.withUpperBounds(*type.upperBounds.map { replaceTypeVariable(it, resolving) }.toTypedArray())
115+
.withLowerBounds(*type.lowerBounds.map { replaceTypeVariable(it, resolving) }.toTypedArray())
116+
.build()
145117
is ResolvedType -> {
146118
if (type.typeParameters.isEmpty()) {
147119
type.erasedType
@@ -152,14 +124,18 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec
152124
}
153125
is TypeVariable<*> -> {
154126
val genericDeclaration = type.genericDeclaration
155-
if (declaringType is ParameterizedType && genericDeclaration is Class<*>) {
156-
// keep the full type argument (e.g. List<Foo>) rather than its raw class so nested generics aren't lost
157-
TypeUtils.getTypeArguments(declaringType, genericDeclaration)?.get(type)
158-
?.takeIf { it != type }
159-
?.let { replaceTypeVariable(it) }
127+
when {
128+
// only a variable leaked from a raw type can be bound to a type containing itself (e.g. T -> List<T>),
129+
// erase it like the raw type does instead of expanding it forever
130+
type in resolving -> TypeUtils.getRawType(type.bounds.first(), null) ?: Any::class.java
131+
// the most specific type binds the variables of its supertypes and its owner types those of outer classes,
132+
// the declaring type may not
133+
genericDeclaration is Class<*> -> generateSequence(mostSpecificType) { (it as? ParameterizedType)?.ownerType }
134+
.firstNotNullOfOrNull { TypeUtils.getTypeArguments(it, genericDeclaration)?.get(type) }
135+
// keep the full type argument (e.g. List<Foo>) rather than its raw class so nested generics aren't lost
136+
?.let { replaceTypeVariable(it, resolving + type) }
160137
?: type
161-
} else {
162-
type
138+
else -> type
163139
}
164140
}
165141
else -> {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ internal class SchemaClassScanner(
360360
if (java.util.Map::class.java.isAssignableFrom(javaType.unwrap())) {
361361
throw SchemaClassScannerError("Two different property map classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType}:\n| ${reference.getDescription()}")
362362
}
363-
throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.unwrap()}:\n| ${reference.getDescription()}")
363+
throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.typeName}:\n| ${reference.getDescription()}")
364364
}
365365
}
366366
}
@@ -469,7 +469,7 @@ internal class SchemaClassScanner(
469469
references.add(reference)
470470
}
471471

472-
fun joinReferences() = "- ${typeClass()}:\n| " + references.joinToString("\n| ") { it.getDescription() }
472+
fun joinReferences() = "- ${javaType?.typeName}:\n| " + references.joinToString("\n| ") { it.getDescription() }
473473

474474
fun hasResolverRef(): Boolean {
475475
references.filterIsInstance<ReturnValueReference>().forEach { reference ->

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,39 @@ class GenericInputTypesTest {
166166
assertEquals(data["audit"], "why:op:1")
167167
}
168168

169+
@Test
170+
fun `generic list fields inherited from parameterized superclasses are parsed`() {
171+
val schema = SchemaParser.newParser()
172+
.schemaString(
173+
"""
174+
type Query {
175+
mutate(input: MutationInput!): String!
176+
}
177+
178+
input MutationInput {
179+
items: [Item!]!
180+
}
181+
182+
input Item {
183+
prop: String!
184+
}
185+
""")
186+
.resolvers(InheritedListQueryResolver())
187+
.build()
188+
.makeExecutableSchema()
189+
val gql = GraphQL.newGraphQL(schema).build()
190+
191+
val data = assertNoGraphQlErrors(gql) {
192+
"""
193+
query {
194+
mutate(input: { items: [{ prop: "a" }, { prop: "b" }] })
195+
}
196+
"""
197+
}
198+
199+
assertEquals(data["mutate"], "a,b")
200+
}
201+
169202
class QueryResolver : GraphQLQueryResolver {
170203
fun audit(input: AuditWrapper<LanguageInput>): String = "${input.operator}:${input.content?.id}"
171204
fun audits(input: AuditWrapper<List<LanguageInput>>): String = "${input.operator}:${input.content?.joinToString(",") { it.id.orEmpty() }}"
@@ -184,6 +217,10 @@ class GenericInputTypesTest {
184217
fun audit(input: AuditRequest): String = "${input.reason}:${input.audit?.operator}:${input.audit?.content?.id}"
185218
}
186219

220+
class InheritedListQueryResolver : GraphQLQueryResolver {
221+
fun mutate(input: MutationInput): String = input.items.orEmpty().joinToString(",") { it.prop.orEmpty() }
222+
}
223+
187224
open class AuditWrapper<T> {
188225
var content: T? = null
189226
var operator: String? = null
@@ -196,6 +233,19 @@ class GenericInputTypesTest {
196233
var reason: String? = null
197234
}
198235

236+
abstract class GenericMutationInput<T> {
237+
@JvmField
238+
var items: List<T>? = null
239+
}
240+
241+
abstract class RenamedMutationInput<U> : GenericMutationInput<U>()
242+
243+
class MutationInput : RenamedMutationInput<MutationInput.Item>() {
244+
class Item {
245+
var prop: String? = null
246+
}
247+
}
248+
199249
class LanguageInput {
200250
var id: String? = null
201251
}

0 commit comments

Comments
 (0)