Merge pull request #29883 from manovotn/producerWildcardDetection

Arc - improve producer wildcard type detection
quarkusio · Dec 16, 2022 · afe32fa · afe32fa
2 parents b0d399d + b39b96b
commit afe32fa
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 27 deletions.
diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Types.java
@@ -432,6 +432,30 @@ static List<Type> getResolvedParameters(ClassInfo classInfo, Map<String, Type> r
         }
     }
 
+    static void detectWildcardAndThrow(Type type, AnnotationTarget producerFieldOrMethod) {
+        if (producerFieldOrMethod == null) {
+            // not a producer, no further analysis required
+            return;
+        }
+        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.");
+        } else if (type.kind().equals(Kind.PARAMETERIZED_TYPE)) {
+            for (Type t : type.asParameterizedType().arguments()) {
+                // recursive check of all parameterized types
+                detectWildcardAndThrow(t, producerFieldOrMethod);
+            }
+        }
+    }
+
     static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFieldOrMethod,
             Map<String, Type> resolvedTypeParameters,
             BeanDeployment beanDeployment, BiConsumer<ClassInfo, Map<String, Type>> resolvedTypeVariablesConsumer) {
@@ -445,19 +469,12 @@ 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());
-                // this should only be the case for producers; wildcard is not a legal bean type
+                // 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
-                if (typeParams[i].kind().equals(Kind.WILDCARD_TYPE) && producerFieldOrMethod != null) {
-                    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.");
-                    skipThisType = true;
-                }
+                // NOTE: wildcard can be nested, such as List<Set<? extends Number>>
+                detectWildcardAndThrow(typeParams[i], producerFieldOrMethod);
             }
             if (resolvedTypeVariablesConsumer != null) {
                 Map<String, Type> resolved = new HashMap<>();
@@ -466,9 +483,7 @@ static Set<Type> getTypeClosure(ClassInfo classInfo, AnnotationTarget producerFi
                 }
                 resolvedTypeVariablesConsumer.accept(classInfo, resolved);
             }
-            if (!skipThisType) {
-                types.add(ParameterizedType.create(classInfo.name(), typeParams, null));
-            }
+            types.add(ParameterizedType.create(classInfo.name(), typeParams, null));
         }
         // Interfaces
         for (Type interfaceType : classInfo.interfaceTypes()) {

diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/TypesTest.java
@@ -1,6 +1,7 @@
 package io.quarkus.arc.processor;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
@@ -11,11 +12,11 @@
 import java.util.Map;
 import java.util.Set;
 
+import javax.enterprise.inject.spi.DefinitionException;
+
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
-import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.IndexView;
-import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.ParameterizedType;
 import org.jboss.jandex.Type;
 import org.jboss.jandex.Type.Kind;
@@ -26,7 +27,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);
+                Iterable.class, Set.class);
         DotName bazName = DotName.createSimple(Baz.class.getName());
         DotName fooName = DotName.createSimple(Foo.class.getName());
         DotName producerName = DotName.createSimple(Producer.class.getName());
@@ -64,18 +65,19 @@ public void testGetTypeClosure() throws IOException {
             }
         }
         ClassInfo producerClass = index.getClassByName(producerName);
-        String producersName = "produce";
-        MethodInfo producerMethod = producerClass.method(producersName);
-        // Object is the sole type
-        Set<Type> producerMethodTypes = Types.getProducerMethodTypeClosure(producerMethod,
-                dummyDeployment);
-        assertEquals(1, producerMethodTypes.size());
+        final String producersName = "produce";
+        assertThrows(DefinitionException.class,
+                () -> Types.getProducerMethodTypeClosure(producerClass.method(producersName), dummyDeployment));
+        assertThrows(DefinitionException.class,
+                () -> Types.getProducerFieldTypeClosure(producerClass.field(producersName), dummyDeployment));
+
+        // now assert the same with nested wildcard
+        final String nestedWildCardProducersName = "produceNested";
+        assertThrows(DefinitionException.class,
+                () -> Types.getProducerMethodTypeClosure(producerClass.method(nestedWildCardProducersName), dummyDeployment));
+        assertThrows(DefinitionException.class,
+                () -> Types.getProducerFieldTypeClosure(producerClass.field(nestedWildCardProducersName), dummyDeployment));
 
-        // Object is the sole type
-        FieldInfo producerField = producerClass.field(producersName);
-        Set<Type> producerFieldTypes = Types.getProducerFieldTypeClosure(producerField,
-                dummyDeployment);
-        assertEquals(1, producerFieldTypes.size());
     }
 
     static class Foo<T> {
@@ -94,6 +96,12 @@ public List<? extends Number> produce() {
             return null;
         }
 
+        public List<Set<? extends Number>> produceNested() {
+            return null;
+        }
+
         List<? extends Number> produce;
+
+        List<Set<? extends Number>> produceNested;
     }
 }