Make `GenericType<T>` not implement `java.lang.reflect.Type`
Avaliação
- Dificuldade
- 4/5
- Tempo estimado
- 3-5 dias
- Facilidade para iniciantes
- 35/100
Direção de pesquisa
Localize GenericType e inspecione seus chamadores e testes existentes, especialmente o comportamento que depende de ele ser um java.lang.reflect.Type. Revise a discussão em #73 e as preocupações de compatibilidade do issue antes de alterar a API pública. Considera-se concluído quando GenericType não implementar mais Type, enquanto seu comportamento de contêiner de tipo e os testes afetados continuarem corretos.
Escrita pelo modelo de indexação a partir do texto da issue.
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 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!
- Linguagem predominante
- Java
- Estrelas
- 266
- Forks
- 53
- Merge médio
- 1d 1h
- PRs com merge (30d)
- 5
Preparar o ambiente
Este projeto não oferece contêiner de desenvolvimento, Dockerfile nem guia de contribuição, então a configuração fica por sua conta: comece pelo README e veja nosso guia da primeira contribuição para os passos gerais.
Primeiros passos
- Leia a issue inteira e depois o guia de contribuição do projeto.
- Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
- Faça um fork do repositório e trabalhe em uma branch.
- Abra um pull request que referencie o número da issue.
Mais de FasterXML/java-classmate
-
Dificuldade 4/5 3-5 dias Facilidade para iniciantes 45/100
FasterXML/java-classmate#133 ·
-
Dificuldade 4/5 3-5 dias Facilidade para iniciantes 35/100
FasterXML/java-classmate#132 ·
-
Dificuldade 5/5 Mais de uma semana Facilidade para iniciantes 25/100
FasterXML/java-classmate#111 · 10 comentários ·
-
Dificuldade 5/5 Mais de uma semana Facilidade para iniciantes 20/100
FasterXML/java-classmate#81 · 1 comentário ·
-
Dificuldade 5/5 Mais de uma semana Facilidade para iniciantes 20/100
FasterXML/java-classmate#72 · 3 comentários ·
Todas as issues de FasterXML/java-classmate
Issues semelhantes
-
`Processing lsp` never exits and leaves orphaned processesTalvez já em andamento @overcast302 assumiu hoje. Abertabug
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 72/100
processing/processing4#1578 · 1 comentário ·
-
ASM is not up-to-dateAberta
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 60/100
Mantenedores costumam responder em até 1 dia
-
[BUG] S3 CORS responses omit Access-Control-Allow-Credentials for matched originsTalvez já em andamento Um pull request vinculado a esta issue está aberto ou já foi mesclado. Aberta
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 72/100
floci-io/floci#5369 · 1 comentário ·
Mantenedores costumam responder em até 1 dia
-
securityHeaders replaces a route's own Content-Security-Policy (0.9.9; weakens embedders' pages)Abertabug
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 72/100
Mantenedores costumam responder em até 1 dia
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 72/100
sqlcipher/sqlcipher-android#97 · 1 comentário ·