Fix hints for @Bean config props that could be constructor bound

Previously, if a `@ConfigurationProperties`-annotated `@Bean` method
returned a type that looked like it could be constructor bound, the
registered runtime hints were incorrect. With only the bean's class
to work with, the hints registrar would incorrectly determine that
the type would be constructor bound and would not register the hints
required for Java bean binding.

This commit updates the registrar to allow the caller to provide a
Bindable which knows both what should be bound and how it should be
bound, thereby allowing the registrar to generate the correct hints.
The tests for the AOT processor have also been updated to remove
duplication of the tests in BindableRuntimeHintsRegistrarTests and
to focus on the contribution creating Bindable instances with the
bind method that is required to produce the correct reflection hints.

Closes gh-35564

Co-authored-by: Phillip Webb <pwebb@vmware.com>

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanFactoryInitializationAotProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,9 +24,10 @@ import org.springframework.beans.factory.aot.BeanFactoryInitializationAotContrib
 import org.springframework.beans.factory.aot.BeanFactoryInitializationAotProcessor;
 import org.springframework.beans.factory.aot.BeanFactoryInitializationCode;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.boot.context.properties.bind.BindMethod;
+import org.springframework.boot.context.properties.bind.Bindable;
 import org.springframework.boot.context.properties.bind.BindableRuntimeHintsRegistrar;
 import org.springframework.util.ClassUtils;
