FasterXML / FasterXML/java-classmate
Make `GenericType<T>` not implement `java.lang.reflect.Type`
- Linguagem predominante
- Java
- Estrelas
- 266
- Forks
- 47
- Merge médio
- 1h 42min
- PRs com merge (30d)
- 1
Descrição
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!
Guia de contribuição
Nenhum guia de contribuição indexado para este repositório
Direção de pesquisa
Locate GenericType and inspect its existing callers and tests, especially the behavior that depends on it being a java.lang.reflect.Type. Review the discussion in #73 and the issue's compatibility concerns before changing the public API. Done means GenericType no longer implements Type while its type-holder behavior and affected tests remain correct.
Escrita pelo modelo de indexação a partir do texto da issue.
Avaliação
- Stack de tecnologia
- java
- Domínio
- devtools
- Tipo de issue
- Refatoração
- Dificuldade
- 4/5
- Tempo estimado
- 3-5 dias
- Status de atividade
- Estagnada
- Clareza
- Razoavelmente clara
- Facilidade para iniciantes
- 35/100