Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arc - Change Types#getTypeClosure so that superclasses and interfaces of producer types no longer throw on finding wildcards #30415

Merged
merged 1 commit into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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> {
}
}