From fcc99a67b6e9ffe34c170a7f817353408ec567da Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 5 Aug 2024 16:27:55 +0300 Subject: [PATCH] Support lists for varargs invocations in SpEL The changes made in conjunction with gh-33013 resulted in a regression for varargs support in SpEL expressions. Specifically, before gh-33013 one could supply a list -- for example, an "inline list" -- as the varargs array when invoking a varargs constructor, method, or function within a SpEL expression. However, after gh-33013 an inline list (or collection in general) is no longer converted to an array for varargs invocations. Instead, the list is supplied as a single argument of the resulting varargs array which breaks applications that depend on the previous behavior. Although it was never intended that one could supply a collection as the set of varargs, we concede that this is a regression in existing behavior, and this commit therefore restores support for supplying a java.util.List as the varargs "array". In addition, this commit introduces the same "list to array" conversion support for MethodHandle-based functions that accept varargs. Note, however, that this commit does not restore support for converting arbitrary single objects to an array for varargs invocations if the single object is already an instance of the varargs array's component type. See gh-33013 Closes gh-33315 --- .../spel/support/ReflectionHelper.java | 18 ++++++++---- .../spel/MethodInvocationTests.java | 16 +++++++++++ .../spel/VariableAndFunctionTests.java | 28 +++++++++++++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) 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 b0f2943b6f6..10854120d5d 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 @@ -314,11 +314,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: @@ -413,11 +416,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 6782245d262..3da7456189d 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,7 @@ 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; @@ -364,6 +365,21 @@ class MethodInvocationTests 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 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 varargsOptionalInvocation() { // Calling 'public String optionalVarargsMethod(Optional... values)' 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 c4af0e4ad8b..29a4255254a 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();