diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index 1e311e2ed26..010a797904d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -448,24 +448,29 @@ public abstract class ReflectionHelper { } /** - * Package up the arguments so that they correctly match what is expected in requiredParameterTypes. - *

For example, if requiredParameterTypes is {@code (int, String[])} because the second parameter - * was declared {@code String...}, then if arguments is {@code [1,"a","b"]} then it must be - * repackaged as {@code [1,new String[]{"a","b"}]} in order to match the expected types. + * Package up the supplied {@code args} so that they correctly match what is + * expected in {@code requiredParameterTypes}. + *

For example, if {@code requiredParameterTypes} is {@code (int, String[])} + * because the second parameter was declared as {@code String...}, then if + * {@code args} is {@code [1, "a", "b"]} it must be repackaged as + * {@code [1, new String[] {"a", "b"}]} in order to match the expected types. * @param requiredParameterTypes the types of the parameters for the invocation - * @param args the arguments to be setup ready for the invocation - * @return a repackaged array of arguments where any varargs setup has been done + * @param args the arguments to be set up for the invocation + * @return a repackaged array of arguments where any varargs setup has performed */ public static Object[] setupArgumentsForVarargsInvocation(Class[] requiredParameterTypes, Object... args) { - // Check if array already built for final argument + Assert.notEmpty(requiredParameterTypes, "Required parameter types array must not be empty"); + int parameterCount = requiredParameterTypes.length; + Class lastRequiredParameterType = requiredParameterTypes[parameterCount - 1]; + 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); // Check if repackaging is needed... - if (parameterCount != args.length || - requiredParameterTypes[parameterCount - 1] != - (args[argumentCount - 1] != null ? args[argumentCount - 1].getClass() : null)) { - + 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. @@ -477,7 +482,7 @@ public abstract class ReflectionHelper { 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]); @@ -486,6 +491,7 @@ public abstract class ReflectionHelper { newArgs[newArgs.length - 1] = varargsArray; return newArgs; } + return args; } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index d664d1f7974..32dd5eac5f9 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -4994,9 +4994,20 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { 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); + // assertCannotCompile(expression); // expression.getValue(tc); // assertThat(tc.s).isEqualTo("aaabbbccc"); // assertCanCompile(expression); @@ -6848,6 +6859,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } } } + + public String[] seventeen() { + return new String[] { "aaa", "bbb", "ccc" }; + } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java index 1e990417dd8..8a2884e8f11 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java @@ -40,6 +40,8 @@ import org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMat import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +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; import static org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind.REQUIRES_CONVERSION; @@ -251,15 +253,72 @@ class ReflectionHelperTests extends AbstractExpressionTests { } @Test - void setupArgumentsForVarargsInvocation() { - Object[] newArray = ReflectionHelper.setupArgumentsForVarargsInvocation( - new Class[] {String[].class}, "a", "b", "c"); + void setupArgumentsForVarargsInvocationPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> ReflectionHelper.setupArgumentsForVarargsInvocation(new Class[] {}, "a")) + .withMessage("Required parameter types array must not be empty"); - 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"); + 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"); + 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( + 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 }); + 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(); } @Test