-import org.springframework.util.CollectionUtils;
 
 /**
  * {@link BeanFactoryInitializationAotProcessor} that contributes runtime hints for
@@ -43,36 +44,38 @@ class ConfigurationPropertiesBeanFactoryInitializationAotProcessor implements Be
 	public ConfigurationPropertiesReflectionHintsContribution processAheadOfTime(
 			ConfigurableListableBeanFactory beanFactory) {
 		String[] beanNames = beanFactory.getBeanNamesForAnnotation(ConfigurationProperties.class);
-		List<Class<?>> types = new ArrayList<>();
+		List<Bindable<?>> bindables = new ArrayList<>();
 		for (String beanName : beanNames) {
 			Class<?> beanType = beanFactory.getType(beanName, false);
 			if (beanType != null) {
-				types.add(ClassUtils.getUserClass(beanType));
+				BindMethod bindMethod = beanFactory.containsBeanDefinition(beanName)
+						? (BindMethod) beanFactory.getBeanDefinition(beanName).getAttribute(BindMethod.class.getName())
+						: null;
+				bindables.add(Bindable.of(ClassUtils.getUserClass(beanType))
+					.withBindMethod((bindMethod != null) ? bindMethod : BindMethod.JAVA_BEAN));
 			}
 		}
-		if (!CollectionUtils.isEmpty(types)) {
-			return new ConfigurationPropertiesReflectionHintsContribution(types);
-		}
-		return null;
+		return (!bindables.isEmpty()) ? new ConfigurationPropertiesReflectionHintsContribution(bindables) : null;
 	}
 
 	static final class ConfigurationPropertiesReflectionHintsContribution
 			implements BeanFactoryInitializationAotContribution {
 
-		private final Iterable<Class<?>> types;
+		private final List<Bindable<?>> bindables;
 
-		private ConfigurationPropertiesReflectionHintsContribution(Iterable<Class<?>> types) {
-			this.types = types;
+		private ConfigurationPropertiesReflectionHintsContribution(List<Bindable<?>> bindables) {
+			this.bindables = bindables;
 		}
 
 		@Override
 		public void applyTo(GenerationContext generationContext,
 				BeanFactoryInitializationCode beanFactoryInitializationCode) {
-			BindableRuntimeHintsRegistrar.forTypes(this.types).registerHints(generationContext.getRuntimeHints());
+			BindableRuntimeHintsRegistrar.forBindables(this.bindables)
+				.registerHints(generationContext.getRuntimeHints());
 		}
 
-		Iterable<Class<?>> getTypes() {
-			return this.types;
+		Iterable<Bindable<?>> getBindables() {
+			return this.bindables;
 		}
 
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindableRuntimeHintsRegistrar.java

@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
 import kotlin.jvm.JvmClassMappingKt;
@@ -51,8 +52,8 @@ import org.springframework.util.ReflectionUtils;
  * {@link RuntimeHintsRegistrar} that can be used to register {@link ReflectionHints} for
  * {@link Bindable} types, discovering any nested type it may expose through a property.
  * <p>
- * This class can be used as a base-class, or instantiated using the {@code forTypes}
- * factory methods.
+ * This class can be used as a base-class, or instantiated using the {@code forTypes} and
+ * {@code forBindables} factory methods.
  *
  * @author Andy Wilkinson
  * @author Moritz Halbritter
@@ -62,14 +63,23 @@ import org.springframework.util.ReflectionUtils;
  */
 public class BindableRuntimeHintsRegistrar implements RuntimeHintsRegistrar {
 
-	private final Class<?>[] types;
+	private final Bindable<?>[] bindables;
 
 	/**
 	 * Create a new {@link BindableRuntimeHintsRegistrar} for the specified types.
 	 * @param types the types to process
 	 */
 	protected BindableRuntimeHintsRegistrar(Class<?>... types) {
-		this.types = types;
+		this(Stream.of(types).map(Bindable::of).toArray(Bindable[]::new));
+	}
+
+	/**
+	 * Create a new {@link BindableRuntimeHintsRegistrar} for the specified bindables.
+	 * @param bindables the bindables to process
+	 * @since 3.0.8
+	 */
+	protected BindableRuntimeHintsRegistrar(Bindable<?>... bindables) {
+		this.bindables = bindables;
 	}
 
 	@Override
@@ -83,8 +93,8 @@ public class BindableRuntimeHintsRegistrar implements RuntimeHintsRegistrar {
 	 */
 	public void registerHints(RuntimeHints hints) {
 		Set<Class<?>> compiledWithoutParameters = new HashSet<>();
-		for (Class<?> type : this.types) {
-			new Processor(type, compiledWithoutParameters).process(hints.reflection());
+		for (Bindable<?> bindable : this.bindables) {
+			new Processor(bindable, compiledWithoutParameters).process(hints.reflection());
 		}
 		if (!compiledWithoutParameters.isEmpty()) {
 			throw new MissingParametersCompilerArgumentException(compiledWithoutParameters);
@@ -110,6 +120,27 @@ public class BindableRuntimeHintsRegistrar implements RuntimeHintsRegistrar {
 		return new BindableRuntimeHintsRegistrar(types);
 	}
 
+	/**
+	 * Create a new {@link BindableRuntimeHintsRegistrar} for the specified bindables.
+	 * @param bindables the bindables to process
+	 * @return a new {@link BindableRuntimeHintsRegistrar} instance
+	 * @since 3.0.8
+	 */
+	public static BindableRuntimeHintsRegistrar forBindables(Iterable<Bindable<?>> bindables) {
+		Assert.notNull(bindables, "Bindables must not be null");
+		return forBindables(StreamSupport.stream(bindables.spliterator(), false).toArray(Bindable[]::new));
+	}
+
+	/**
+	 * Create a new {@link BindableRuntimeHintsRegistrar} for the specified bindables.
+	 * @param bindables the bindables to process
+	 * @return a new {@link BindableRuntimeHintsRegistrar} instance
+	 * @since 3.0.8
+	 */
+	public static BindableRuntimeHintsRegistrar forBindables(Bindable<?>... bindables) {
+		return new BindableRuntimeHintsRegistrar(bindables);
+	}
+
 	/**
 	 * Processor used to register the hints.
 	 */
@@ -136,15 +167,17 @@ public class BindableRuntimeHintsRegistrar implements RuntimeHintsRegistrar {
 
 		private final Set<Class<?>> compiledWithoutParameters;
 
-		Processor(Class<?> type, Set<Class<?>> compiledWithoutParameters) {
-			this(type, false, new HashSet<>(), compiledWithoutParameters);
+		Processor(Bindable<?> bindable, Set<Class<?>> compiledWithoutParameters) {
+			this(bindable, false, new HashSet<>(), compiledWithoutParameters);
 		}
 
-		private Processor(Class<?> type, boolean nestedType, Set<Class<?>> seen,
+		private Processor(Bindable<?> bindable, boolean nestedType, Set<Class<?>> seen,
 				Set<Class<?>> compiledWithoutParameters) {
-			this.type = type;
-			this.bindConstructor = BindConstructorProvider.DEFAULT.getBindConstructor(Bindable.of(type), nestedType);
-			this.bean = JavaBeanBinder.BeanProperties.of(Bindable.of(type));
+			this.type = bindable.getType().getRawClass();
+			this.bindConstructor = (bindable.getBindMethod() != BindMethod.JAVA_BEAN)
+					? BindConstructorProvider.DEFAULT.getBindConstructor(bindable.getType().resolve(), nestedType)
+					: null;
+			this.bean = JavaBeanBinder.BeanProperties.of(bindable);
 			this.seen = seen;
 			this.compiledWithoutParameters = compiledWithoutParameters;
 		}
@@ -235,7 +268,7 @@ public class BindableRuntimeHintsRegistrar implements RuntimeHintsRegistrar {
 		}
 
 		private void processNested(Class<?> type, ReflectionHints hints) {
-			new Processor(type, true, this.seen, this.compiledWithoutParameters).process(hints);
+			new Processor(Bindable.of(type), true, this.seen, this.compiledWithoutParameters).process(hints);
 		}
 
 		private Class<?> getComponentClass(ResolvableType type) {

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanFactoryInitializationAotProcessorTests.java

@@ -18,22 +18,26 @@ package org.springframework.boot.context.properties;
 
 import java.util.stream.Stream;
 
+import org.assertj.core.api.AbstractAssert;
+import org.assertj.core.api.AssertProvider;
+import org.assertj.core.error.BasicErrorMessageFactory;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.aot.generate.GenerationContext;
 import org.springframework.aot.hint.TypeHint;
 import org.springframework.aot.hint.TypeReference;
 import org.springframework.aot.test.generate.TestGenerationContext;
 import org.springframework.beans.factory.aot.AotServices;
 import org.springframework.beans.factory.aot.BeanFactoryInitializationAotProcessor;
-import org.springframework.beans.factory.aot.BeanFactoryInitializationCode;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
-import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.boot.context.properties.ConfigurationPropertiesBeanFactoryInitializationAotProcessor.ConfigurationPropertiesReflectionHintsContribution;
+import org.springframework.boot.context.properties.bind.BindMethod;
+import org.springframework.boot.context.properties.bind.Bindable;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
 
 /**
  * Tests for {@link ConfigurationPropertiesBeanFactoryInitializationAotProcessor}.
@@ -41,6 +45,7 @@ import static org.mockito.Mockito.mock;
  * @author Stephane Nicoll
  * @author Moritz Halbritter
  * @author Sebastien Deleuze
+ * @author Andy Wilkinson
  */
 class ConfigurationPropertiesBeanFactoryInitializationAotProcessorTests {
 
@@ -58,44 +63,192 @@ class ConfigurationPropertiesBeanFactoryInitializationAotProcessorTests {
 	}
 
 	@Test
-	void processManuallyRegisteredSingleton() {
+	void manuallyRegisteredSingletonBindsAsJavaBean() {
 		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		beanFactory.registerSingleton("test", new SampleProperties());
 		ConfigurationPropertiesReflectionHintsContribution contribution = process(beanFactory);
-		assertThat(contribution.getTypes()).containsExactly(SampleProperties.class);
+		assertThat(singleBindable(contribution)).hasBindMethod(BindMethod.JAVA_BEAN).hasType(SampleProperties.class);
 		assertThat(typeHints(contribution).map(TypeHint::getType))
 			.containsExactly(TypeReference.of(SampleProperties.class));
 	}
 
 	@Test
-	void processDefinedBean() {
-		ConfigurationPropertiesReflectionHintsContribution contribution = process(SampleProperties.class);
-		assertThat(contribution.getTypes()).containsExactly(SampleProperties.class);
+	void javaBeanConfigurationPropertiesBindAsJavaBean() {
+		ConfigurationPropertiesReflectionHintsContribution contribution = process(EnableJavaBeanProperties.class);
+		assertThat(singleBindable(contribution)).hasBindMethod(BindMethod.JAVA_BEAN).hasType(JavaBeanProperties.class);
 		assertThat(typeHints(contribution).map(TypeHint::getType))
-			.containsExactly(TypeReference.of(SampleProperties.class));
+			.containsExactly(TypeReference.of(JavaBeanProperties.class));
 	}
 
-	Stream<TypeHint> typeHints(ConfigurationPropertiesReflectionHintsContribution contribution) {
-		GenerationContext generationContext = new TestGenerationContext();
-		contribution.applyTo(generationContext, mock(BeanFactoryInitializationCode.class));
+	@Test
+	void constructorBindingConfigurationPropertiesBindAsValueObject() {
+		ConfigurationPropertiesReflectionHintsContribution contribution = process(
+				EnableConstructorBindingProperties.class);
+		assertThat(singleBindable(contribution)).hasBindMethod(BindMethod.VALUE_OBJECT)
+			.hasType(ConstructorBindingProperties.class);
+		assertThat(typeHints(contribution).map(TypeHint::getType))
+			.containsExactly(TypeReference.of(ConstructorBindingProperties.class));
+	}
+
+	@Test
+	void possibleConstructorBindingPropertiesDefinedThroughBeanMethodBindAsJavaBean() {
+		ConfigurationPropertiesReflectionHintsContribution contribution = process(
+				PossibleConstructorBindingPropertiesBeanMethodConfiguration.class);
+		assertThat(singleBindable(contribution)).hasBindMethod(BindMethod.JAVA_BEAN)
+			.hasType(PossibleConstructorBindingProperties.class);
+		assertThat(typeHints(contribution).map(TypeHint::getType))
+			.containsExactly(TypeReference.of(PossibleConstructorBindingProperties.class));
+	}
+
+	@Test
+	void possibleConstructorBindingPropertiesDefinedThroughEnabledAnnotationBindAsValueObject() {
+		ConfigurationPropertiesReflectionHintsContribution contribution = process(
+				EnablePossibleConstructorBindingProperties.class);
+		assertThat(singleBindable(contribution)).hasBindMethod(BindMethod.VALUE_OBJECT)
+			.hasType(PossibleConstructorBindingProperties.class);
+		assertThat(typeHints(contribution).map(TypeHint::getType))
+			.containsExactly(TypeReference.of(PossibleConstructorBindingProperties.class));
+	}
+
+	private Stream<TypeHint> typeHints(ConfigurationPropertiesReflectionHintsContribution contribution) {
+		TestGenerationContext generationContext = new TestGenerationContext();
+		contribution.applyTo(generationContext, null);
 		return generationContext.getRuntimeHints().reflection().typeHints();
 	}
 
-	private ConfigurationPropertiesReflectionHintsContribution process(Class<?>... types) {
-		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
-		for (Class<?> type : types) {
-			beanFactory.registerBeanDefinition(type.getName(), new RootBeanDefinition(type));
+	private ConfigurationPropertiesReflectionHintsContribution process(Class<?> config) {
+		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(config)) {
+			return process(context.getBeanFactory());
 		}
-		return process(beanFactory);
 	}
 
 	private ConfigurationPropertiesReflectionHintsContribution process(ConfigurableListableBeanFactory beanFactory) {
 		return this.processor.processAheadOfTime(beanFactory);
 	}
 
+	private BindableAssertProvider singleBindable(ConfigurationPropertiesReflectionHintsContribution contribution) {
+		assertThat(contribution.getBindables()).hasSize(1);
+		return new BindableAssertProvider(contribution.getBindables().iterator().next());
+	}
+
 	@ConfigurationProperties("test")
 	static class SampleProperties {
 
 	}
 
+	@EnableConfigurationProperties(JavaBeanProperties.class)
+	static class EnableJavaBeanProperties {
+
+	}
+
+	@ConfigurationProperties("java-bean")
+	static class JavaBeanProperties {
+
+		private String value;
+
+		String getValue() {
+			return this.value;
+		}
+
+		void setValue(String value) {
+			this.value = value;
+		}
+
+	}
+
+	@EnableConfigurationProperties(ConstructorBindingProperties.class)
+	static class EnableConstructorBindingProperties {
+
+	}
+
+	@ConfigurationProperties("constructor-binding")
+	static class ConstructorBindingProperties {
+
+		private final String value;
+
+		ConstructorBindingProperties(String value) {
+			this.value = value;
+		}
+
+		String getValue() {
+			return this.value;
+		}
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	static class PossibleConstructorBindingPropertiesBeanMethodConfiguration {
+
+		@Bean
+		@ConfigurationProperties(prefix = "bean-method")
+		PossibleConstructorBindingProperties possibleConstructorBindingProperties() {
+			return new PossibleConstructorBindingProperties("alpha");
+		}
+
+	}
+
+	@EnableConfigurationProperties(PossibleConstructorBindingProperties.class)
+	static class EnablePossibleConstructorBindingProperties {
+
+	}
+
+	@ConfigurationProperties("possible-constructor-binding")
+	static class PossibleConstructorBindingProperties {
+
+		private String value;
+
+		PossibleConstructorBindingProperties(String arg) {
+
+		}
+
+		String getValue() {
+			return this.value;
+		}
+
+		void setValue(String value) {
+			this.value = value;
+		}
+
+	}
+
+	static class BindableAssertProvider implements AssertProvider<BindableAssert> {
+
+		private final Bindable<?> bindable;
+
+		BindableAssertProvider(Bindable<?> bindable) {
+			this.bindable = bindable;
+		}
+
+		@Override
+		public BindableAssert assertThat() {
+			return new BindableAssert(this.bindable);
+		}
+
+	}
+
+	static class BindableAssert extends AbstractAssert<BindableAssert, Bindable<?>> {
+
+		BindableAssert(Bindable<?> bindable) {
+			super(bindable, BindableAssert.class);
+		}
+
+		BindableAssert hasBindMethod(BindMethod bindMethod) {
+			if (this.actual.getBindMethod() != bindMethod) {
+				throwAssertionError(
+						new BasicErrorMessageFactory("Expected %s to have bind method %s but bind method was %s",
+								this.actual, bindMethod, this.actual.getBindMethod()));
+			}
+			return this;
+		}
+
+		BindableAssert hasType(Class<?> type) {
+			if (!type.equals(this.actual.getType().resolve())) {
+				throwAssertionError(new BasicErrorMessageFactory("Expected %s to have type %s but type was %s",
+						this.actual, type, this.actual.getType().resolve()));
+			}
+			return this;
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableRuntimeHintsRegistrarTests.java

@@ -75,7 +75,7 @@ class BindableRuntimeHintsRegistrarTests {
 	@Test
 	void registerHintsWhenNoClasses() {
 		RuntimeHints runtimeHints = new RuntimeHints();
-		BindableRuntimeHintsRegistrar registrar = new BindableRuntimeHintsRegistrar();
+		BindableRuntimeHintsRegistrar registrar = new BindableRuntimeHintsRegistrar(new Class<?>[0]);
 		registrar.registerHints(runtimeHints);
 		assertThat(runtimeHints.reflection().typeHints()).isEmpty();
 	}