Fix detection of property setter

Previously, the meta-data annotation processor was taking the first
setter that match the property name it has to handle. Contrary to
getters that are enforced by a return type (no argument), multiple
setter candidates may exist.

If a property's type got narrowed over time, the original setter may
have been marked as Deprecated. As the annotation processor takes the
first setter that matches based on the name only, it may pick up the
deprecated one and therefore mark the property as being (wrongly)
deprecatede in the meta-data.

It turns out that checking for the actual type of the setter parameter
brought a side effect: some primitive properties may use the primitive
or the Wrapper counter part. This commit not only look at the proper
setter based on the type but also fallback on the wrapper (or) primitive
if necessary.

Closes gh-4338

spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java

@@ -204,9 +204,9 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
 				.entrySet()) {
 			String name = entry.getKey();
 			ExecutableElement getter = entry.getValue();
-			ExecutableElement setter = members.getPublicSetters().get(name);
-			VariableElement field = members.getFields().get(name);
 			TypeMirror returnType = getter.getReturnType();
+			ExecutableElement setter = members.getPublicSetter(name, returnType);
+			VariableElement field = members.getFields().get(name);
 			Element returnTypeElement = this.processingEnv.getTypeUtils()
 					.asElement(returnType);
 			boolean isExcluded = this.typeExcludeFilter.isExcluded(returnType);

spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-2015 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,8 +16,10 @@
 
 package org.springframework.boot.configurationprocessor;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 
 import javax.annotation.processing.ProcessingEnvironment;
@@ -27,6 +29,7 @@ import javax.lang.model.element.Modifier;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.TypeKind;
+import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.ElementFilter;
 
 /**
@@ -42,14 +45,18 @@ class TypeElementMembers {
 
 	private final ProcessingEnvironment env;
 
+	private final TypeUtils typeUtils;
+
 	private final Map<String, VariableElement> fields = new LinkedHashMap<String, VariableElement>();
 
 	private final Map<String, ExecutableElement> publicGetters = new LinkedHashMap<String, ExecutableElement>();
 
-	private final Map<String, ExecutableElement> publicSetters = new LinkedHashMap<String, ExecutableElement>();
+	private final Map<String, List<ExecutableElement>> publicSetters =
+			new LinkedHashMap<String, List<ExecutableElement>>();
 
 	TypeElementMembers(ProcessingEnvironment env, TypeElement element) {
 		this.env = env;
+		this.typeUtils = new TypeUtils(this.env);
 		process(element);
 	}
 
@@ -75,10 +82,29 @@ class TypeElementMembers {
 			if (isGetter(method) && !this.publicGetters.containsKey(name)) {
 				this.publicGetters.put(getAccessorName(name), method);
 			}
-			else if (isSetter(method) && !this.publicSetters.containsKey(name)) {
-				this.publicSetters.put(getAccessorName(name), method);
+			else if (isSetter(method)) {
+				String propertyName = getAccessorName(name);
+				List<ExecutableElement> matchingSetters = this.publicSetters.get(propertyName);
+				if (matchingSetters == null) {
+					matchingSetters = new ArrayList<ExecutableElement>();
+					this.publicSetters.put(propertyName, matchingSetters);
+				}
+				TypeMirror paramType = method.getParameters().get(0).asType();
+				if (getMatchingSetter(matchingSetters, paramType) == null) {
+					matchingSetters.add(method);
+				}
+			}
+		}
+	}
+
+	private ExecutableElement getMatchingSetter(List<ExecutableElement> candidates, TypeMirror type) {
+		for (ExecutableElement candidate : candidates) {
+			TypeMirror paramType = candidate.getParameters().get(0).asType();
+			if (this.env.getTypeUtils().isSameType(paramType, type)) {
+				return candidate;
 			}
 		}
+		return null;
 	}
 
 	private boolean isGetter(ExecutableElement method) {
@@ -122,8 +148,19 @@ class TypeElementMembers {
 		return Collections.unmodifiableMap(this.publicGetters);
 	}
 
-	public Map<String, ExecutableElement> getPublicSetters() {
-		return Collections.unmodifiableMap(this.publicSetters);
+	public ExecutableElement getPublicSetter(String name, TypeMirror type) {
+		List<ExecutableElement> candidates = this.publicSetters.get(name);
+		if (candidates != null) {
+			ExecutableElement matching = getMatchingSetter(candidates, type);
+			if (matching != null) {
+				return matching;
+			}
+			TypeMirror alternative = this.typeUtils.getWrapperOrPrimitiveFor(type);
+			if (alternative != null) {
+				return getMatchingSetter(candidates, alternative);
+			}
+		}
+		return null;
 	}
 
 }

spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-2015 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.
@@ -40,6 +40,7 @@ import javax.lang.model.util.Types;
 class TypeUtils {
 
 	private static final Map<TypeKind, Class<?>> PRIMITIVE_WRAPPERS;
+	private static final Map<String, TypeKind> WRAPPER_TO_PRIMITIVE;
 
 	static {
 		Map<TypeKind, Class<?>> wrappers = new HashMap<TypeKind, Class<?>>();
@@ -52,6 +53,17 @@ class TypeUtils {
 		wrappers.put(TypeKind.LONG, Long.class);
 		wrappers.put(TypeKind.SHORT, Short.class);
 		PRIMITIVE_WRAPPERS = Collections.unmodifiableMap(wrappers);
+
+		Map<String, TypeKind> primitives = new HashMap<String, TypeKind>();
+		primitives.put(Boolean.class.getName(), TypeKind.BOOLEAN);
+		primitives.put(Byte.class.getName(), TypeKind.BYTE);
+		primitives.put(Character.class.getName(), TypeKind.CHAR);
+		primitives.put(Double.class.getName(), TypeKind.DOUBLE);
+		primitives.put(Float.class.getName(), TypeKind.FLOAT);
+		primitives.put(Integer.class.getName(), TypeKind.INT);
+		primitives.put(Long.class.getName(), TypeKind.LONG);
+		primitives.put(Short.class.getName(), TypeKind.SHORT);
+		WRAPPER_TO_PRIMITIVE = primitives;
 	}
 
 	private final ProcessingEnvironment env;
@@ -79,7 +91,7 @@ class TypeUtils {
 		if (type == null) {
 			return null;
 		}
-		Class<?> wrapper = PRIMITIVE_WRAPPERS.get(type.getKind());
+		Class<?> wrapper = getWrapperFor(type);
 		if (wrapper != null) {
 			return wrapper.getName();
 		}
@@ -99,6 +111,7 @@ class TypeUtils {
 				|| this.env.getTypeUtils().isAssignable(type, this.mapType);
 	}
 
+
 	public boolean isEnclosedIn(Element candidate, TypeElement element) {
 		if (candidate == null || element == null) {
 			return false;
@@ -118,4 +131,24 @@ class TypeUtils {
 		return ("".equals(javadoc) ? null : javadoc);
 	}
 
+	public TypeMirror getWrapperOrPrimitiveFor(TypeMirror typeMirror) {
+		Class<?> candidate = getWrapperFor(typeMirror);
+		if (candidate != null) {
+			return this.env.getElementUtils().getTypeElement(candidate.getName()).asType();
+		}
+		TypeKind primitiveKind = getPrimitiveFor(typeMirror);
+		if (primitiveKind != null) {
+			return this.env.getTypeUtils().getPrimitiveType(primitiveKind);
+		}
+		return null;
+	}
+
+	private Class<?> getWrapperFor(TypeMirror type) {
+		return PRIMITIVE_WRAPPERS.get(type.getKind());
+	}
+
+	private TypeKind getPrimitiveFor(TypeMirror type) {
+		return WRAPPER_TO_PRIMITIVE.get(type.toString());
+	}
+
 }

spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java

@@ -51,7 +51,9 @@ import org.springframework.boot.configurationsample.simple.SimpleCollectionPrope
 import org.springframework.boot.configurationsample.simple.SimplePrefixValueProperties;
 import org.springframework.boot.configurationsample.simple.SimpleProperties;
 import org.springframework.boot.configurationsample.simple.SimpleTypeProperties;
+import org.springframework.boot.configurationsample.specific.BoxingPojo;
 import org.springframework.boot.configurationsample.specific.BuilderPojo;
+import org.springframework.boot.configurationsample.specific.DeprecatedUnrelatedMethodPojo;
 import org.springframework.boot.configurationsample.specific.ExcludedTypesPojo;
 import org.springframework.boot.configurationsample.specific.InnerClassAnnotatedGetterConfig;
 import org.springframework.boot.configurationsample.specific.InnerClassProperties;
@@ -197,6 +199,30 @@ public class ConfigurationMetadataAnnotationProcessorTests {
 						.withDeprecation("renamed", "singledeprecated.new-name"));
 	}
 
+	@Test
+	public void deprecatedOnUnrelatedSetter() throws Exception {
+		Class<?> type = DeprecatedUnrelatedMethodPojo.class;
+		ConfigurationMetadata metadata = compile(type);
+		assertThat(metadata, containsGroup("not.deprecated").fromSource(type));
+		assertThat(metadata, containsProperty("not.deprecated.counter", Integer.class)
+				.withNoDeprecation()
+				.fromSource(type));
+		assertThat(metadata, containsProperty("not.deprecated.flag", Boolean.class)
+				.withNoDeprecation()
+				.fromSource(type));
+	}
+
+	@Test
+	public void boxingOnSetter() throws IOException {
+		Class<?> type = BoxingPojo.class;
+		ConfigurationMetadata metadata = compile(type);
+		assertThat(metadata, containsGroup("boxing").fromSource(type));
+		assertThat(metadata, containsProperty("boxing.flag", Boolean.class)
+				.fromSource(type));
+		assertThat(metadata, containsProperty("boxing.counter", Integer.class)
+				.fromSource(type));
+	}
+
 	@Test
 	public void parseCollectionConfig() throws Exception {
 		ConfigurationMetadata metadata = compile(SimpleCollectionProperties.class);
@@ -343,7 +369,9 @@ public class ConfigurationMetadataAnnotationProcessorTests {
 		assertThat(metadata,
 				containsProperty("simple.flag", Boolean.class)
 						.fromSource(SimpleProperties.class)
-						.withDescription("A simple flag.").withDefaultValue(is(true)));
+						.withDescription("A simple flag.")
+						.withDeprecation(null, null)
+						.withDefaultValue(is(true)));
 		assertThat(metadata.getItems().size(), is(4));
 	}
 

spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataMatchers.java

@@ -125,6 +125,9 @@ public final class ConfigurationMetadataMatchers {
 					&& !this.description.equals(itemMetadata.getDescription())) {
 				return false;
 			}
+			if (this.deprecation == null && itemMetadata.getDeprecation() != null) {
+				return false;
+			}
 			if (this.deprecation != null
 					&& !this.deprecation.equals(itemMetadata.getDeprecation())) {
 				return false;
@@ -200,6 +203,11 @@ public final class ConfigurationMetadataMatchers {
 					new ItemDeprecation(reason, replacement));
 		}
 
+		public ContainsItemMatcher withNoDeprecation() {
+			return new ContainsItemMatcher(this.itemType, this.name, this.type,
+					this.sourceType, this.description, this.defaultValue, null);
+		}
+
 		private ItemMetadata getFirstItemWithName(ConfigurationMetadata metadata,
 				String name) {
 			for (ItemMetadata item : metadata.getItems()) {

spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/BoxingPojo.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2012-2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.configurationsample.specific;
+
+import org.springframework.boot.configurationsample.ConfigurationProperties;
+
+/**
+ * Demonstrate the use of boxing/unboxing. Even if the type does not
+ * strictly match, it should still be detected.
+ *
+ * @author Stephane Nicoll
+ */
+@ConfigurationProperties("boxing")
+public class BoxingPojo {
+
+	private boolean flag;
+
+	private Integer counter;
+
+	public boolean isFlag() {
+		return this.flag;
+	}
+
+	// Setter use Boolean
+	public void setFlag(Boolean flag) {
+		this.flag = flag;
+	}
+
+	public Integer getCounter() {
+		return this.counter;
+	}
+
+	// Setter use int
+	public void setCounter(int counter) {
+		this.counter = counter;
+	}
+
+}

spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/DeprecatedUnrelatedMethodPojo.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2012-2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.configurationsample.specific;
+
+import org.springframework.boot.configurationsample.ConfigurationProperties;
+
+/**
+ * Demonstrate that an unrelated setter is not taken into account
+ * to detect the deprecated flag.
+ *
+ * @author Stephane Nicoll
+ */
+@ConfigurationProperties("not.deprecated")
+public class DeprecatedUnrelatedMethodPojo {
+
+	private Integer counter;
+
+	private boolean flag;
+
+	public Integer getCounter() {
+		return this.counter;
+	}
+
+	public void setCounter(Integer counter) {
+		this.counter = counter;
+	}
+
+	@Deprecated
+	public void setCounter(String counterAsString) {
+		this.counter = Integer.valueOf(counterAsString);
+	}
+
+	public boolean isFlag() {
+		return this.flag;
+	}
+
+	public void setFlag(boolean flag) {
+		this.flag = flag;
+	}
+
+	@Deprecated
+	public void setFlag(Boolean flag) {
+		this.flag = flag;
+	}
+
+}