Repository navigation
Make GenericType<T> not implement java.lang.reflect.Type #76
Description
In the same vein as #73, probably GenericType should not implement java.lang.reflect.Type. It's not really a "type" itself—it's a type holder.
I can appreciated that the original conception was to have GenericType be a "type" representing a "type with generics" (sort of like an improved ParameterizedType). But in the larger scheme of things and in hindsight, it's really just one of many duplicates of the Super Type Token pattern. And to put this in context:
- The original example in Neil Gafter's famous Super Type Tokens does not implement
Type. - Jackson's
TypeReferencedoes not implementType. - Spring's
ParameterizedTypeReferencedoes not implementType. - The Guice
TypeLiteraldoes not implementType. - The GeantyRef fork of gentyref has a
TypeTokenwhich does not implementType. - Guava's
TypeTokendoes not implementType.
Thus ClassMate's GenericType<T> is the only one that implements java.lang.reflect.Type.
The reason I bring this up is that the whole Type system (as you explain so well) is already confusing and awkward; adding one other wrapper that itself pretends to be a Type, in contrast with all the other uses of this pattern, adds even more confusion.
Let me give an illustration of the confusion it can cause. Let's say (following the discussion in #69) that I want to make a general method to convert some "type token" to a Type (so that I can then use Jackson to convert it to a JavaType, etc.). Look closely at this code:
public static Type typeTokenToType(@Nonnull final Object typeToken) {
//super type token (check first, because ClassMate `GenericType<T>` is also a `Type`)
final Type superTypeTokenSuperClass = typeToken.getClass().getGenericSuperclass();
if(superTypeTokenSuperClass instanceof ParameterizedType parameterizedType) {
final Type[] actualTypeArguments = parameterizedType.getActualTypeArguments();
if(actualTypeArguments.length == 1) {
return actualTypeArguments[0];
}
}
//type
if(typeToken instanceof Type) { //types in general, if they are not super type tokens needing "unwrapping", can be returned directly
return (Type)typeToken;
}
throw new IllegalArgumentException("Type token must be an instance of `Class`, or have a super class providing a single generic type argument.");
}You see the potential bug if the developer weren't paying attention? In a perfect world, I would first test to see if typeToken is a Type, and just return it, because the type token is already a Type (e.g. a Class<?> or a ParameterizedType). Otherwise I would see if it is a "super type token". And that approach would work with all the other super type tokens—except for ClassMate's GenericType<T>, because GenericType<T> claims it is a Type itself already!
The fix for this example is easy (as shown in the method above): just leave the instanceof Type check for last. But it would be more efficient to test for Type up front. More worrisome, I would have had to have noticed before writing the method (I actually didn't at first) that GenericType<T> implements Type, or I would have wound up with such a bug.
Lots of other bugs may crop up. A developer may write a doFoo(Type type) method, assuming the type has already been "unwrapped", and someone could send it a GenericType<T>, forgetting to unwrap/extract the Type first. I could see this happening all over the place.
At the end of the day this isn't a blocker, and it's certainly not a critical bug. And if you don't agree with me, and you see value in having GenericType<T> implement Type, that's fine. Nevertheless I thought I'd document what I see as an issue so that you can think about it. Cheers!
Activity
cowtowncoder commentedon Oct 11, 2023
Yes, I concur.
In fact, I have hard time understanding why on earth did I make
GenericTypeclaim to bejava.lang.reflect.Type.But... alas, removing breaks out all the things within library that rely on "convenience" of common base type.
So I guess that's the answer really; API takes in
GenericTypeas well as variations.So: I am +1 on getting rid of this, but don't have much time to spend on figuring it out (yes there's bit of theme here :) ).
On plus side I was able to upgrade things so that:
masteris now for 1.7.0, with JDK 8 baselinewhich should improve project management, make easier to verify goodness of PRs and so on.
cowtowncoder commentedon Dec 5, 2025
Ok:
1.7.1released. I think that change here, along with #103 (add mapper to go fromResolvedTypetojava.lang.reflect.Type) could go in 1.8.0.[-]Make `GenericType<T>` not implement `Type`.[/-][+]Make `GenericType<T>` not implement `java.lang.reflect.Type`[/+]Fix #76
cowtowncoder commentedon Dec 5, 2025
Hmmmh. Ok. There is one big problem here:
TypeResolverAPI which takesjava.lang.reflect.TypeandGenericTypeis passed as one.While this could be changed, it is gnarly source-compatible, binary-incompatible breakage: exactly kind that could easily break lots of transitive dependencies. Meaning that someone might upgrade from 1.7.1 to 1.8.0, without recompiling intermediate library (or updating it to version recompiled against ClassMate 1.8.0), and get Class loading (Linkage) error.
Not 100% sure what to do here, TBH. Need to think a bit.
Theoretically major version bump (2.0) would allow this, but with Java tooling I am not sure it'd be less likely to cause problems in reality.
garretwilson commentedon Dec 18, 2025
I'm actually working on another large framework at this very moment that will be using ClassMate's
GenericTypeas its standard type token, so it will be nice to make this as elegant as possible. I confess it's taking me a bit to get back up to speed about what I was talking about back then. 😅 I could see how integratingGenericTypeinto this new library might run into problems if I forgetGenericTypeisn't really a type, and forget to check things in the correct order (which I think was one of the downsides I realized of the current design decision).FWIW, I added this ticket into the LLM conversation I used to analyze PR #103. It pointed out that you would need to add an overload like
resolve(GenericType<?>)to theTypeResolverAPI ifGenericTypewere to cease to implementType. But that doesn't seem to be a "big problem" or even a problem really. More of a tiny task. 😃 But yeah a new major version increment may be appropriate.cowtowncoder commentedon Dec 23, 2025
@garretwilson My major concern really is that unless 2.0 is "fork-like" major version (with new Maven coordinates, Java packages -- like Jackson 1.x -> 2.x -> 3.x), someone, somewhere (... Github Dependabot pushing PRs f.ex) with push breaking change through: binary (signature) incompatibility where
TypeResolver's methods do not match. And when Classmate is a transitive dependency, getting into whole 1.x and 2.x cannot co-exist quicksand.So I am not at all worried about direct usage where recompilation solves source-compatible change which is indeed minor.
But then again, doing "full" major version bump for just this seemingly small change looks dramatic too (and lots of work downstream).
cowtowncoder commentedon Dec 23, 2025
Having just written all of above, I do wonder if overloads for
resolve(...)methods ofTypeResolvermight allow work-around -- adding overloads that takeGenericTypeinstead ofjava.lang.reflect.Type.