diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java index 075de06661523..f85f7eafa470d 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java @@ -1,6 +1,5 @@ package io.quarkus.arc.deployment; -import static io.quarkus.arc.processor.KotlinUtils.isKotlinClass; import static io.quarkus.deployment.annotations.ExecutionTime.RUNTIME_INIT; import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT; @@ -9,7 +8,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; @@ -23,11 +21,9 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.AmbiguousResolutionException; import jakarta.enterprise.inject.UnsatisfiedResolutionException; -import jakarta.enterprise.inject.spi.DefinitionException; import org.jboss.jandex.AnnotationInstance; import org.jboss.jandex.AnnotationTarget; -import org.jboss.jandex.AnnotationValue; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.FieldInfo; @@ -46,7 +42,6 @@ import io.quarkus.arc.deployment.UnremovableBeanBuildItem.BeanTypeExclusion; import io.quarkus.arc.deployment.ValidationPhaseBuildItem.ValidationErrorBuildItem; import io.quarkus.arc.processor.AlternativePriorities; -import io.quarkus.arc.processor.Annotations; import io.quarkus.arc.processor.AnnotationsTransformer; import io.quarkus.arc.processor.BeanConfigurator; import io.quarkus.arc.processor.BeanDefiningAnnotation; @@ -56,13 +51,10 @@ import io.quarkus.arc.processor.BeanProcessor; import io.quarkus.arc.processor.BeanRegistrar; import io.quarkus.arc.processor.BeanResolver; -import io.quarkus.arc.processor.Beans; import io.quarkus.arc.processor.BytecodeTransformer; import io.quarkus.arc.processor.ContextConfigurator; import io.quarkus.arc.processor.ContextRegistrar; import io.quarkus.arc.processor.DotNames; -import io.quarkus.arc.processor.InjectionPointInfo; -import io.quarkus.arc.processor.InjectionPointInfo.TypeAndQualifiers; import io.quarkus.arc.processor.ObserverConfigurator; import io.quarkus.arc.processor.ObserverRegistrar; import io.quarkus.arc.processor.ReflectionRegistration; @@ -466,39 +458,6 @@ public ObserverRegistrationPhaseBuildItem registerSyntheticObservers(BeanRegistr // Initialize the type -> bean map beanRegistrationPhase.getBeanProcessor().getBeanDeployment().initBeanByTypeMap(); - // Register a synthetic bean for each List with qualifier @All - List listAll = beanRegistrationPhase.getInjectionPoints().stream() - .filter(this::isListAllInjectionPoint).collect(Collectors.toList()); - for (InjectionPointInfo injectionPoint : listAll) { - // Note that at this point we can be sure that the required type is List<> - Type typeParam = injectionPoint.getType().asParameterizedType().arguments().get(0); - if (typeParam.kind() == Type.Kind.WILDCARD_TYPE) { - ClassInfo declaringClass; - if (injectionPoint.isField()) { - declaringClass = injectionPoint.getTarget().asField().declaringClass(); - } else { - declaringClass = injectionPoint.getTarget().asMethod().declaringClass(); - } - if (isKotlinClass(declaringClass)) { - validationErrors.produce(new ValidationErrorBuildItem( - new DefinitionException( - "kotlin.collections.List cannot be used together with the @All qualifier, please use MutableList or java.util.List instead: " - + injectionPoint.getTargetInfo()))); - } else { - validationErrors.produce(new ValidationErrorBuildItem( - new DefinitionException( - "Wildcard is not a legal type argument for " + injectionPoint.getTargetInfo()))); - } - } else if (typeParam.kind() == Type.Kind.TYPE_VARIABLE) { - validationErrors.produce(new ValidationErrorBuildItem(new DefinitionException( - "Type variable is not a legal type argument for " + injectionPoint.getTargetInfo()))); - } - } - if (!listAll.isEmpty()) { - registerUnremovableListBeans(beanRegistrationPhase, listAll, reflectiveMethods, reflectiveFields, - unremovableBeans); - } - BeanProcessor beanProcessor = beanRegistrationPhase.getBeanProcessor(); ObserverRegistrar.RegistrationContext registrationContext = beanProcessor.registerSyntheticObservers(); @@ -793,60 +752,6 @@ void registerContextPropagation(ArcConfig config, BuildProducer injectionPoints, BuildProducer reflectiveMethods, - BuildProducer reflectiveFields, - BuildProducer unremovableBeans) { - BeanDeployment beanDeployment = beanRegistrationPhase.getBeanProcessor().getBeanDeployment(); - List unremovables = new ArrayList<>(); - - for (InjectionPointInfo injectionPoint : injectionPoints) { - // All qualifiers but @All - Set qualifiers = new HashSet<>(injectionPoint.getRequiredQualifiers()); - for (Iterator it = qualifiers.iterator(); it.hasNext();) { - AnnotationInstance qualifier = it.next(); - if (DotNames.ALL.equals(qualifier.name())) { - it.remove(); - } - } - if (qualifiers.isEmpty()) { - // If no other qualifier is used then add @Any - qualifiers.add(AnnotationInstance.create(DotNames.ANY, null, new AnnotationValue[] {})); - } - - Type elementType = injectionPoint.getType().asParameterizedType().arguments().get(0); - - // make note of all types inside @All List to make sure they are unremovable - unremovables.add(new TypeAndQualifiers( - elementType.name().equals(DotNames.INSTANCE_HANDLE) - ? elementType.asParameterizedType().arguments().get(0) - : elementType, - qualifiers)); - } - if (!unremovables.isEmpty()) { - // New beans were registered - we need to re-init the type -> bean map - // Also make all beans that match the List<> injection points unremovable - beanDeployment.initBeanByTypeMap(); - // And make all the matching beans unremovable - unremovableBeans.produce(new UnremovableBeanBuildItem(new Predicate() { - @Override - public boolean test(BeanInfo bean) { - for (TypeAndQualifiers tq : unremovables) { - if (Beans.matches(bean, tq)) { - return true; - } - } - return false; - } - })); - } - } - - private boolean isListAllInjectionPoint(InjectionPointInfo injectionPoint) { - return DotNames.LIST.equals(injectionPoint.getRequiredType().name()) - && Annotations.contains(injectionPoint.getRequiredQualifiers(), DotNames.ALL); - } - private abstract static class AbstractCompositeApplicationClassesPredicate implements Predicate { private final IndexView applicationClassesIndex; diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java index d1e16194e5311..9e093574ad54e 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java @@ -455,7 +455,8 @@ private Set removeUnusedDecorators(Set removedDeco } private Set removeUnusedBeans(Set declaresObserver, List> allUnusedExclusions) { - Set removableBeans = UnusedBeans.findRemovableBeans(this.beans, this.injectionPoints, declaresObserver, + Set removableBeans = UnusedBeans.findRemovableBeans(beanResolver, this.beans, this.injectionPoints, + declaresObserver, allUnusedExclusions); if (!removableBeans.isEmpty()) { this.beans.removeAll(removableBeans); diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BuiltinBean.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BuiltinBean.java index 8ebee1d654d9c..847b9cf87347c 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BuiltinBean.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BuiltinBean.java @@ -1,6 +1,7 @@ package io.quarkus.arc.processor; import static io.quarkus.arc.processor.IndexClassLookupUtils.getClassByName; +import static io.quarkus.arc.processor.KotlinUtils.isKotlinClass; import java.lang.reflect.Member; import java.util.HashSet; @@ -64,7 +65,7 @@ public enum BuiltinBean { BuiltinBean::validateEventMetadata, DotNames.EVENT_METADATA), LIST(BuiltinBean::generateListBytecode, (ip, names) -> cdiAndRawTypeMatches(ip, DotNames.LIST) && ip.getRequiredQualifier(DotNames.ALL) != null, - DotNames.LIST), + BuiltinBean::validateList, DotNames.LIST), ; private final DotName[] rawTypeDotNames; @@ -443,6 +444,38 @@ private static void validateInstance(InjectionTargetInfo injectionTarget, Inject } } + private static void validateList(InjectionTargetInfo injectionTarget, InjectionPointInfo injectionPoint, + Consumer errors) { + if (injectionPoint.getType().kind() != Kind.PARAMETERIZED_TYPE) { + errors.accept( + new DefinitionException("An injection point of raw type is defined: " + injectionPoint.getTargetInfo())); + } else { + // Note that at this point we can be sure that the required type is List<> + Type typeParam = injectionPoint.getType().asParameterizedType().arguments().get(0); + if (typeParam.kind() == Type.Kind.WILDCARD_TYPE) { + ClassInfo declaringClass; + if (injectionPoint.isField()) { + declaringClass = injectionPoint.getTarget().asField().declaringClass(); + } else { + declaringClass = injectionPoint.getTarget().asMethod().declaringClass(); + } + if (isKotlinClass(declaringClass)) { + errors.accept( + new DefinitionException( + "kotlin.collections.List cannot be used together with the @All qualifier, please use MutableList or java.util.List instead: " + + injectionPoint.getTargetInfo())); + } else { + errors.accept( + new DefinitionException( + "Wildcard is not a legal type argument for: " + injectionPoint.getTargetInfo())); + } + } else if (typeParam.kind() == Type.Kind.TYPE_VARIABLE) { + errors.accept(new DefinitionException( + "Type variable is not a legal type argument for: " + injectionPoint.getTargetInfo())); + } + } + } + private static void validateInjectionPoint(InjectionTargetInfo injectionTarget, InjectionPointInfo injectionPoint, Consumer errors) { if (injectionTarget.kind() != TargetKind.BEAN || !BuiltinScope.DEPENDENT.is(injectionTarget.asBean().getScope())) { diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/UnusedBeans.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/UnusedBeans.java index f4c261df3be14..ebacd822c3c3c 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/UnusedBeans.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/UnusedBeans.java @@ -1,9 +1,9 @@ package io.quarkus.arc.processor; -import static java.util.function.Predicate.not; - +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -11,8 +11,12 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import org.jboss.jandex.AnnotationInstance; +import org.jboss.jandex.Type; import org.jboss.logging.Logger; +import io.quarkus.arc.processor.InjectionPointInfo.TypeAndQualifiers; + final class UnusedBeans { private static final Logger LOG = Logger.getLogger(UnusedBeans.class); @@ -20,22 +24,49 @@ final class UnusedBeans { private UnusedBeans() { } - static Set findRemovableBeans(Collection beans, Collection injectionPoints, - Set declaresObserver, List> allUnusedExclusions) { + static Set findRemovableBeans(BeanResolver beanResolver, Collection beans, + Iterable injectionPoints, Set declaresObserver, + List> allUnusedExclusions) { Set removableBeans = new HashSet<>(); Set unusedProducers = new HashSet<>(); Set unusedButDeclaresProducer = new HashSet<>(); List producers = beans.stream().filter(BeanInfo::isProducer) .collect(Collectors.toList()); - List instanceInjectionPoints = injectionPoints.stream() - .filter(InjectionPointInfo::isProgrammaticLookup) - .collect(Collectors.toList()); - // Collect all injected beans; skip delegate injection points and injection points that resolve to a built-in bean - Set injected = injectionPoints.stream() - .filter(not(InjectionPointInfo::isDelegate).and(InjectionPointInfo::hasResolvedBean)) - .map(InjectionPointInfo::getResolvedBean) - .collect(Collectors.toSet()); + // Collect all: + // - injected beans; skip delegate injection points and injection points that resolve to a built-in bean + // - Instance<> injection points + // - @All List<> injection points + Set injected = new HashSet<>(); + List instanceInjectionPoints = new ArrayList<>(); + List listAllInjectionPoints = new ArrayList<>(); + + for (InjectionPointInfo injectionPoint : injectionPoints) { + if (injectionPoint.isProgrammaticLookup()) { + instanceInjectionPoints.add(injectionPoint); + } else if (!injectionPoint.isDelegate()) { + BeanInfo resolved = injectionPoint.getResolvedBean(); + if (resolved != null) { + injected.add(resolved); + } else { + BuiltinBean builtin = BuiltinBean.resolve(injectionPoint); + if (builtin == BuiltinBean.LIST) { + Type requiredType = injectionPoint.getType().asParameterizedType().arguments().get(0); + if (requiredType.name().equals(DotNames.INSTANCE_HANDLE)) { + requiredType = requiredType.asParameterizedType().arguments().get(0); + } + Set qualifiers = new HashSet<>(injectionPoint.getRequiredQualifiers()); + for (Iterator it = qualifiers.iterator(); it.hasNext();) { + AnnotationInstance qualifier = it.next(); + if (qualifier.name().equals(DotNames.ALL)) { + it.remove(); + } + } + listAllInjectionPoints.add(new TypeAndQualifiers(requiredType, qualifiers)); + } + } + } + } Set declaresProducer = producers.stream().map(BeanInfo::getDeclaringBean).collect(Collectors.toSet()); // Beans - first pass to find unused beans that do not declare a producer @@ -70,12 +101,20 @@ static Set findRemovableBeans(Collection beans, Collection for (InjectionPointInfo injectionPoint : instanceInjectionPoints) { if (Beans.hasQualifiers(bean, injectionPoint.getRequiredQualifiers()) - && bean.getDeployment().getBeanResolver().matchesType(bean, + && beanResolver.matchesType(bean, injectionPoint.getType().asParameterizedType().arguments().get(0))) { LOG.debugf("Unremovable - programmatic lookup: %s", bean); continue test; } } + // @All List + for (TypeAndQualifiers tq : listAllInjectionPoints) { + if (Beans.hasQualifiers(bean, tq.qualifiers) + && beanResolver.matchesType(bean, tq.type)) { + LOG.debugf("Unremovable - @All List: %s", bean); + continue test; + } + } // Declares a producer - see also second pass if (declaresProducer.contains(bean)) { unusedButDeclaresProducer.add(bean); diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/all/ListAllTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/all/ListAllTest.java index bab8073de77f2..c82ae074abaac 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/all/ListAllTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/all/ListAllTest.java @@ -31,7 +31,6 @@ public class ListAllTest { public ArcTestContainer container = new ArcTestContainer(Service.class, ServiceAlpha.class, ServiceBravo.class, MyQualifier.class, Foo.class); - @SuppressWarnings("serial") @Test public void testSelectAll() { verifyHandleInjection(Arc.container().listAll(Service.class), Object.class); diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/unused/RemoveUnusedBeansTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/unused/RemoveUnusedBeansTest.java index 5de6b95a640bd..7532b145b3394 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/unused/RemoveUnusedBeansTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/unused/RemoveUnusedBeansTest.java @@ -6,6 +6,7 @@ import java.math.BigDecimal; import java.math.BigInteger; +import java.util.List; import jakarta.annotation.PostConstruct; import jakarta.annotation.Priority; @@ -22,6 +23,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.arc.All; import io.quarkus.arc.Arc; import io.quarkus.arc.ArcContainer; import io.quarkus.arc.impl.ArcContainerImpl; @@ -32,10 +34,9 @@ public class RemoveUnusedBeansTest extends RemoveUnusedComponentsTest { @RegisterExtension public ArcTestContainer container = ArcTestContainer.builder() .beanClasses(HasObserver.class, Foo.class, FooAlternative.class, HasName.class, UnusedProducers.class, - InjectedViaInstance.class, InjectedViaInstanceWithWildcard.class, - InjectedViaProvider.class, Excluded.class, - UsedProducers.class, - UnusedProducerButInjected.class, UsedViaInstanceWithUnusedProducer.class, UsesBeanViaInstance.class) + InjectedViaInstance.class, InjectedViaInstanceWithWildcard.class, InjectedViaProvider.class, Excluded.class, + UsedProducers.class, UnusedProducerButInjected.class, UsedViaInstanceWithUnusedProducer.class, + UsesBeanViaInstance.class, UsedViaAllList.class) .removeUnusedBeans(true) .addRemovalExclusion(b -> b.getBeanClass().toString().equals(Excluded.class.getName())) .build(); @@ -67,6 +68,7 @@ public void testRemoval() { assertFalse(ArcContainerImpl.instance().getRemovedBeans().isEmpty()); assertNotPresent(UnusedBean.class); assertNotPresent(OnlyInjectedInUnusedBean.class); + assertPresent(UsedViaAllList.class); } @Dependent @@ -197,6 +199,10 @@ static class UsesBeanViaInstance { @Inject Instance instance; + + @Inject + @All + List list; } @Singleton @@ -212,4 +218,9 @@ static class OnlyInjectedInUnusedBean { } + @Singleton + static class UsedViaAllList { + + } + }