Revise contribution

See gh-32704

Author: sbrannen

Diff for: spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java

@@ -36,8 +36,6 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.MethodInvoker;
 
-import static org.springframework.util.ObjectUtils.isArray;
-
 /**
  * Utility methods used by the reflection resolver code to discover the appropriate
  * methods/constructors and fields that should be used in expressions.
@@ -453,19 +451,18 @@ private static boolean isFirstEntryInArray(Object value, @Nullable Object possib
 	 * @return a repackaged array of arguments where any varargs setup has performed
 	 */
 	public static Object[] setupArgumentsForVarargsInvocation(Class<?>[] requiredParameterTypes, Object... args) {
-		int parameterCount = requiredParameterTypes.length;
-		Assert.notEmpty(requiredParameterTypes, "Required parameter types must not be empty");
+		Assert.notEmpty(requiredParameterTypes, "Required parameter types array must not be empty");
 
+		int parameterCount = requiredParameterTypes.length;
 		Class<?> lastRequiredParameterType = requiredParameterTypes[parameterCount - 1];
-		Assert.isTrue(lastRequiredParameterType.isArray(), "Method must be varargs");
+		Assert.isTrue(lastRequiredParameterType.isArray(),
+				"The last required parameter type must be an array to support varargs invocation");
 
 		int argumentCount = args.length;
-		Object lastArgument = argumentCount > 0 ? args[argumentCount - 1] : null;
+		Object lastArgument = (argumentCount > 0 ? args[argumentCount - 1] : null);
 
 		// Check if repackaging is needed...
-		if (parameterCount != args.length ||
-			(!isArray(lastArgument) && differentTypes(lastRequiredParameterType, lastArgument))) {
-
+		if (parameterCount != argumentCount || !lastRequiredParameterType.isInstance(lastArgument)) {
 			// Create an array for the leading arguments plus the varargs array argument.
 			Object[] newArgs = new Object[parameterCount];
 			// Copy all leading arguments to the new array, omitting the varargs array argument.
@@ -486,12 +483,10 @@ public static Object[] setupArgumentsForVarargsInvocation(Class<?>[] requiredPar
 			newArgs[newArgs.length - 1] = varargsArray;
 			return newArgs;
 		}
+
 		return args;
 	}
 
-	private static boolean differentTypes(Class<?> lastRequiredParameterType, @Nullable Object lastArgument) {
-		return lastArgument == null || lastRequiredParameterType != lastArgument.getClass();
-	}
 
 	/**
 	 * Arguments match kinds.

Diff for: spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java

@@ -40,7 +40,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.assertj.core.api.InstanceOfAssertFactories.array;
 import static org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind.CLOSE;
 import static org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind.EXACT;
@@ -252,34 +252,42 @@ void convertAllArguments() throws Exception {
 		checkArguments(args, "3", null, "3.0");
 	}
 
+	@Test
+	void setupArgumentsForVarargsInvocationPreconditions() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> ReflectionHelper.setupArgumentsForVarargsInvocation(new Class[] {}, "a"))
+				.withMessage("Required parameter types array must not be empty");
+
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> ReflectionHelper.setupArgumentsForVarargsInvocation(
+						new Class<?>[] { Integer.class, Integer.class }, 123))
+				.withMessage("The last required parameter type must be an array to support varargs invocation");
+	}
+
 	@Test
 	void setupArgumentsForVarargsInvocation() {
 		Object[] newArray;
 
-		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] {String[].class}, "a", "b", "c");
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(new Class<?>[] { String[].class }, "a", "b", "c");
 		assertThat(newArray)
 				.singleElement()
 				.asInstanceOf(array(String[].class))
 				.containsExactly("a", "b", "c");
 
-		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] { Object[].class }, "a", "b", "c");
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(new Class<?>[] { Object[].class }, "a", "b", "c");
 		assertThat(newArray)
 				.singleElement()
 				.asInstanceOf(array(Object[].class))
 				.containsExactly("a", "b", "c");
 
 		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
 				new Class<?>[] { Integer.class, Integer.class, String[].class }, 123, 456, "a", "b", "c");
-		assertThat(newArray)
-				.satisfiesExactly(
-						i -> assertThat(i).isEqualTo(123),
-						i -> assertThat(i).isEqualTo(456),
-						i -> assertThat(i).asInstanceOf(array(String[].class)).containsExactly("a", "b", "c"));
+		assertThat(newArray).satisfiesExactly(
+				one -> assertThat(one).isEqualTo(123),
+				two -> assertThat(two).isEqualTo(456),
+				three -> assertThat(three).asInstanceOf(array(String[].class)).containsExactly("a", "b", "c"));
 
-		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] { String[].class });
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(new Class<?>[] { String[].class });
 		assertThat(newArray)
 				.singleElement()
 				.asInstanceOf(array(String[].class))
@@ -299,30 +307,18 @@ void setupArgumentsForVarargsInvocation() {
 				.asInstanceOf(array(Object[].class))
 				.containsExactly("a", "b", "c");
 
-		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] { String[].class }, "a");
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(new Class<?>[] { String[].class }, "a");
 		assertThat(newArray)
 				.singleElement()
 				.asInstanceOf(array(String[].class))
 				.containsExactly("a");
 
-
-		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] { String[].class }, new Object[]{null});
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(new Class<?>[] { String[].class }, new Object[] { null });
 		assertThat(newArray)
 				.singleElement()
 				.asInstanceOf(array(String[].class))
 				.singleElement()
 				.isNull();
-
-		assertThatThrownBy(() -> ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] { Integer.class, Integer.class }, 123, 456))
-				.isInstanceOf(IllegalArgumentException.class)
-				.hasMessage("Method must be varargs");
-
-		assertThatThrownBy(() -> ReflectionHelper.setupArgumentsForVarargsInvocation(new Class[] {}, "a", "b", "c"))
-				.isInstanceOf(IllegalArgumentException.class)
-				.hasMessage("Required parameter types must not be empty");
 	}
 
 	@Test