ArC: defer unproxyability errors to runtime in strict mode

The CDI specification demands that if an injection point resolves
to a bean, unproxyability of that bean should be treated as
a deployment problem. ArC used to treat all unproxyable beans
uniformly, even those that are not injected anywhere. This commit
changes that: in strict mode, only unproxyable beans that are
actually injected somewhere trigger a deployment-time error.
Unproxyable beans that are not injected anywhere do not cause
a deployment-time error and instead, the `Bean` class (its
`get(CreationalContext)` method, specifically) is generated
in a way that `UnproxyableResolutionException` is thrown when
obtaining a contextual reference, as expected by the CDI spec.

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java

@@ -104,6 +104,8 @@ public class BeanDeployment {
 
     private final Set<BeanInfo> removedBeans;
 
+    private final Set<BeanInfo> beansWithRuntimeDeferredUnproxyableError;
+
     private final Map<ScopeInfo, Function<MethodCreator, ResultHandle>> customContexts;
 
     private final Map<DotName, BeanDefiningAnnotation> beanDefiningAnnotations;
@@ -149,6 +151,7 @@ public class BeanDeployment {
         this.removeUnusedBeans = builder.removeUnusedBeans;
         this.unusedExclusions = removeUnusedBeans ? new ArrayList<>(builder.removalExclusions) : null;
         this.removedBeans = removeUnusedBeans ? new CopyOnWriteArraySet<>() : Collections.emptySet();
+        this.beansWithRuntimeDeferredUnproxyableError = Collections.newSetFromMap(new ConcurrentHashMap<>());
         this.customContexts = new ConcurrentHashMap<>();
 
         this.excludeTypes = builder.excludeTypes != null ? new ArrayList<>(builder.excludeTypes) : Collections.emptyList();
@@ -506,6 +509,14 @@ public Collection<BeanInfo> getRemovedBeans() {
         return Collections.unmodifiableSet(removedBeans);
     }
 
+    boolean hasRuntimeDeferredUnproxyableError(BeanInfo bean) {
+        return beansWithRuntimeDeferredUnproxyableError.contains(bean);
+    }
+
+    void deferUnproxyableErrorToRuntime(BeanInfo bean) {
+        beansWithRuntimeDeferredUnproxyableError.add(bean);
+    }
+
     public Collection<ClassInfo> getQualifiers() {
         return Collections.unmodifiableCollection(qualifiers.values());
     }
@@ -1522,6 +1533,17 @@ private void validateBeans(List<Throwable> errors, Consumer<BytecodeTransformer>
         Map<String, List<BeanInfo>> namedBeans = new HashMap<>();
         Set<DotName> classesReceivingNoArgsCtor = new HashSet<>();
 
+        // this set is only used in strict compatible mode (see `Beans.validateBean()`),
+        // so no need to initialize it otherwise
+        Set<BeanInfo> injectedBeans = new HashSet<>();
+        if (strictCompatibility) {
+            for (InjectionPointInfo injectionPoint : this.injectionPoints) {
+                if (injectionPoint.hasResolvedBean()) {
+                    injectedBeans.add(injectionPoint.getResolvedBean());
+                }
+            }
+        }
+
         for (BeanInfo bean : beans) {
             if (bean.getName() != null) {
                 List<BeanInfo> named = namedBeans.get(bean.getName());
@@ -1532,7 +1554,7 @@ private void validateBeans(List<Throwable> errors, Consumer<BytecodeTransformer>
                 named.add(bean);
                 findNamespaces(bean, namespaces);
             }
-            bean.validate(errors, bytecodeTransformerConsumer, classesReceivingNoArgsCtor);
+            bean.validate(errors, bytecodeTransformerConsumer, classesReceivingNoArgsCtor, injectedBeans);
         }
 
         if (!namedBeans.isEmpty()) {

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java

@@ -30,6 +30,7 @@
 import jakarta.enterprise.context.spi.CreationalContext;
 import jakarta.enterprise.inject.CreationException;
 import jakarta.enterprise.inject.IllegalProductException;
+import jakarta.enterprise.inject.UnproxyableResolutionException;
 import jakarta.enterprise.inject.literal.InjectLiteral;
 import jakarta.enterprise.inject.spi.InterceptionType;
 import jakarta.interceptor.InvocationContext;
@@ -1927,7 +1928,10 @@ protected void implementGet(BeanInfo bean, ClassCreator beanCreator, ProviderTyp
         MethodCreator get = beanCreator.getMethodCreator("get", providerType.descriptorName(), CreationalContext.class)
                 .setModifiers(ACC_PUBLIC);
 
-        if (BuiltinScope.DEPENDENT.is(bean.getScope())) {
+        if (bean.getDeployment().hasRuntimeDeferredUnproxyableError(bean)) {
+            get.throwException(UnproxyableResolutionException.class, "Bean not proxyable: " + bean);
+            get.returnValue(get.loadNull());
+        } else if (BuiltinScope.DEPENDENT.is(bean.getScope())) {
             // @Dependent pseudo-scope
             // Foo instance = create(ctx)
             ResultHandle instance = get.invokeVirtualMethod(
@@ -2214,6 +2218,10 @@ private ResultHandle wrapCurrentInjectionPoint(BeanInfo bean,
     }
 
     private void initializeProxy(BeanInfo bean, String baseName, ClassCreator beanCreator) {
+        if (bean.getDeployment().hasRuntimeDeferredUnproxyableError(bean)) {
+            return;
+        }
+
         // Add proxy volatile field
         String proxyTypeName = getProxyTypeName(bean, baseName);
         beanCreator.getFieldCreator(FIELD_NAME_PROXY, proxyTypeName)

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java

@@ -582,8 +582,8 @@ public String getClientProxyPackageName() {
     }
 
     void validate(List<Throwable> errors, Consumer<BytecodeTransformer> bytecodeTransformerConsumer,
-            Set<DotName> classesReceivingNoArgsCtor) {
-        Beans.validateBean(this, errors, bytecodeTransformerConsumer, classesReceivingNoArgsCtor);
+            Set<DotName> classesReceivingNoArgsCtor, Set<BeanInfo> injectedBeans) {
+        Beans.validateBean(this, errors, bytecodeTransformerConsumer, classesReceivingNoArgsCtor, injectedBeans);
     }
 
     void validateInterceptorDecorator(List<Throwable> errors, Consumer<BytecodeTransformer> bytecodeTransformerConsumer) {

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Beans.java

@@ -711,25 +711,34 @@ static void validateInterceptorDecorator(BeanInfo bean, List<Throwable> errors,
     }
 
     static void validateBean(BeanInfo bean, List<Throwable> errors, Consumer<BytecodeTransformer> bytecodeTransformerConsumer,
-            Set<DotName> classesReceivingNoArgsCtor) {
+            Set<DotName> classesReceivingNoArgsCtor, Set<BeanInfo> injectedBeans) {
+
+        // by default, we fail deployment due to unproxyability for all beans, but in strict mode,
+        // we only do that for beans that are injected somewhere -- and defer the error to runtime otherwise,
+        // due to CDI spec requirements
+        boolean failIfNotProxyable = bean.getDeployment().strictCompatibility ? injectedBeans.contains(bean) : true;
+
         if (bean.isClassBean()) {
             ClassInfo beanClass = bean.getTarget().get().asClass();
             String classifier = bean.getScope().isNormal() ? "Normal scoped" : null;
             if (classifier == null && bean.isSubclassRequired()) {
                 classifier = "Intercepted";
+                failIfNotProxyable = true;
             }
             if (Modifier.isFinal(beanClass.flags()) && classifier != null) {
                 // Client proxies and subclasses require a non-final class
                 if (bean.getDeployment().transformUnproxyableClasses) {
                     bytecodeTransformerConsumer
                             .accept(new BytecodeTransformer(beanClass.name().toString(), new FinalClassTransformFunction()));
-                } else {
+                } else if (failIfNotProxyable) {
                     errors.add(new DeploymentException(String.format("%s bean must not be final: %s", classifier, bean)));
+                } else {
+                    bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                 }
             }
             if (bean.getDeployment().strictCompatibility && classifier != null) {
-                validateNonStaticFinalMethods(beanClass, bean.getDeployment().getBeanArchiveIndex(),
-                        classifier, errors);
+                validateNonStaticFinalMethods(bean, beanClass, bean.getDeployment().getBeanArchiveIndex(),
+                        classifier, errors, failIfNotProxyable);
             }
 
             MethodInfo noArgsConstructor = beanClass.method(Methods.INIT);
@@ -754,13 +763,16 @@ static void validateBean(BeanInfo bean, List<Throwable> errors, Consumer<Bytecod
                             classesReceivingNoArgsCtor.add(beanClass.name());
                         }
 
-                    } else {
+                    } else if (failIfNotProxyable) {
                         errors.add(cannotAddSyntheticNoArgsConstructor(beanClass));
+                    } else {
+                        bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                     }
+                } else if (failIfNotProxyable) {
+                    errors.add(new DeploymentException(String.format(
+                            "Normal scoped beans must declare a non-private constructor with no parameters: %s", bean)));
                 } else {
-                    errors.add(new DeploymentException(String
-                            .format("Normal scoped beans must declare a non-private constructor with no parameters: %s",
-                                    bean)));
+                    bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                 }
             }
 
@@ -770,12 +782,12 @@ static void validateBean(BeanInfo bean, List<Throwable> errors, Consumer<Bytecod
                     bytecodeTransformerConsumer.accept(
                             new BytecodeTransformer(beanClass.name().toString(),
                                     new PrivateNoArgsConstructorTransformFunction()));
+                } else if (failIfNotProxyable) {
+                    errors.add(new DeploymentException(String.format(
+                            "%s bean is not proxyable because it has a private no-args constructor: %s. To fix this problem, change the constructor to be package-private",
+                            classifier, bean)));
                 } else {
-                    errors.add(
-                            new DeploymentException(
-                                    String.format(
-                                            "%s bean is not proxyable because it has a private no-args constructor: %s. To fix this problem, change the constructor to be package-private",
-                                            classifier, bean)));
+                    bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                 }
             }
 
@@ -811,15 +823,16 @@ static void validateBean(BeanInfo bean, List<Throwable> errors, Consumer<Bytecod
                         bytecodeTransformerConsumer
                                 .accept(new BytecodeTransformer(returnTypeClass.name().toString(),
                                         new FinalClassTransformFunction()));
+                    } else if (failIfNotProxyable) {
+                        errors.add(new DeploymentException(
+                                String.format("%s must not have a return type that is final: %s", classifier, bean)));
                     } else {
-                        errors.add(
-                                new DeploymentException(String.format("%s must not have a" +
-                                        " return type that is final: %s", classifier, bean)));
+                        bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                     }
                 }
                 if (bean.getDeployment().strictCompatibility) {
-                    validateNonStaticFinalMethods(returnTypeClass, bean.getDeployment().getBeanArchiveIndex(),
-                            classifier, errors);
+                    validateNonStaticFinalMethods(bean, returnTypeClass, bean.getDeployment().getBeanArchiveIndex(),
+                            classifier, errors, failIfNotProxyable);
                 }
                 MethodInfo noArgsConstructor = returnTypeClass.method(Methods.INIT);
                 if (noArgsConstructor == null) {
@@ -844,22 +857,23 @@ static void validateBean(BeanInfo bean, List<Throwable> errors, Consumer<Bytecod
                         } else {
                             errors.add(cannotAddSyntheticNoArgsConstructor(returnTypeClass));
                         }
+                    } else if (failIfNotProxyable) {
+                        errors.add(new DefinitionException(String.format(
+                                "Return type of a producer %s for normal scoped beans must declare a non-private constructor with no parameters: %s",
+                                methodOrField, bean)));
                     } else {
-                        errors.add(new DefinitionException(String
-                                .format("Return type of a producer %s for normal scoped beans must" +
-                                        " declare a non-private constructor with no parameters: %s", methodOrField, bean)));
+                        bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                     }
                 } else if (Modifier.isPrivate(noArgsConstructor.flags())) {
                     if (bean.getDeployment().transformUnproxyableClasses) {
                         bytecodeTransformerConsumer.accept(
                                 new BytecodeTransformer(returnTypeClass.name().toString(),
                                         new PrivateNoArgsConstructorTransformFunction()));
+                    } else if (failIfNotProxyable) {
+                        errors.add(new DeploymentException(String.format(
+                                "%s is not proxyable because it has a private no-args constructor: %s.", classifier, bean)));
                     } else {
-                        errors.add(
-                                new DeploymentException(
-                                        String.format(
-                                                "%s is not proxyable because it has a private no-args constructor: %s.",
-                                                classifier, bean)));
+                        bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                     }
                 }
             }
@@ -894,21 +908,23 @@ static void validateBean(BeanInfo bean, List<Throwable> errors, Consumer<Bytecod
                     } else {
                         errors.add(cannotAddSyntheticNoArgsConstructor(beanClass));
                     }
+                } else if (failIfNotProxyable) {
+                    errors.add(new DeploymentException(String.format(
+                            "Normal scoped beans must declare a non-private constructor with no parameters: %s", bean)));
                 } else {
-                    errors.add(new DeploymentException(String
-                            .format("Normal scoped beans must declare a non-private constructor with no parameters: %s",
-                                    bean)));
+                    bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
                 }
             }
             if (bean.getScope().isNormal() && !Modifier.isInterface(beanClass.flags())
                     && bean.getDeployment().strictCompatibility) {
-                validateNonStaticFinalMethods(beanClass, bean.getDeployment().getBeanArchiveIndex(), "Normal scoped", errors);
+                validateNonStaticFinalMethods(bean, beanClass, bean.getDeployment().getBeanArchiveIndex(), "Normal scoped",
+                        errors, failIfNotProxyable);
             }
         }
     }
 
-    private static void validateNonStaticFinalMethods(ClassInfo clazz, IndexView beanArchiveIndex,
-            String classifier, List<Throwable> errors) {
+    private static void validateNonStaticFinalMethods(BeanInfo bean, ClassInfo clazz, IndexView beanArchiveIndex,
+            String classifier, List<Throwable> errors, boolean failIfNotProxyable) {
         // see also Methods.skipForClientProxy()
         while (!clazz.name().equals(DotNames.OBJECT)) {
             for (MethodInfo method : clazz.methods()) {
@@ -920,9 +936,13 @@ private static void validateNonStaticFinalMethods(ClassInfo clazz, IndexView bea
                 }
 
                 if (Modifier.isFinal(method.flags())) {
-                    errors.add(new DeploymentException(String.format(
-                            "%s bean must not declare non-static final methods with public, protected or default visibility: %s",
-                            classifier, method)));
+                    if (failIfNotProxyable) {
+                        errors.add(new DeploymentException(String.format(
+                                "%s bean must not declare non-static final methods with public, protected or default visibility: %s",
+                                classifier, method)));
+                    } else {
+                        bean.getDeployment().deferUnproxyableErrorToRuntime(bean);
+                    }
                 }
             }
 

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java

@@ -78,6 +78,12 @@ public ClientProxyGenerator(Predicate<DotName> applicationClassPredicate, boolea
     Collection<Resource> generate(BeanInfo bean, String beanClassName,
             Consumer<BytecodeTransformer> bytecodeTransformerConsumer, boolean transformUnproxyableClasses) {
 
+        // see `BeanGenerator` -- if this bean is unproxyable and that error is deferred to runtime,
+        // we don't need to (and cannot, in fact) generate the client proxy class
+        if (bean.getDeployment().hasRuntimeDeferredUnproxyableError(bean)) {
+            return Collections.emptySet();
+        }
+
         ProviderType providerType = new ProviderType(bean.getProviderType());
         ClassInfo providerClass = getClassByName(bean.getDeployment().getBeanArchiveIndex(), providerType.name());
         String baseName = getBaseName(bean, beanClassName);

independent-projects/arc/tcks/cdi-tck-runner/src/test/resources/testng.xml

@@ -70,15 +70,6 @@
                     <exclude name="testGetBean"/>
                 </methods>
             </class>
-            <class name="org.jboss.cdi.tck.tests.implementation.simple.lifecycle.unproxyable.UnproxyableManagedBeanTest">
-                <methods>
-                    <exclude name="testNormalScopedUnproxyableBeanWithPublicFinalMethodResolution"/>
-                    <exclude name="testNormalScopedUnproxyableBeanWithPrivateConstructorResolution"/>
-                    <exclude name="testNormalScopedUnproxyableBeanWithFinalClassResolution"/>
-                    <exclude name="testNormalScopedUnproxyableBeanWithPackagePrivateFinalMethodResolution"/>
-                    <exclude name="testNormalScopedUnproxyableBeanWithProtectedFinalMethodResolution"/>
-                </methods>
-            </class>
             <class name="org.jboss.cdi.tck.tests.interceptors.definition.InterceptorDefinitionTest">
                 <methods>
                     <!-- https://github.com/jakartaee/cdi-tck/issues/455 -->

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIllegalTest.java → independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIllegalWhenInjectedTest.java

@@ -5,17 +5,19 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.context.Dependent;
 import jakarta.enterprise.inject.spi.DeploymentException;
+import jakarta.inject.Inject;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import io.quarkus.arc.test.ArcTestContainer;
 
-public class FinalMethodIllegalTest {
+public class FinalMethodIllegalWhenInjectedTest {
     @RegisterExtension
     public ArcTestContainer container = ArcTestContainer.builder()
-            .beanClasses(Moo.class)
+            .beanClasses(Moo.class, MooConsumer.class)
             .strictCompatibility(true)
             .shouldFail()
             .build();
@@ -34,4 +36,11 @@ final int getVal() {
             return -1;
         }
     }
+
+    // to trigger deployment-time error (in strict compatible mode)
+    @Dependent
+    static class MooConsumer {
+        @Inject
+        Moo moo;
+    }
 }

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/clientproxy/finalmethod/FinalMethodIllegalWhenNotInjectedTest.java

@@ -0,0 +1,35 @@
+package io.quarkus.arc.test.clientproxy.finalmethod;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.UnproxyableResolutionException;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.Arc;
+import io.quarkus.arc.test.ArcTestContainer;
+
+public class FinalMethodIllegalWhenNotInjectedTest {
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder()
+            .beanClasses(Moo.class)
+            .strictCompatibility(true)
+            .shouldFail()
+            .build();
+
+    @Test
+    public void test() {
+        assertThrows(UnproxyableResolutionException.class, () -> {
+            Arc.container().instance(Moo.class).get();
+        });
+    }
+
+    @ApplicationScoped
+    static class Moo {
+        final int getVal() {
+            return -1;
+        }
+    }
+}