Skip to content

Commit

Permalink
Arc - Change Types#getTypeClosure so that superclasses and interfaces…
Browse files Browse the repository at this point in the history 
… of producer types no longer throw on finding wildcards
  • Loading branch information
@manovotn
manovotn committed Jan 17, 2023
1 parent f07da48 commit 3107510
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 20 deletions.
62 changes: 44 additions & 18 deletions independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,31 +432,47 @@ static List<Type> getResolvedParameters(ClassInfo classInfo, Map<String, Type> r
}
}

static void detectWildcardAndThrow(Type type, AnnotationTarget producerFieldOrMethod) {
/**
* Detects wildcard for given producer method/field.
* Based on the boolean parameter, it either throws a {@link DefinitionException} of performs logging.
* Returns true if a wildcard is detected, false otherwise.
*/
static boolean isProducerWithWildcard(Type type, AnnotationTarget producerFieldOrMethod, boolean throwIfDetected) {
if (producerFieldOrMethod == null) {
// not a producer, no further analysis required
return;
return false;
}
if (type.kind().equals(Kind.WILDCARD_TYPE)) {
throw new DefinitionException("Producer " +
(producerFieldOrMethod.kind().equals(AnnotationTarget.Kind.FIELD) ? "field " : "method ") +
producerFieldOrMethod +
" declared on class " +
(producerFieldOrMethod.kind().equals(AnnotationTarget.Kind.FIELD)
? producerFieldOrMethod.asField().declaringClass().name()
: producerFieldOrMethod.asMethod().declaringClass().name())
+
" contains a parameterized type with a wildcard. This type is not a legal bean type" +
" according to CDI specification.");
if (throwIfDetected) {
throw new DefinitionException("Producer " +
(producerFieldOrMethod.kind().equals(AnnotationTarget.Kind.FIELD) ? "field " : "method ") +
producerFieldOrMethod +
" declared on class " +
(producerFieldOrMethod.kind().equals(AnnotationTarget.Kind.FIELD)
? producerFieldOrMethod.asField().declaringClass().name()
: producerFieldOrMethod.asMethod().declaringClass().name())
+
" contains a parameterized type with a wildcard. This type is not a legal bean type" +
" according to CDI specification.");
} else {
LOGGER.info("Producer " +
(producerFieldOrMethod.kind().equals(AnnotationTarget.Kind.FIELD) ? "field " : "method ") +
producerFieldOrMethod +
" contains a parameterized typed with a wildcard. This type is not a legal bean type" +
" according to CDI specification and will be ignored during bean resolution.");
return true;
}
} else if (type.kind().equals(Kind.PARAMETERIZED_TYPE)) {
for (Type t : type.asParameterizedType().arguments()) {
// recursive check of all parameterized types
detectWildcardAndThrow(t, producerFieldOrMethod);
return isProducerWithWildcard(t, producerFieldOrMethod, throwIfDetected);
}
}
return false;
}

static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFieldOrMethod,
private static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFieldOrMethod,
boolean throwOnProducerWildcard,
Map<String, Type> resolvedTypeParameters,
BeanDeployment beanDeployment, BiConsumer<ClassInfo, Map<String, Type>> resolvedTypeVariablesConsumer) {
Set<Type> types = new HashSet<>();
Expand All @@ -469,12 +485,13 @@ static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFi
} else {
// Canonical ParameterizedType with unresolved type variables
Type[] typeParams = new Type[typeParameters.size()];
boolean skipThisType = false;
for (int i = 0; i < typeParameters.size(); i++) {
typeParams[i] = resolvedTypeParameters.get(typeParameters.get(i).identifier());
// for producers, wildcard is not a legal bean type and results in a definition error
// see https://docs.jboss.org/cdi/spec/2.0/cdi-spec.html#legal_bean_types
// NOTE: wildcard can be nested, such as List<Set<? extends Number>>
detectWildcardAndThrow(typeParams[i], producerFieldOrMethod);
skipThisType = isProducerWithWildcard(typeParams[i], producerFieldOrMethod, throwOnProducerWildcard);
}
if (resolvedTypeVariablesConsumer != null) {
Map<String, Type> resolved = new HashMap<>();
Expand All @@ -483,7 +500,9 @@ static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFi
}
resolvedTypeVariablesConsumer.accept(classInfo, resolved);
}
types.add(ParameterizedType.create(classInfo.name(), typeParams, null));
if (!skipThisType) {
types.add(ParameterizedType.create(classInfo.name(), typeParams, null));
}
}
// Interfaces
for (Type interfaceType : classInfo.interfaceTypes()) {
Expand All @@ -497,7 +516,7 @@ static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFi
resolved = buildResolvedMap(interfaceType.asParameterizedType().arguments(),
interfaceClassInfo.typeParameters(), resolvedTypeParameters, beanDeployment.getBeanArchiveIndex());
}
types.addAll(getTypeClosure(interfaceClassInfo, producerFieldOrMethod, resolved, beanDeployment,
types.addAll(getTypeClosure(interfaceClassInfo, producerFieldOrMethod, false, resolved, beanDeployment,
resolvedTypeVariablesConsumer));
}
}
Expand All @@ -511,13 +530,20 @@ static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFi
superClassInfo.typeParameters(),
resolvedTypeParameters, beanDeployment.getBeanArchiveIndex());
}
types.addAll(getTypeClosure(superClassInfo, producerFieldOrMethod, resolved, beanDeployment,
types.addAll(getTypeClosure(superClassInfo, producerFieldOrMethod, false, resolved, beanDeployment,
resolvedTypeVariablesConsumer));
}
}
return types;
}

