Fix array wrapping in varargs invocations.

Author: LeMikaelF

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

@@ -36,6 +36,8 @@
 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.
@@ -457,14 +459,18 @@ private static boolean isFirstEntryInArray(Object value, @Nullable Object possib
 	 * @return a repackaged array of arguments where any varargs setup has been done
 	 */
 	public static Object[] setupArgumentsForVarargsInvocation(Class<?>[] requiredParameterTypes, Object... args) {
-		// Check if array already built for final argument
 		int parameterCount = requiredParameterTypes.length;
+		Assert.notEmpty(requiredParameterTypes, "Required parameter types must not be empty");
+
+		Class<?> lastRequiredParameterType = requiredParameterTypes[parameterCount - 1];
+		Assert.isTrue(lastRequiredParameterType.isArray(), "Method must be varargs");
+
 		int argumentCount = args.length;
+		Object lastArgument = argumentCount > 0 ? args[argumentCount - 1] : null;
 
 		// Check if repackaging is needed...
 		if (parameterCount != args.length ||
-				requiredParameterTypes[parameterCount - 1] !=
-						(args[argumentCount - 1] != null ? args[argumentCount - 1].getClass() : null)) {
+			(!isArray(lastArgument) && differentTypes(lastRequiredParameterType, lastArgument))) {
 
 			// Create an array for the leading arguments plus the varargs array argument.
 			Object[] newArgs = new Object[parameterCount];
@@ -477,7 +483,7 @@ public static Object[] setupArgumentsForVarargsInvocation(Class<?>[] requiredPar
 			if (argumentCount >= parameterCount) {
 				varargsArraySize = argumentCount - (parameterCount - 1);
 			}
-			Class<?> componentType = requiredParameterTypes[parameterCount - 1].componentType();
+			Class<?> componentType = lastRequiredParameterType.componentType();
 			Object varargsArray = Array.newInstance(componentType, varargsArraySize);
 			for (int i = 0; i < varargsArraySize; i++) {
 				Array.set(varargsArray, i, args[parameterCount - 1 + i]);
@@ -489,6 +495,9 @@ public static Object[] setupArgumentsForVarargsInvocation(Class<?>[] requiredPar
 		return args;
 	}
 
+	private static boolean differentTypes(Class<?> lastRequiredParameterType, @Nullable Object lastArgument) {
+		return lastArgument == null || lastRequiredParameterType != lastArgument.getClass();
+	}
 
 	/**
 	 * Arguments match kinds.

spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

@@ -4994,6 +4994,17 @@ void methodReferenceVarargs() {
 		assertThat(tc.s).isEqualTo("aaabbbccc");
 		tc.reset();
 
+		expression = parser.parseExpression("sixteen(seventeen)");
+		assertCannotCompile(expression);
+		expression.getValue(tc);
+		assertThat(tc.s).isEqualTo("aaabbbccc");
+		assertCanCompile(expression);
+		tc.reset();
+		// see TODO below
+		// expression.getValue(tc);
+		// assertThat(tc.s).isEqualTo("aaabbbccc");
+		// tc.reset();
+
 		// TODO Determine why the String[] is passed as the first element of the Object... varargs array instead of the entire varargs array.
 		// expression = parser.parseExpression("sixteen(stringArray)");
 		// assertCantCompile(expression);
@@ -6620,6 +6631,10 @@ public void concat2(Object... vargs) {
 				}
 			}
 		}
+
+		public String[] seventeen() {
+			return new String[] { "aaa", "bbb", "ccc" };
+		}
 	}
 
 

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

@@ -40,6 +40,8 @@
 
 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.InstanceOfAssertFactories.array;
 import static org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind.CLOSE;
 import static org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind.EXACT;
 import static org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind.REQUIRES_CONVERSION;
@@ -252,14 +254,75 @@ void convertAllArguments() throws Exception {
 
 	@Test
 	void setupArgumentsForVarargsInvocation() {
-		Object[] newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
-				new Class<?>[] {String[].class}, "a", "b", "c");
+		Object[] newArray;
 
-		assertThat(newArray).hasSize(1);
-		Object firstParam = newArray[0];
-		assertThat(firstParam.getClass().componentType()).isEqualTo(String.class);
-		Object[] firstParamArray = (Object[]) firstParam;
-		assertThat(firstParamArray).containsExactly("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");
+		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"));
+
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
+				new Class<?>[] { String[].class });
+		assertThat(newArray)
+				.singleElement()
+				.asInstanceOf(array(String[].class))
+				.isEmpty();
+
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
+				new Class<?>[] { String[].class }, new Object[] { new String[] { "a", "b", "c" } });
+		assertThat(newArray)
+				.singleElement()
+				.asInstanceOf(array(String[].class))
+				.containsExactly("a", "b", "c");
+
+		newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(
+				new Class<?>[] { Object[].class }, new Object[] { new String[] { "a", "b", "c" } });
+		assertThat(newArray)
+				.singleElement()
+				.asInstanceOf(array(Object[].class))
+				.containsExactly("a", "b", "c");
+
+		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});
+		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