Skip to content

Commit

Permalink
Merge pull request #33903 from mkouba/arc-alllist-unremovable
Browse files Browse the repository at this point in the history 
ArC: beans injected into All List injection points should be unremovable
  • Loading branch information
@mkouba
mkouba committed Jun 9, 2023
2 parents 76f4d9b + 9c4d044 commit 15108c1
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 115 deletions.
95 changes: 0 additions & 95 deletions extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<InjectionPointInfo> 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();

Expand Down Expand Up @@ -793,60 +752,6 @@ void registerContextPropagation(ArcConfig config, BuildProducer<ThreadContextPro
}
}

private void registerUnremovableListBeans(BeanRegistrationPhaseBuildItem beanRegistrationPhase,
List<InjectionPointInfo> injectionPoints, BuildProducer<ReflectiveMethodBuildItem> reflectiveMethods,
BuildProducer<ReflectiveFieldBuildItem> reflectiveFields,
BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
BeanDeployment beanDeployment = beanRegistrationPhase.getBeanProcessor().getBeanDeployment();
List<TypeAndQualifiers> unremovables = new ArrayList<>();

for (InjectionPointInfo injectionPoint : injectionPoints) {
// All qualifiers but @All
Set<AnnotationInstance> qualifiers = new HashSet<>(injectionPoint.getRequiredQualifiers());
for (Iterator<AnnotationInstance> 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<X> 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<BeanInfo>() {
@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<T> implements Predicate<T> {

private final IndexView applicationClassesIndex;
Expand Down
3 changes: 2 additions & 1 deletion ...pendent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ private Set<DecoratorInfo> removeUnusedDecorators(Set<DecoratorInfo> removedDeco
}

private Set<BeanInfo> removeUnusedBeans(Set<BeanInfo> declaresObserver, List<Predicate<BeanInfo>> allUnusedExclusions) {
Set<BeanInfo> removableBeans = UnusedBeans.findRemovableBeans(this.beans, this.injectionPoints, declaresObserver,
Set<BeanInfo> removableBeans = UnusedBeans.findRemovableBeans(beanResolver, this.beans, this.injectionPoints,
declaresObserver,
allUnusedExclusions);
if (!removableBeans.isEmpty()) {
this.beans.removeAll(removableBeans);
Expand Down
35 changes: 34 additions & 1 deletion independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BuiltinBean.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -443,6 +444,38 @@ private static void validateInstance(InjectionTargetInfo injectionTarget, Inject
}
}

private static void validateList(InjectionTargetInfo injectionTarget, InjectionPointInfo injectionPoint,
Consumer<Throwable> 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<Throwable> errors) {
if (injectionTarget.kind() != TargetKind.BEAN || !BuiltinScope.DEPENDENT.is(injectionTarget.asBean().getScope())) {
Expand Down
65 changes: 52 additions & 13 deletions independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/UnusedBeans.java
View file
Original file line number Diff line number Diff line change
@@ -1,41 +1,72 @@
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;
import java.util.Set;
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);

private UnusedBeans() {
}

static Set<BeanInfo> findRemovableBeans(Collection<BeanInfo> beans, Collection<InjectionPointInfo> injectionPoints,
Set<BeanInfo> declaresObserver, List<Predicate<BeanInfo>> allUnusedExclusions) {
static Set<BeanInfo> findRemovableBeans(BeanResolver beanResolver, Collection<BeanInfo> beans,
Iterable<InjectionPointInfo> injectionPoints, Set<BeanInfo> declaresObserver,
List<Predicate<BeanInfo>> allUnusedExclusions) {
Set<BeanInfo> removableBeans = new HashSet<>();

Set<BeanInfo> unusedProducers = new HashSet<>();
Set<BeanInfo> unusedButDeclaresProducer = new HashSet<>();
List<BeanInfo> producers = beans.stream().filter(BeanInfo::isProducer)
.collect(Collectors.toList());
List<InjectionPointInfo> 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<BeanInfo> 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<BeanInfo> injected = new HashSet<>();
List<InjectionPointInfo> instanceInjectionPoints = new ArrayList<>();
List<TypeAndQualifiers> 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<AnnotationInstance> qualifiers = new HashSet<>(injectionPoint.getRequiredQualifiers());
for (Iterator<AnnotationInstance> it = qualifiers.iterator(); it.hasNext();) {
AnnotationInstance qualifier = it.next();
if (qualifier.name().equals(DotNames.ALL)) {
it.remove();
}
}
listAllInjectionPoints.add(new TypeAndQualifiers(requiredType, qualifiers));
}
}
}
}
Set<BeanInfo> declaresProducer = producers.stream().map(BeanInfo::getDeclaringBean).collect(Collectors.toSet());

// Beans - first pass to find unused beans that do not declare a producer
Expand Down Expand Up @@ -70,12 +101,20 @@ static Set<BeanInfo> findRemovableBeans(Collection<BeanInfo> beans, Collection<I
// Instance<Foo>
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<Foo>
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);
Expand Down
1 change: 0 additions & 1 deletion independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/all/ListAllTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 15 additions & 4 deletions ...nt-projects/arc/tests/src/test/java/io/quarkus/arc/test/unused/RemoveUnusedBeansTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.List;

import jakarta.annotation.PostConstruct;
import jakarta.annotation.Priority;
Expand All @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -67,6 +68,7 @@ public void testRemoval() {
assertFalse(ArcContainerImpl.instance().getRemovedBeans().isEmpty());
assertNotPresent(UnusedBean.class);
assertNotPresent(OnlyInjectedInUnusedBean.class);
assertPresent(UsedViaAllList.class);
}

@Dependent
Expand Down Expand Up @@ -197,6 +199,10 @@ static class UsesBeanViaInstance {

@Inject
Instance<UsedViaInstanceWithUnusedProducer> instance;

@Inject
@All
List<UsedViaAllList> list;
}

@Singleton
Expand All @@ -212,4 +218,9 @@ static class OnlyInjectedInUnusedBean {

}

@Singleton
static class UsedViaAllList {

}

}

0 comments on commit 15108c1

Please sign in to comment.