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 a89e735f30..c45464e545 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 @@ -313,11 +313,14 @@ public abstract class ReflectionHelper { // convert it or wrap it in an array. For example, using StringToArrayConverter to convert // a String containing a comma would result in the String being split and repackaged in an // array when it should be used as-is. Similarly, if the argument is an array that is - // assignable to the varargs array type, there is no need to convert it. + // assignable to the varargs array type, there is no need to convert it. However, if the + // argument is a java.util.List, we let the TypeConverter convert the list to an array. else if (!sourceType.isAssignableTo(componentTypeDesc) || - (sourceType.isArray() && !sourceType.isAssignableTo(targetType))) { + (sourceType.isArray() && !sourceType.isAssignableTo(targetType)) || + (argument instanceof List)) { - TypeDescriptor targetTypeToUse = (sourceType.isArray() ? targetType : componentTypeDesc); + TypeDescriptor targetTypeToUse = + (sourceType.isArray() || argument instanceof List ? targetType : componentTypeDesc); arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetTypeToUse); } // Possible outcomes of the above if-else block: @@ -411,11 +414,14 @@ public abstract class ReflectionHelper { // convert it. For example, using StringToArrayConverter to convert a String containing a // comma would result in the String being split and repackaged in an array when it should // be used as-is. Similarly, if the argument is an array that is assignable to the varargs - // array type, there is no need to convert it. + // array type, there is no need to convert it. However, if the argument is a java.util.List, + // we let the TypeConverter convert the list to an array. else if (!sourceType.isAssignableTo(varargsComponentType) || - (sourceType.isArray() && !sourceType.isAssignableTo(varargsArrayType))) { + (sourceType.isArray() && !sourceType.isAssignableTo(varargsArrayType)) || + (argument instanceof List)) { - TypeDescriptor targetTypeToUse = (sourceType.isArray() ? varargsArrayType : varargsComponentType); + TypeDescriptor targetTypeToUse = + (sourceType.isArray() || argument instanceof List ? varargsArrayType : varargsComponentType); arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetTypeToUse); } // Possible outcomes of the above if-else block: diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 8ea2cdf933..3da7456189 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -32,6 +32,8 @@ import org.springframework.expression.MethodResolver; import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.StandardTypeLocator; +import org.springframework.expression.spel.testresources.Inventor; import org.springframework.expression.spel.testresources.PlaceOfBirth; import static org.assertj.core.api.Assertions.assertThat; @@ -48,12 +50,12 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; class MethodInvocationTests extends AbstractExpressionTests { @Test - void testSimpleAccess01() { + void simpleAccess() { evaluate("getPlaceOfBirth().getCity()", "Smiljan", String.class); } @Test - void testStringClass() { + void stringClass() { evaluate("new java.lang.String('hello').charAt(2)", 'l', Character.class); evaluate("new java.lang.String('hello').charAt(2).equals('l'.charAt(0))", true, Boolean.class); evaluate("'HELLO'.toLowerCase()", "hello", String.class); @@ -61,13 +63,13 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testNonExistentMethods() { + void nonExistentMethods() { // name is ok but madeup() does not exist evaluateAndCheckError("name.madeup()", SpelMessage.METHOD_NOT_FOUND, 5); } @Test - void testWidening01() { + void widening() { // widening of int 3 to double 3 is OK evaluate("new Double(3.0d).compareTo(8)", -1, Integer.class); evaluate("new Double(3.0d).compareTo(3)", 0, Integer.class); @@ -75,18 +77,19 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testArgumentConversion01() { + void argumentConversion() { // Rely on Double>String conversion for calling startsWith() evaluate("new String('hello 2.0 to you').startsWith(7.0d)", false, Boolean.class); evaluate("new String('7.0 foobar').startsWith(7.0d)", true, Boolean.class); } - @Test - void testMethodThrowingException_SPR6760() { + @Test // SPR-6760 + void methodThrowingException() { // Test method on inventor: throwException() - // On 1 it will throw an IllegalArgumentException - // On 2 it will throw a RuntimeException - // On 3 it will exit normally + // On 1 it will throw an IllegalArgumentException. + // On 2 it will throw a RuntimeException. + // On 4 it will throw a TestException. + // Otherwise, it will exit normally. // In each case it increments the Inventor field 'counter' when invoked SpelExpressionParser parser = new SpelExpressionParser(); @@ -115,7 +118,6 @@ class MethodInvocationTests extends AbstractExpressionTests { assertThat(o).isEqualTo(3); assertThat(parser.parseExpression("counter").getValue(eContext)).isEqualTo(2); - // Now cause it to throw an exception: eContext.setVariable("bar", 1); assertThatException() @@ -135,12 +137,13 @@ class MethodInvocationTests extends AbstractExpressionTests { /** * Check on first usage (when the cachedExecutor in MethodReference is null) that the exception is not wrapped. */ - @Test - void testMethodThrowingException_SPR6941() { + @Test // SPR-6941 + void methodThrowingRuntimeException() { // Test method on inventor: throwException() - // On 1 it will throw an IllegalArgumentException - // On 2 it will throw a RuntimeException - // On 3 it will exit normally + // On 1 it will throw an IllegalArgumentException. + // On 2 it will throw a RuntimeException. + // On 4 it will throw a TestException. + // Otherwise, it will exit normally. // In each case it increments the Inventor field 'counter' when invoked SpelExpressionParser parser = new SpelExpressionParser(); @@ -152,12 +155,13 @@ class MethodInvocationTests extends AbstractExpressionTests { .isNotInstanceOf(SpelEvaluationException.class); } - @Test - void testMethodThrowingException_SPR6941_2() { + @Test // SPR-6941 + void methodThrowingCustomException() { // Test method on inventor: throwException() - // On 1 it will throw an IllegalArgumentException - // On 2 it will throw a RuntimeException - // On 3 it will exit normally + // On 1 it will throw an IllegalArgumentException. + // On 2 it will throw a RuntimeException. + // On 4 it will throw a TestException. + // Otherwise, it will exit normally. // In each case it increments the Inventor field 'counter' when invoked SpelExpressionParser parser = new SpelExpressionParser(); @@ -166,12 +170,11 @@ class MethodInvocationTests extends AbstractExpressionTests { context.setVariable("bar", 4); assertThatExceptionOfType(ExpressionInvocationTargetException.class) .isThrownBy(() -> expr.getValue(context)) - .satisfies(ex -> assertThat(ex.getCause().getClass().getName()).isEqualTo( - "org.springframework.expression.spel.testresources.Inventor$TestException")); + .withCauseExactlyInstanceOf(Inventor.TestException.class); } - @Test - void testMethodFiltering_SPR6764() { + @Test // SPR-6764 + void methodFiltering() { SpelExpressionParser parser = new SpelExpressionParser(); StandardEvaluationContext context = new StandardEvaluationContext(); context.setRootObject(new TestObject()); @@ -211,7 +214,7 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testAddingMethodResolvers() { + void addingMethodResolvers() { StandardEvaluationContext ctx = new StandardEvaluationContext(); // reflective method accessor is the only one by default @@ -235,7 +238,7 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testVarargsInvocation01() { + void varargsInvocation01() { // Calling 'public String aVarargsMethod(String... strings)' evaluate("aVarargsMethod('a','b','c')", "[a, b, c]", String.class); evaluate("aVarargsMethod('a')", "[a]", String.class); @@ -252,7 +255,7 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testVarargsInvocation02() { + void varargsInvocation02() { // Calling 'public String aVarargsMethod2(int i, String... strings)' evaluate("aVarargsMethod2(5,'a','b','c')", "5-[a, b, c]", String.class); evaluate("aVarargsMethod2(2,'a')", "2-[a]", String.class); @@ -267,7 +270,7 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testVarargsInvocation03() { + void varargsInvocation03() { // Calling 'public int aVarargsMethod3(String str1, String... strings)' - returns all strings concatenated with "-" // No conversion necessary @@ -295,7 +298,7 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test // gh-33013 - void testVarargsWithObjectArrayType() { + void varargsWithObjectArrayType() { // Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) // No var-args and no conversion necessary @@ -336,7 +339,7 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testVarargsWithPrimitiveArrayType() { + void varargsWithPrimitiveArrayType() { // Calling 'public String formatPrimitiveVarargs(String format, int... nums)' -> effectively String.format(format, args) // No var-args and no conversion necessary @@ -357,13 +360,28 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testVarargsWithPrimitiveArrayToObjectArrayConversion() { + void varargsWithPrimitiveArrayToObjectArrayConversion() { evaluate("formatObjectVarargs('x -> %s %s %s', new short[]{1, 2, 3})", "x -> 1 2 3", String.class); // short[] to Object[] evaluate("formatObjectVarargs('x -> %s %s %s', new int[]{1, 2, 3})", "x -> 1 2 3", String.class); // int[] to Object[] } + @Test // gh-33315 + void varargsWithListConvertedToVarargsArray() { + ((StandardTypeLocator) context.getTypeLocator()).registerImport("java.util"); + + // Calling 'public String aVarargsMethod(String... strings)' -> Arrays.toString(strings) + String expected = "[a, b, c]"; + evaluate("aVarargsMethod(T(List).of('a', 'b', 'c'))", expected, String.class); + evaluate("aVarargsMethod({'a', 'b', 'c'})", expected, String.class); + + // Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + expected = "x -> a b c"; + evaluate("formatObjectVarargs('x -> %s %s %s', T(List).of('a', 'b', 'c'))", expected, String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', {'a', 'b', 'c'})", expected, String.class); + } + @Test - void testVarargsOptionalInvocation() { + void varargsOptionalInvocation() { // Calling 'public String optionalVarargsMethod(Optional... values)' evaluate("optionalVarargsMethod()", "[]", String.class); evaluate("optionalVarargsMethod(new String[0])", "[]", String.class); @@ -379,12 +397,12 @@ class MethodInvocationTests extends AbstractExpressionTests { } @Test - void testInvocationOnNullContextObject() { + void invocationOnNullContextObject() { evaluateAndCheckError("null.toString()",SpelMessage.METHOD_CALL_ON_NULL_OBJECT_NOT_ALLOWED); } @Test - void testMethodOfClass() { + void methodOfClass() { Expression expression = parser.parseExpression("getName()"); Object value = expression.getValue(new StandardEvaluationContext(String.class)); assertThat(value).isEqualTo("java.lang.String"); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java index c4af0e4ad8..29a4255254 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java @@ -20,6 +20,7 @@ import org.junit.jupiter.api.Test; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.StandardTypeLocator; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -211,6 +212,33 @@ class VariableAndFunctionTests extends AbstractExpressionTests { evaluate("#formatObjectVarargs('x -> %s %s %s', new int[]{1, 2, 3})", "x -> 1 2 3", String.class); // int[] to Object[] } + @Test // gh-33315 + void functionFromMethodWithListConvertedToVarargsArray() { + ((StandardTypeLocator) context.getTypeLocator()).registerImport("java.util"); + String expected = "[a, b, c]"; + + evaluate("#varargsFunction(T(List).of('a', 'b', 'c'))", expected, String.class); + evaluate("#varargsFunction({'a', 'b', 'c'})", expected, String.class); + + // Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + evaluate("#varargsObjectFunction(T(List).of('a', 'b', 'c'))", expected, String.class); + evaluate("#varargsObjectFunction({'a', 'b', 'c'})", expected, String.class); + } + + @Test // gh-33315 + void functionFromMethodHandleWithListConvertedToVarargsArray() { + ((StandardTypeLocator) context.getTypeLocator()).registerImport("java.util"); + String expected = "x -> a b c"; + + // Calling 'public static String message(String template, String... args)' -> template.formatted((Object[]) args) + evaluate("#message('x -> %s %s %s', T(List).of('a', 'b', 'c'))", expected, String.class); + evaluate("#message('x -> %s %s %s', {'a', 'b', 'c'})", expected, String.class); + + // Calling 'public static String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + evaluate("#formatObjectVarargs('x -> %s %s %s', T(List).of('a', 'b', 'c'))", expected, String.class); + evaluate("#formatObjectVarargs('x -> %s %s %s', {'a', 'b', 'c'})", expected, String.class); + } + @Test void functionMethodMustBeStatic() throws Exception { SpelExpressionParser parser = new SpelExpressionParser(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index 99c5eaa4cc..37c454d02b 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java @@ -119,20 +119,16 @@ public class Inventor { public int throwException(int valueIn) throws Exception { counter++; - if (valueIn==1) { - throw new IllegalArgumentException("IllegalArgumentException for 1"); - } - if (valueIn==2) { - throw new RuntimeException("RuntimeException for 2"); - } - if (valueIn==4) { - throw new TestException(); + switch (valueIn) { + case 1 -> throw new IllegalArgumentException("IllegalArgumentException for 1"); + case 2 -> throw new RuntimeException("RuntimeException for 2"); + case 4 -> throw new TestException(); } return valueIn; } @SuppressWarnings("serial") - static class TestException extends Exception {} + public static class TestException extends Exception {} public String throwException(PlaceOfBirth pob) { return pob.getCity();