FasterXML / FasterXML/java-classmate
Make `GenericType<T>` not implement `java.lang.reflect.Type`
- Langage dominant
- Java
- Étoiles
- 266
- Forks
- 47
- Merge moyen
- 1 h 42 min
- PR mergées (30 j)
- 1
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](https://gafter.blogspot.com/2006/12/super-type-tokens.html) pattern. And to put this in context:
* The original example in Neil Gafter's famous [_Super Type Tokens_](https://gafter.blogspot.com/2006/12/super-type-tokens.html) does _not_ implement `Type`.
* Jackson's `TypeReference` does _not_ implement `Type`.
* Spring's [`ParameterizedTypeReference`](https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/core/ParameterizedTypeReference.html) does _not_ implement `Type`.
* The Guice [`TypeLiteral`](https://google.github.io/guice/api-docs/7.0.0/javadoc/com/google/inject/TypeLiteral.html) does _not_ implement `Type`.
* The [GeantyRef](https://github.com/leangen/geantyref) fork of [gentyref](https://code.google.com/archive/p/gentyref/) has a [`TypeToken`](https://github.com/leangen/geantyref/blob/master/src/main/java/io/leangen/geantyref/TypeToken.java) which does _not_ implement `Type`.
* Guava's [`TypeToken`](https://guava.dev/releases/snapshot-jre/api/docs/com/google/common/reflect/TypeToken.html) does _not_ implement `Type`.
Thus ClassMate's `GenericType` 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](https://www.cowtowncoder.com/blog/archives/2010/12/entry_436.html)) 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:
```java
public static Type typeTokenToType(@Nonnull final Object typeToken) {
//super type token (check first, because ClassMate `GenericType` 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`, because `GenericType` 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` 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`, 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` 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!
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Évaluation
Cette issue n'a pas encore été évaluée.