static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFieldOrMethod,
Map<String, Type> resolvedTypeParameters,
BeanDeployment beanDeployment, BiConsumer<ClassInfo, Map<String, Type>> resolvedTypeVariablesConsumer) {
return getTypeClosure(classInfo, producerFieldOrMethod, true, resolvedTypeParameters, beanDeployment,
resolvedTypeVariablesConsumer);
}

static Set<Type> getDelegateTypeClosure(InjectionPointInfo delegateInjectionPoint, BeanDeployment beanDeployment) {
Set<Type> types;
Type delegateType = delegateInjectionPoint.getRequiredType();
Expand Down
33 changes: 31 additions & 2 deletions independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.arc.processor;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -27,7 +28,7 @@ public class TypesTest {
@Test
public void testGetTypeClosure() throws IOException {
IndexView index = Basics.index(Foo.class, Baz.class, Producer.class, Object.class, List.class, Collection.class,
Iterable.class, Set.class);
Iterable.class, Set.class, Eagle.class, Bird.class, Animal.class, AnimalHolder.class);
DotName bazName = DotName.createSimple(Baz.class.getName());
DotName fooName = DotName.createSimple(Foo.class.getName());
DotName producerName = DotName.createSimple(Producer.class.getName());
Expand Down Expand Up @@ -78,6 +79,13 @@ public void testGetTypeClosure() throws IOException {
assertThrows(DefinitionException.class,
() -> Types.getProducerFieldTypeClosure(producerClass.field(nestedWildCardProducersName), dummyDeployment));

// now assert following ones do NOT throw, the wildcard in the hierarchy is just ignored
final String wildcardInTypeHierarchy = "eagleProducer";
assertDoesNotThrow(
() -> Types.getProducerMethodTypeClosure(producerClass.method(wildcardInTypeHierarchy), dummyDeployment));
assertDoesNotThrow(
() -> Types.getProducerFieldTypeClosure(producerClass.field(wildcardInTypeHierarchy), dummyDeployment));

}

static class Foo<T> {
Expand All @@ -90,7 +98,7 @@ static class Baz extends Foo<String> {

}

static class Producer {
static class Producer<T> {

public List<? extends Number> produce() {
return null;
Expand All @@ -103,5 +111,26 @@ public List<Set<? extends Number>> produceNested() {
List<? extends Number> produce;

List<Set<? extends Number>> produceNested;

// following producer should NOT throw an exception because the return types doesn't contain wildcard
// but the hierarchy of the return type actually contains one
// taken from CDI TCK setup, see https://github.com/jakartaee/cdi-tck/blob/4.0.7/impl/src/main/java/org/jboss/cdi/tck/tests/definition/bean/types/illegal/BeanTypesWithIllegalTypeTest.java
public Eagle<T> eagleProducer() {
return new Eagle<>();
}

public Eagle<T> eagleProducer;
}

static class Eagle<T> extends Bird<T> {
}

static class Bird<T> extends AnimalHolder<Animal<? extends T>> {
}

static class Animal<T> {
}

static class AnimalHolder<T extends Animal> {
}
}

0 comments on commit 3107510

Please sign in to comment.