Deprecate mutable methods of MethodParameter

Deprecate all mutation methods in `MethodParameter` in favor of factory
methods that return a new instance. Existing code that previously relied
on mutation has been updated to use the replacement methods.

Closes gh-23385

spring-core/src/main/java/org/springframework/core/GenericTypeResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-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.
@@ -57,12 +57,13 @@ public final class GenericTypeResolver {
 	 * @param methodParameter the method parameter specification
 	 * @param implementationClass the class to resolve type variables against
 	 * @return the corresponding generic parameter or return type
+	 * @deprecated since 5.2 in favor of {@code methodParameter.withContainingClass(implementationClass).getParameterType()}
 	 */
+	@Deprecated
 	public static Class<?> resolveParameterType(MethodParameter methodParameter, Class<?> implementationClass) {
 		Assert.notNull(methodParameter, "MethodParameter must not be null");
 		Assert.notNull(implementationClass, "Class must not be null");
 		methodParameter.setContainingClass(implementationClass);
-		ResolvableType.resolveMethodParameter(methodParameter);
 		return methodParameter.getParameterType();
 	}
 

spring-core/src/main/java/org/springframework/core/MethodParameter.java

@@ -55,6 +55,7 @@ import org.springframework.util.ObjectUtils;
  * @author Andy Clement
  * @author Sam Brannen
  * @author Sebastien Deleuze
+ * @author Phillip Webb
  * @since 2.0
  * @see org.springframework.core.annotation.SynthesizingMethodParameter
  */
@@ -76,6 +77,7 @@ public class MethodParameter {
 	@Nullable
 	Map<Integer, Integer> typeIndexesPerLevel;
 
+	/** The containing class. Could also be supplied by overriding {@link #getContainingClass()} */
 	@Nullable
 	private volatile Class<?> containingClass;
 
@@ -150,6 +152,24 @@ public class MethodParameter {
 		this.nestingLevel = nestingLevel;
 	}
 
+	/**
+	 * Internal constructor used to create a {@link MethodParameter} with a
+	 * containing class already set.
+	 * @param executable the Executable to specify a parameter for
+	 * @param parameterIndex the index of the parameter
+	 * @param containingClass the containing class
+	 * @since 5.2
+	 */
+	MethodParameter(Executable executable, int parameterIndex,
+			@Nullable Class<?> containingClass) {
+
+		Assert.notNull(executable, "Executable must not be null");
+		this.executable = executable;
+		this.parameterIndex = validateIndex(executable, parameterIndex);
+		this.nestingLevel = 1;
+		this.containingClass = containingClass;
+	}
+
 	/**
 	 * Copy constructor, resulting in an independent MethodParameter object
 	 * based on the same metadata and cache state that the original object was in.
@@ -252,7 +272,9 @@ public class MethodParameter {
 	/**
 	 * Increase this parameter's nesting level.
 	 * @see #getNestingLevel()
+	 * @deprecated since 5.2 in favor of {@link #nested(Integer)}
 	 */
+	@Deprecated
 	public void increaseNestingLevel() {
 		this.nestingLevel++;
 	}
@@ -260,7 +282,10 @@ public class MethodParameter {
 	/**
 	 * Decrease this parameter's nesting level.
 	 * @see #getNestingLevel()
+	 * @deprecated since 5.2 in favor of retaining the original MethodParameter and
+	 * using {@link #nested(Integer)} if nesting is required
 	 */
+	@Deprecated
 	public void decreaseNestingLevel() {
 		getTypeIndexesPerLevel().remove(this.nestingLevel);
 		this.nestingLevel--;
@@ -280,11 +305,22 @@ public class MethodParameter {
 	 * @param typeIndex the corresponding type index
 	 * (or {@code null} for the default type index)
 	 * @see #getNestingLevel()
+	 * @deprecated since 5.2 in favor of {@link #withTypeIndex}
 	 */
+	@Deprecated
 	public void setTypeIndexForCurrentLevel(int typeIndex) {
 		getTypeIndexesPerLevel().put(this.nestingLevel, typeIndex);
 	}
 
+	/**
+	 * Return a variant of this {@code MethodParameter} with the type for the current
+	 * level set to the specified value.
+	 * @param typeIndex the new type index
+	 */
+	public MethodParameter withTypeIndex(int typeIndex) {
+		return nested(this.nestingLevel, typeIndex);
+	}
+
 	/**
 	 * Return the type index for the current nesting level.
 	 * @return the corresponding type index, or {@code null}
@@ -319,22 +355,45 @@ public class MethodParameter {
 
 	/**
 	 * Return a variant of this {@code MethodParameter} which points to the
-	 * same parameter but one nesting level deeper. This is effectively the
-	 * same as {@link #increaseNestingLevel()}, just with an independent
-	 * {@code MethodParameter} object (e.g. in case of the original being cached).
+	 * same parameter but one nesting level deeper.
 	 * @since 4.3
 	 */
 	public MethodParameter nested() {
+		return nested(null);
+	}
+
+	/**
+	 * Return a variant of this {@code MethodParameter} which points to the
+	 * same parameter but one nesting level deeper.
+	 * @param typeIndex the type index for the new nesting level
+	 * @since 5.2
+	 */
+	public MethodParameter nested(@Nullable Integer typeIndex) {
 		MethodParameter nestedParam = this.nestedMethodParameter;
-		if (nestedParam != null) {
+		if (nestedParam != null && typeIndex == null) {
 			return nestedParam;
 		}
-		nestedParam = clone();
-		nestedParam.nestingLevel = this.nestingLevel + 1;
-		this.nestedMethodParameter = nestedParam;
+		nestedParam = nested(this.nestingLevel + 1, typeIndex);
+		if (typeIndex == null) {
+			this.nestedMethodParameter = nestedParam;
+		}
 		return nestedParam;
 	}
 
+	private MethodParameter nested(int nestingLevel, @Nullable Integer typeIndex) {
+		MethodParameter copy = clone();
+		copy.nestingLevel = nestingLevel;
+		if (this.typeIndexesPerLevel != null) {
+			copy.typeIndexesPerLevel = new HashMap<>(this.typeIndexesPerLevel);
+		}
+		if (typeIndex != null) {
+			copy.getTypeIndexesPerLevel().put(copy.nestingLevel, typeIndex);
+		}
+		copy.parameterType = null;
+		copy.genericParameterType = null;
+		return copy;
+	}
+
 	/**
 	 * Return whether this method indicates a parameter which is not required:
 	 * either in the form of Java 8's {@link java.util.Optional}, any variant
@@ -376,21 +435,28 @@ public class MethodParameter {
 		return (getParameterType() == Optional.class ? nested() : this);
 	}
 
-	/**
-	 * Set a containing class to resolve the parameter type against.
-	 */
+	public Class<?> getContainingClass() {
+		Class<?> containingClass = this.containingClass;
+		return (containingClass != null ? containingClass : getDeclaringClass());
+	}
+
+	@Deprecated
 	void setContainingClass(Class<?> containingClass) {
 		this.containingClass = containingClass;
+		this.parameterType = null;
 	}
 
-	public Class<?> getContainingClass() {
-		Class<?> containingClass = this.containingClass;
-		return (containingClass != null ? containingClass : getDeclaringClass());
+	public MethodParameter withContainingClass(@Nullable Class<?> containingClass) {
+		MethodParameter result = clone();
+		result.containingClass = containingClass;
+		result.parameterType = null;
+		return result;
 	}
 
 	/**
 	 * Set a resolved (generic) parameter type.
 	 */
+	@Deprecated
 	void setParameterType(@Nullable Class<?> parameterType) {
 		this.parameterType = parameterType;
 	}
@@ -401,18 +467,16 @@ public class MethodParameter {
 	 */
 	public Class<?> getParameterType() {
 		Class<?> paramType = this.parameterType;
+		if (paramType != null) {
+			return paramType;
+		}
+		if (getContainingClass() != null) {
+			paramType = ResolvableType.forMethodParameter(this, null, 1, null).resolve();
+		}
 		if (paramType == null) {
-			if (this.parameterIndex < 0) {
-				Method method = getMethod();
-				paramType = (method != null ?
-						(KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isKotlinType(getContainingClass()) ?
-						KotlinDelegate.getReturnType(method) : method.getReturnType()) : void.class);
-			}
-			else {
-				paramType = this.executable.getParameterTypes()[this.parameterIndex];
-			}
-			this.parameterType = paramType;
+			paramType = computeParameterType();
 		}
+		this.parameterType = paramType;
 		return paramType;
 	}
 
@@ -442,13 +506,27 @@ public class MethodParameter {
 					index = this.parameterIndex - 1;
 				}
 				paramType = (index >= 0 && index < genericParameterTypes.length ?
-						genericParameterTypes[index] : getParameterType());
+						genericParameterTypes[index] : computeParameterType());
 			}
 			this.genericParameterType = paramType;
 		}
 		return paramType;
 	}
 
+	private Class<?> computeParameterType() {
+		if (this.parameterIndex < 0) {
+			Method method = getMethod();
+			if (method == null) {
+				return void.class;
+			}
+			if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isKotlinType(getContainingClass())) {
+				return KotlinDelegate.getReturnType(method);
+			}
+			return method.getReturnType();
+		}
+		return this.executable.getParameterTypes()[this.parameterIndex];
+	}
+
 	/**
 	 * Return the nested type of the method/constructor parameter.
 	 * @return the parameter type (never {@code null})
@@ -688,7 +766,6 @@ public class MethodParameter {
 		return new MethodParameter(this);
 	}
 
-
 	/**
 	 * Create a new MethodParameter for the given method or constructor.
 	 * <p>This is a convenience factory method for scenarios where a

spring-core/src/main/java/org/springframework/core/ResolvableType.java

@@ -1200,8 +1200,7 @@ public class ResolvableType implements Serializable {
 			Class<?> implementationClass) {
 
 		Assert.notNull(constructor, "Constructor must not be null");
-		MethodParameter methodParameter = new MethodParameter(constructor, parameterIndex);
-		methodParameter.setContainingClass(implementationClass);
+		MethodParameter methodParameter = new MethodParameter(constructor, parameterIndex, implementationClass);
 		return forMethodParameter(methodParameter);
 	}
 
@@ -1227,8 +1226,7 @@ public class ResolvableType implements Serializable {
 	 */
 	public static ResolvableType forMethodReturnType(Method method, Class<?> implementationClass) {
 		Assert.notNull(method, "Method must not be null");
-		MethodParameter methodParameter = new MethodParameter(method, -1);
-		methodParameter.setContainingClass(implementationClass);
+		MethodParameter methodParameter = new MethodParameter(method, -1, implementationClass);
 		return forMethodParameter(methodParameter);
 	}
 
@@ -1258,8 +1256,7 @@ public class ResolvableType implements Serializable {
 	 */
 	public static ResolvableType forMethodParameter(Method method, int parameterIndex, Class<?> implementationClass) {
 		Assert.notNull(method, "Method must not be null");
-		MethodParameter methodParameter = new MethodParameter(method, parameterIndex);
-		methodParameter.setContainingClass(implementationClass);
+		MethodParameter methodParameter = new MethodParameter(method, parameterIndex, implementationClass);
 		return forMethodParameter(methodParameter);
 	}
 
@@ -1303,22 +1300,29 @@ public class ResolvableType implements Serializable {
 	 */
 	public static ResolvableType forMethodParameter(MethodParameter methodParameter, @Nullable Type targetType) {
 		Assert.notNull(methodParameter, "MethodParameter must not be null");
-		ResolvableType owner = forType(methodParameter.getContainingClass()).as(methodParameter.getDeclaringClass());
-		return forType(targetType, new MethodParameterTypeProvider(methodParameter), owner.asVariableResolver()).
-				getNested(methodParameter.getNestingLevel(), methodParameter.typeIndexesPerLevel);
+		int nestingLevel = methodParameter.getNestingLevel();
+		Map<Integer, Integer> typeIndexesPerLevel = methodParameter.typeIndexesPerLevel;
+		return forMethodParameter(methodParameter, targetType, nestingLevel,
+				typeIndexesPerLevel);
 	}
 
 	/**
-	 * Resolve the top-level parameter type of the given {@code MethodParameter}.
-	 * @param methodParameter the method parameter to resolve
-	 * @since 4.1.9
-	 * @see MethodParameter#setParameterType
+	 * Return a {@link ResolvableType} for the specified {@link MethodParameter} at
+	 * a specific nesting level, overriding the target type to resolve with a specific
+	 * given type.
+	 * @param methodParameter the source method parameter (must not be {@code null})
+	 * @param targetType the type to resolve (a part of the method parameter's type)
+	 * @param nestingLevel the nesting level to use
+	 * @param typeIndexesPerLevel the type indexes per nesting level
+	 * @return a {@link ResolvableType} for the specified method parameter
+	 * @since 5.2
+	 * @see #forMethodParameter(Method, int)
 	 */
-	static void resolveMethodParameter(MethodParameter methodParameter) {
-		Assert.notNull(methodParameter, "MethodParameter must not be null");
+	static ResolvableType forMethodParameter(MethodParameter methodParameter, @Nullable Type targetType,
+			int nestingLevel, @Nullable Map<Integer, Integer> typeIndexesPerLevel) {
 		ResolvableType owner = forType(methodParameter.getContainingClass()).as(methodParameter.getDeclaringClass());
-		methodParameter.setParameterType(
-				forType(null, new MethodParameterTypeProvider(methodParameter), owner.asVariableResolver()).resolve());
+		return forType(targetType, new MethodParameterTypeProvider(methodParameter), owner.asVariableResolver()).
+				getNested(nestingLevel, typeIndexesPerLevel);
 	}
 
 	/**

spring-core/src/main/java/org/springframework/core/convert/Property.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-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.
@@ -23,7 +23,6 @@ import java.lang.reflect.Method;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
-import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.MethodParameter;
 import org.springframework.lang.Nullable;
 import org.springframework.util.ConcurrentReferenceHashMap;
@@ -187,7 +186,7 @@ public final class Property {
 		if (getReadMethod() == null) {
 			return null;
 		}
-		return resolveParameterType(new MethodParameter(getReadMethod(), -1));
+		return new MethodParameter(getReadMethod(), -1).withContainingClass(getObjectType());
 	}
 
 	@Nullable
@@ -195,13 +194,7 @@ public final class Property {
 		if (getWriteMethod() == null) {
 			return null;
 		}
-		return resolveParameterType(new MethodParameter(getWriteMethod(), 0));
-	}
-
-	private MethodParameter resolveParameterType(MethodParameter parameter) {
-		// needed to resolve generic property types that parameterized by sub-classes e.g. T getFoo();
-		GenericTypeResolver.resolveParameterType(parameter, getObjectType());
-		return parameter;
+		return new MethodParameter(getWriteMethod(), 0).withContainingClass(getObjectType());
 	}
 
 	private Annotation[] resolveAnnotations() {

spring-core/src/test/java/org/springframework/core/MethodParameterTests.java

@@ -32,9 +32,12 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 /**
+ * Tests for {@link MethodParameter}.
+ *
  * @author Arjen Poutsma
  * @author Juergen Hoeller
  * @author Sam Brannen
+ * @author Phillip Webb
  */
 public class MethodParameterTests {
 
@@ -151,6 +154,7 @@ public class MethodParameterTests {
 	}
 
 	@Test
+	@Deprecated
 	public void multipleResolveParameterTypeCalls() throws Exception {
 		Method method = ArrayList.class.getMethod("get", int.class);
 		MethodParameter methodParameter = MethodParameter.forExecutable(method, -1);
@@ -174,17 +178,63 @@ public class MethodParameterTests {
 	@Test
 	public void equalsAndHashCodeConsidersNesting() throws Exception {
 		Method method = ArrayList.class.getMethod("get", int.class);
-		MethodParameter m1 = MethodParameter.forExecutable(method, -1);
-		GenericTypeResolver.resolveParameterType(m1, StringList.class);
-		MethodParameter m2 = MethodParameter.forExecutable(method, -1);
-		GenericTypeResolver.resolveParameterType(m2, StringList.class);
-		MethodParameter m3 = MethodParameter.forExecutable(method, -1);
-		GenericTypeResolver.resolveParameterType(m3, IntegerList.class);
+		MethodParameter m1 = MethodParameter.forExecutable(method, -1)
+				.withContainingClass(StringList.class);
+		MethodParameter m2 = MethodParameter.forExecutable(method, -1)
+				.withContainingClass(StringList.class);
+		MethodParameter m3 = MethodParameter.forExecutable(method, -1)
+				.withContainingClass(IntegerList.class);
 		MethodParameter m4 = MethodParameter.forExecutable(method, -1);
 		assertThat(m1).isEqualTo(m2).isNotEqualTo(m3).isNotEqualTo(m4);
 		assertThat(m1.hashCode()).isEqualTo(m2.hashCode());
 	}
 
+	public void withContainingClassReturnsNewInstance() throws Exception {
+		Method method = ArrayList.class.getMethod("get", int.class);
+		MethodParameter m1 = MethodParameter.forExecutable(method, -1);
+		MethodParameter m2 = m1.withContainingClass(StringList.class);
+		MethodParameter m3 = m1.withContainingClass(IntegerList.class);
+		assertThat(m1).isNotSameAs(m2).isNotSameAs(m3);
+		assertThat(m1.getParameterType()).isEqualTo(Object.class);
+		assertThat(m2.getParameterType()).isEqualTo(String.class);
+		assertThat(m3.getParameterType()).isEqualTo(Integer.class);
+	}
+
+	@Test
+	public void withTypeIndexReturnsNewInstance() throws Exception {
+		Method method = ArrayList.class.getMethod("get", int.class);
+		MethodParameter m1 = MethodParameter.forExecutable(method, -1);
+		MethodParameter m2 = m1.withTypeIndex(2);
+		MethodParameter m3 = m1.withTypeIndex(3);
+		assertThat(m1).isNotSameAs(m2).isNotSameAs(m3);
+		assertThat(m1.getTypeIndexForCurrentLevel()).isNull();
+		assertThat(m2.getTypeIndexForCurrentLevel()).isEqualTo(2);
+		assertThat(m3.getTypeIndexForCurrentLevel()).isEqualTo(3);
+	}
+
+	@Test
+	@SuppressWarnings("deprecation")
+	public void mutatingNestingLevelShouldNotChangeNewInstance() throws Exception {
+		Method method = ArrayList.class.getMethod("get", int.class);
+		MethodParameter m1 = MethodParameter.forExecutable(method, -1);
+		MethodParameter m2 = m1.withTypeIndex(2);
+		assertThat(m2.getTypeIndexForCurrentLevel()).isEqualTo(2);
+		m1.setTypeIndexForCurrentLevel(1);
+		m2.decreaseNestingLevel();
+		assertThat(m2.getTypeIndexForCurrentLevel()).isNull();
+	}
+
+	@Test
+	public void nestedWithTypeIndexReturnsNewInstance() throws Exception {
+		Method method = ArrayList.class.getMethod("get", int.class);
+		MethodParameter m1 = MethodParameter.forExecutable(method, -1);
+		MethodParameter m2 = m1.nested(2);
+		MethodParameter m3 = m1.nested(3);
+		assertThat(m1).isNotSameAs(m2).isNotSameAs(m3);
+		assertThat(m1.getTypeIndexForCurrentLevel()).isNull();
+		assertThat(m2.getTypeIndexForCurrentLevel()).isEqualTo(2);
+		assertThat(m3.getTypeIndexForCurrentLevel()).isEqualTo(3);
+	}
 
 	public int method(String p1, long p2) {
 		return 42;

spring-messaging/src/main/java/org/springframework/messaging/handler/HandlerMethod.java

@@ -26,7 +26,6 @@ import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.core.BridgeMethodResolver;
-import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.annotation.AnnotatedElementUtils;
 import org.springframework.core.annotation.SynthesizingMethodParameter;
@@ -161,9 +160,7 @@ public class HandlerMethod {
 		int count = this.bridgedMethod.getParameterCount();
 		MethodParameter[] result = new MethodParameter[count];
 		for (int i = 0; i < count; i++) {
-			HandlerMethodParameter parameter = new HandlerMethodParameter(i);
-			GenericTypeResolver.resolveParameterType(parameter, this.beanType);
-			result[i] = parameter;
+			result[i] = new HandlerMethodParameter(i);
 		}
 		return result;
 	}

spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java

@@ -30,7 +30,6 @@ import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.core.BridgeMethodResolver;
-import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotatedElementUtils;
@@ -188,9 +187,7 @@ public class HandlerMethod {
 		int count = this.bridgedMethod.getParameterCount();
 		MethodParameter[] result = new MethodParameter[count];
 		for (int i = 0; i < count; i++) {
-			HandlerMethodParameter parameter = new HandlerMethodParameter(i);
-			GenericTypeResolver.resolveParameterType(parameter, this.beanType);
-			result[i] = parameter;
+			result[i] = new HandlerMethodParameter(i);
 		}
 		return result;
 	}