Support overloaded setters when binding beans

Update `JavaBeanBinder` so that overloaded setters can be used when
binding. Prior to this commit the setter picked would depend on the
order that the JVM returned the declared methods. We now consistently
prefer using the setter with a parameter type that matches the getter.

Closes gh-16206

Author: philwebb

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2018 the original author or authors.
+ * Copyright 2012-2019 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.
@@ -97,8 +97,10 @@ else if (value == null || !bound.equals(value.get())) {
 
 	/**
 	 * The bean being bound.
+	 *
+	 * @param <T> the bean type
 	 */
-	private static class Bean<T> {
+	static class Bean<T> {
 
 		private static Bean<?> cached;
 
@@ -111,23 +113,36 @@ private static class Bean<T> {
 		Bean(ResolvableType type, Class<?> resolvedType) {
 			this.type = type;
 			this.resolvedType = resolvedType;
-			putProperties(resolvedType);
+			addProperties(resolvedType);
 		}
 
-		private void putProperties(Class<?> type) {
+		private void addProperties(Class<?> type) {
 			while (type != null && !Object.class.equals(type)) {
-				for (Method method : type.getDeclaredMethods()) {
-					if (isCandidate(method)) {
-						addMethod(method);
-					}
-				}
-				for (Field field : type.getDeclaredFields()) {
-					addField(field);
-				}
+				Method[] declaredMethods = type.getDeclaredMethods();
+				Field[] declaredFields = type.getDeclaredFields();
+				addProperties(declaredMethods, declaredFields);
 				type = type.getSuperclass();
 			}
 		}
 
+		protected void addProperties(Method[] declaredMethods, Field[] declaredFields) {
+			for (int i = 0; i < declaredMethods.length; i++) {
+				if (!isCandidate(declaredMethods[i])) {
+					declaredMethods[i] = null;
+				}
+			}
+			for (Method method : declaredMethods) {
+				addMethodIfPossible(method, "get", 0, BeanProperty::addGetter);
+			}
+			for (Method method : declaredMethods) {
+				addMethodIfPossible(method, "is", 0, BeanProperty::addGetter);
+				addMethodIfPossible(method, "set", 1, BeanProperty::addSetter);
+			}
+			for (Field field : declaredFields) {
+				addField(field);
+			}
+		}
+
 		private boolean isCandidate(Method method) {
 			int modifiers = method.getModifiers();
 			return Modifier.isPublic(modifiers) && !Modifier.isAbstract(modifiers)
@@ -136,15 +151,9 @@ private boolean isCandidate(Method method) {
 					&& !Class.class.equals(method.getDeclaringClass());
 		}
 
-		private void addMethod(Method method) {
-			addMethodIfPossible(method, "get", 0, BeanProperty::addGetter);
-			addMethodIfPossible(method, "is", 0, BeanProperty::addGetter);
-			addMethodIfPossible(method, "set", 1, BeanProperty::addSetter);
-		}
-
 		private void addMethodIfPossible(Method method, String prefix, int parameterCount,
 				BiConsumer<BeanProperty, Method> consumer) {
-			if (method.getParameterCount() == parameterCount
+			if (method != null && method.getParameterCount() == parameterCount
 					&& method.getName().startsWith(prefix)
 					&& method.getName().length() > prefix.length()) {
 				String propertyName = Introspector
@@ -250,7 +259,7 @@ public T get() {
 	/**
 	 * A bean property being bound.
 	 */
-	private static class BeanProperty {
+	static class BeanProperty {
 
 		private final String name;
 
@@ -274,11 +283,16 @@ public void addGetter(Method getter) {
 		}
 
 		public void addSetter(Method setter) {
-			if (this.setter == null) {
+			if (this.setter == null || isBetterSetter(setter)) {
 				this.setter = setter;
 			}
 		}
 
+		private boolean isBetterSetter(Method setter) {
+			return this.getter != null
+					&& this.getter.getReturnType().equals(setter.getParameterTypes()[0]);
+		}
+
 		public void addField(Field field) {
 			if (this.field == null) {
 				this.field = field;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2018 the original author or authors.
+ * Copyright 2012-2019 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.
@@ -16,6 +16,8 @@
 
 package org.springframework.boot.context.properties.bind;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.time.LocalDate;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -28,11 +30,14 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import org.springframework.boot.context.properties.bind.JavaBeanBinder.Bean;
+import org.springframework.boot.context.properties.bind.JavaBeanBinder.BeanProperty;
 import org.springframework.boot.context.properties.bind.handler.IgnoreErrorsBindHandler;
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
 import org.springframework.boot.context.properties.source.MockConfigurationPropertySource;
 import org.springframework.boot.convert.Delimiter;
+import org.springframework.core.ResolvableType;
 import org.springframework.format.annotation.DateTimeFormat;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -44,6 +49,7 @@
  *
  * @author Phillip Webb
  * @author Madhura Bhave
+ * @author Andy Wilkinson
  */
 public class JavaBeanBinderTests {
 
@@ -505,6 +511,56 @@ public void bindToClassShouldCacheWithGenerics() {
 		assertThat(bean.getBooleans().get("b").getValue()).isEqualTo(true);
 	}
 
+	public void bindToClassWithOverloadedSetterShouldUseSetterThatMatchesField() {
+		// gh-16206
+		MockConfigurationPropertySource source = new MockConfigurationPropertySource();
+		source.put("foo.property", "some string");
+		this.sources.add(source);
+		PropertyWithOverloadedSetter bean = this.binder
+				.bind("foo", Bindable.of(PropertyWithOverloadedSetter.class)).get();
+		assertThat(bean.getProperty()).isEqualTo("some string");
+	}
+
+	@Test
+	public void beanProperiesPreferMatchingType() {
+		// gh-16206
+
+		ResolvableType type = ResolvableType.forClass(PropertyWithOverloadedSetter.class);
+		Bean<PropertyWithOverloadedSetter> bean = new Bean<PropertyWithOverloadedSetter>(
+				type, type.resolve()) {
+
+			@Override
+			protected void addProperties(Method[] declaredMethods,
+					Field[] declaredFields) {
+				// We override here because we need a specific order of the declared
+				// methods and the JVM doesn't give us one
+				int intSetter = -1;
+				int stringSetter = -1;
+				for (int i = 0; i < declaredMethods.length; i++) {
+					Method method = declaredMethods[i];
+					if (method.getName().equals("setProperty")) {
+						if (method.getParameters()[0].getType().equals(int.class)) {
+							intSetter = i;
+						}
+						else {
+							stringSetter = i;
+						}
+					}
+				}
+				if (intSetter > stringSetter) {
+					Method method = declaredMethods[intSetter];
+					declaredMethods[intSetter] = declaredMethods[stringSetter];
+					declaredMethods[stringSetter] = method;
+				}
+				super.addProperties(declaredMethods, declaredFields);
+			}
+
+		};
+		BeanProperty property = bean.getProperties().get("property");
+		PropertyWithOverloadedSetter target = new PropertyWithOverloadedSetter();
+		property.setValue(() -> target, "some string");
+	}
+
 	public static class ExampleValueBean {
 
 		private int intValue;
@@ -955,4 +1011,22 @@ public void setValue(T value) {
 
 	}
 
+	public static class PropertyWithOverloadedSetter {
+
+		private String property;
+
+		public void setProperty(int property) {
+			this.property = String.valueOf(property);
+		}
+
+		public void setProperty(String property) {
+			this.property = property;
+		}
+
+		public String getProperty() {
+			return this.property;
+		}
+
+	}
+
 }