Polish "Detect config props using builder pattern and generics"

See gh-19099
spring-projects · Dec 23, 2019 · b364396 · b364396
1 parent 743f4a4
commit b364396
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 54 deletions.
diff --git a/...sor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java b/...sor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java
@@ -28,10 +28,8 @@
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
-import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
-import javax.lang.model.type.TypeVariable;
 import javax.lang.model.util.ElementFilter;
 
 /**
@@ -46,15 +44,18 @@ class TypeElementMembers {
 
 	private final MetadataGenerationEnvironment env;
 
+	private final TypeElement targetType;
+
 	private final Map<String, VariableElement> fields = new LinkedHashMap<>();
 
 	private final Map<String, ExecutableElement> publicGetters = new LinkedHashMap<>();
 
 	private final Map<String, List<ExecutableElement>> publicSetters = new LinkedHashMap<>();
 
-	TypeElementMembers(MetadataGenerationEnvironment env, TypeElement element) {
+	TypeElementMembers(MetadataGenerationEnvironment env, TypeElement targetType) {
 		this.env = env;
-		process(element);
+		this.targetType = targetType;
+		process(targetType);
 	}
 
 	private void process(TypeElement element) {
@@ -118,26 +119,19 @@ private boolean isSetter(ExecutableElement method) {
 
 	private boolean isSetterReturnType(ExecutableElement method) {
 		TypeMirror returnType = method.getReturnType();
-		// void
 		if (TypeKind.VOID == returnType.getKind()) {
 			return true;
 		}
-
-		TypeMirror classType = method.getEnclosingElement().asType();
-		TypeUtils typeUtils = this.env.getTypeUtils();
-		// Chain
-		if (typeUtils.isSameType(classType, returnType)) {
+		if (TypeKind.DECLARED == returnType.getKind()
+				&& this.env.getTypeUtils().isSameType(method.getEnclosingElement().asType(), returnType)) {
 			return true;
 		}
-
-		// Chain generic type, <T extends classType>
-		List<? extends TypeMirror> genericTypes = ((DeclaredType) classType).getTypeArguments();
-		return genericTypes.stream().anyMatch((genericType) -> {
-			TypeMirror upperBound = ((TypeVariable) genericType).getUpperBound();
-			String classTypeName = typeUtils.getQualifiedName(((DeclaredType) classType).asElement());
-			String genericTypeName = typeUtils.getQualifiedName(((DeclaredType) upperBound).asElement());
-			return classTypeName.equals(genericTypeName);
-		});
+		if (TypeKind.TYPEVAR == returnType.getKind()) {
+			String resolvedType = this.env.getTypeUtils().getType(this.targetType, returnType);
+			return (resolvedType != null
+					&& resolvedType.equals(this.env.getTypeUtils().getQualifiedName(this.targetType)));
+		}
+		return false;
 	}
 
 	private String getAccessorName(String methodName) {

diff --git a/...java/org/springframework/boot/configurationprocessor/GenericsMetadataGenerationTests.java b/...java/org/springframework/boot/configurationprocessor/GenericsMetadataGenerationTests.java
@@ -21,9 +21,8 @@
 import org.springframework.boot.configurationprocessor.metadata.ConfigurationMetadata;
 import org.springframework.boot.configurationprocessor.metadata.Metadata;
 import org.springframework.boot.configurationsample.generic.AbstractGenericProperties;
-import org.springframework.boot.configurationsample.generic.ChainGenericConfig;
-import org.springframework.boot.configurationsample.generic.ChainGenericProperties;
 import org.springframework.boot.configurationsample.generic.ComplexGenericProperties;
+import org.springframework.boot.configurationsample.generic.ConcreteBuilderProperties;
 import org.springframework.boot.configurationsample.generic.GenericConfig;
 import org.springframework.boot.configurationsample.generic.SimpleGenericProperties;
 import org.springframework.boot.configurationsample.generic.UnresolvedGenericProperties;
@@ -113,13 +112,13 @@ void wildcardTypes() {
 	}
 
 	@Test
-	void chainGenericProperties() {
-		ConfigurationMetadata metadata = compile(ChainGenericProperties.class);
-		assertThat(metadata).has(Metadata.withGroup("generic").fromSource(ChainGenericProperties.class));
-		assertThat(metadata).has(Metadata.withGroup("generic.config", ChainGenericConfig.class)
-				.fromSource(ChainGenericProperties.class));
-		assertThat(metadata).has(Metadata.withProperty("generic.config.ping-timeout", Integer.class)
-				.fromSource(ChainGenericConfig.class).withDefaultValue(null));
+	void builderPatternWithGenericReturnType() {
+		ConfigurationMetadata metadata = compile(ConcreteBuilderProperties.class);
+		assertThat(metadata).has(Metadata.withGroup("builder").fromSource(ConcreteBuilderProperties.class));
+		assertThat(metadata).has(
+				Metadata.withProperty("builder.number", Integer.class).fromSource(ConcreteBuilderProperties.class));
+		assertThat(metadata).has(
+				Metadata.withProperty("builder.description", String.class).fromSource(ConcreteBuilderProperties.class));
 		assertThat(metadata.getItems()).hasSize(3);
 	}
 

diff --git a/...ample/generic/ChainGenericProperties.java → ...le/generic/ConcreteBuilderProperties.java b/...ample/generic/ChainGenericProperties.java → ...le/generic/ConcreteBuilderProperties.java
@@ -17,28 +17,24 @@
 package org.springframework.boot.configurationsample.generic;
 
 import org.springframework.boot.configurationsample.ConfigurationProperties;
-import org.springframework.boot.configurationsample.NestedConfigurationProperty;
 
 /**
- * Chain Generic Properties
+ * Builder pattern with a resolved generic
  *
- * @author L.cm
+ * @author Stephane Nicoll
  */
-@ConfigurationProperties("generic")
-public class ChainGenericProperties {
+@ConfigurationProperties("builder")
+public class ConcreteBuilderProperties extends GenericBuilderProperties<ConcreteBuilderProperties> {
 
-	/**
-	 * Generic config.
-	 */
-	@NestedConfigurationProperty
-	private ChainGenericConfig config;
+	private String description;
 
-	public ChainGenericConfig getConfig() {
-		return this.config;
+	public String getDescription() {
+		return this.description;
 	}
 
-	public void setConfig(ChainGenericConfig config) {
-		this.config = config;
+	public ConcreteBuilderProperties setDescription(String description) {
+		this.description = description;
+		return this;
 	}
 
 }
diff --git a/...ionsample/generic/ChainGenericConfig.java → ...ple/generic/GenericBuilderProperties.java b/...ionsample/generic/ChainGenericConfig.java → ...ple/generic/GenericBuilderProperties.java
@@ -17,24 +17,21 @@
 package org.springframework.boot.configurationsample.generic;
 
 /**
- * Chain Generic
+ * A configuration properties that uses the builder pattern with a generic.
  *
- * @param <T> name type
- * @author L.cm
+ * @param <T> the type of the return type
+ * @author Stephane Nicoll
  */
-public class ChainGenericConfig<T extends ChainGenericConfig> {
+public class GenericBuilderProperties<T extends GenericBuilderProperties<T>> {
 
-	/**
-	 * Generic config pingTimeout.
-	 */
-	private Integer pingTimeout = 1000;
+	private int number;
 
-	public int getPingTimeout() {
-		return this.pingTimeout;
+	public int getNumber() {
+		return this.number;
 	}
 
-	public T setPingTimeout(int pingTimeout) {
-		this.pingTimeout = pingTimeout;
+	public T setNumber(int number) {
+		this.number = number;
 		return (T) this;
 	}