Resolve type variables through generic superclasses - #825
Open
oryan-block wants to merge 3 commits into
Open
oryan-block wants to merge 3 commits into
oryan-block wants to merge 3 commits into
Conversation
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>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
2 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #460
Fixes #218
Checklist
Description
An object type backed by a class that inherits a type variable through more than one generic superclass fails to build. Take marcust's example from #460:
ConcreteItem extends BaseItem<Long>,BaseItem<T> extends AbstractItem<T>, andpublic T iddeclared onAbstractItem. It used to fail withTypeUtils.getRawType(type, declaringType) must not be null, and since #819 it's aStackOverflowError. If the superclasses rename the variable, like theIdentifiable<I>/IdentifiableAdapter<K>/AuditableEntity<E>chain in #218, it fails withIllegalStateException: No type variable found for: ...IdentifiableAdapter:K. If a superclass reorders them (B<T, U> extends A<U, T>), it silently picks the wrong argument, which surfaces as aFieldResolverErroron the wrong class or as a wrong mapping.GenericType.RelativeToresolved type variables against the declaring type, which only knows the arguments passed in by its direct subclass. When those were type variables too,unwrapGenericType(declaringType, type)fell back toTypeUtils.determineTypeArgumentsagainst the first parameterized superclass and matched the result by name (that's the 6.0.0 fix for #218). When every level uses the same name, it resolvesAbstractItem.TtoBaseItem.Tand loops forever. With different names it finds nothing, and with reordered names it finds the wrong one.Now
replaceTypeVariableresolves every type variable declared by a class withTypeUtils.getTypeArguments(mostSpecificType, genericDeclaration), which is whatgetRawClassalready does. It then keeps resolving whatever the variable is bound to, so nested arguments likeList<Edge<T>>come out fully resolved. It also walks the owner types ofmostSpecificTypefor variables of an outer class (Listing<T>.Entry), and it resolves wildcard bounds (List<? extends T>, which Kotlin emits forList<T>parameters). I removed the name matching (parameterizedDeclaringTypeOrSuperTypeandunwrapGenericType(declaringType, type)). Every caller's declaring type is the most specific type or one of its supertypes, so it can't resolve anything the new lookup can't. A variable that still can't be resolved now fails withCould not resolve type variable 'T' of X relative to Yinstead of looping. A variable bound to a type containing itself can only leak out of a raw type (e.g. a rawTreereturningTree<List<T>>). It's now erased to its bound, the way the raw type would erase it.The original #460 case (an input type inheriting
List<T>) already works on master since #819. I added a test for it with an extra superclass that renames the variable, and that version does fail on master. I didn't addT[]support. It fails with the same clear error as before, and fixing it needsGenericArrayTypehandling inTypeClassMatchertoo. I also didn't fix resolver arguments typedT, which are still passed to Jackson unresolved at runtime, same as before.RelativeTo.declaringTypeis now only used in the error message. Removing it would touch theFieldResolverandSchemaClassScannercall sites, so I left that for a follow-up.Behaviour change: generic types returned by methods inherited from a generic superclass now keep their resolved arguments (e.g.
Meta<Owner>), as inherited fields already did. So if two subclasses bind them differently and both map to the same GraphQL type, the build fails with "Two different classes used for type Meta". That's the same thing that happens whenPage<A>andPage<B>are referenced directly (#343). On 14.0.3 these schemas built as long as the shared type had no field depending onT. The error now prints the parameterized types (Meta<Owner>vsMeta<Tag>) instead of the same raw class twice. Also, a type variable declared by a generic method (<T> T owner()) used to be matched by name to a class variable with the same name when the resolver had a parameterized superclass. It now fails with the error above.🤖 Generated with Claude Code