From a4fcd301f21c7fb38033ca7c831709a29f5d5ad8 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 10 Jul 2024 11:19:36 +0200 Subject: [PATCH 1/2] Polish SpEL's ReflectionHelper --- .../spel/support/ReflectionHelper.java | 132 +++++++++--------- 1 file changed, 69 insertions(+), 63 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 b103ba0d24e..5fd242044fb 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 @@ -49,12 +49,12 @@ public abstract class ReflectionHelper { /** * Compare argument arrays and return information about whether they match. - * A supplied type converter and conversionAllowed flag allow for matches to take - * into account that a type may be transformed into a different type by the converter. + *

The supplied type converter allows for matches to take into account that a type + * may be transformed into a different type by the converter. * @param expectedArgTypes the types the method/constructor is expecting * @param suppliedArgTypes the types that are being supplied at the point of invocation * @param typeConverter a registered type converter - * @return a MatchInfo object indicating what kind of match it was, + * @return an {@code ArgumentsMatchInfo} object indicating what kind of match it was, * or {@code null} if it was not a match */ @Nullable @@ -62,7 +62,7 @@ public abstract class ReflectionHelper { List expectedArgTypes, List suppliedArgTypes, TypeConverter typeConverter) { Assert.isTrue(expectedArgTypes.size() == suppliedArgTypes.size(), - "Expected argument types and supplied argument types should be arrays of same length"); + "Expected argument types and supplied argument types should be lists of the same size"); ArgumentsMatchKind match = ArgumentsMatchKind.EXACT; for (int i = 0; i < expectedArgTypes.size() && match != null; i++) { @@ -136,13 +136,14 @@ public abstract class ReflectionHelper { /** * Compare argument arrays and return information about whether they match. - * A supplied type converter and conversionAllowed flag allow for matches to - * take into account that a type may be transformed into a different type by the - * converter. This variant of compareArguments also allows for a varargs match. + *

The supplied type converter allows for matches to take into account that a type + * may be transformed into a different type by the converter. + *

This variant of {@link #compareArguments(List, List, TypeConverter)} also allows + * for a varargs match. * @param expectedArgTypes the types the method/constructor is expecting * @param suppliedArgTypes the types that are being supplied at the point of invocation * @param typeConverter a registered type converter - * @return a MatchInfo object indicating what kind of match it was, + * @return an {@code ArgumentsMatchInfo} object indicating what kind of match it was, * or {@code null} if it was not a match */ @Nullable @@ -200,26 +201,26 @@ public abstract class ReflectionHelper { // Now... we have the final argument in the method we are checking as a match and we have 0 // or more other arguments left to pass to it. TypeDescriptor varargsDesc = expectedArgTypes.get(expectedArgTypes.size() - 1); - TypeDescriptor elementDesc = varargsDesc.getElementTypeDescriptor(); - Assert.state(elementDesc != null, "No element type"); - Class varargsParamType = elementDesc.getType(); + TypeDescriptor componentTypeDesc = varargsDesc.getElementTypeDescriptor(); + Assert.state(componentTypeDesc != null, "Component type must not be null for a varargs array"); + Class varargsComponentType = componentTypeDesc.getType(); // All remaining parameters must be of this type or convertible to this type for (int i = expectedArgTypes.size() - 1; i < suppliedArgTypes.size(); i++) { TypeDescriptor suppliedArg = suppliedArgTypes.get(i); if (suppliedArg == null) { - if (varargsParamType.isPrimitive()) { + if (varargsComponentType.isPrimitive()) { match = null; } } else { - if (varargsParamType != suppliedArg.getType()) { - if (ClassUtils.isAssignable(varargsParamType, suppliedArg.getType())) { + if (varargsComponentType != suppliedArg.getType()) { + if (ClassUtils.isAssignable(varargsComponentType, suppliedArg.getType())) { if (match != ArgumentsMatchKind.REQUIRES_CONVERSION) { match = ArgumentsMatchKind.CLOSE; } } - else if (typeConverter.canConvert(suppliedArg, TypeDescriptor.valueOf(varargsParamType))) { + else if (typeConverter.canConvert(suppliedArg, TypeDescriptor.valueOf(varargsComponentType))) { match = ArgumentsMatchKind.REQUIRES_CONVERSION; } else { @@ -234,19 +235,22 @@ public abstract class ReflectionHelper { } - // TODO could do with more refactoring around argument handling and varargs /** - * Convert a supplied set of arguments into the requested types. If the parameterTypes are related to - * a varargs method then the final entry in the parameterTypes array is going to be an array itself whose - * component type should be used as the conversion target for extraneous arguments. (For example, if the - * parameterTypes are {Integer, String[]} and the input arguments are {Integer, boolean, float} then both - * the boolean and float must be converted to strings). This method does *not* repackage the arguments - * into a form suitable for the varargs invocation - a subsequent call to setupArgumentsForVarargsInvocation handles that. + * Convert the supplied set of arguments into the parameter types of the supplied + * {@link Method}. + *

If the supplied method is a varargs method, the final parameter type must be an + * array whose component type should be used as the conversion target for extraneous + * arguments. For example, if the parameter types are {Integer, String[]} + * and the input arguments are {Integer, boolean, float}, then both the + * {@code boolean} and the {@code float} must be converted to strings. + *

This method does not repackage the arguments into a form suitable + * for the varargs invocation: a subsequent call to + * {@link #setupArgumentsForVarargsInvocation(Class[], Object...)} is required for that. * @param converter the converter to use for type conversions - * @param arguments the arguments to convert to the requested parameter types - * @param method the target Method - * @return true if some kind of conversion occurred on the argument - * @throws SpelEvaluationException if there is a problem with conversion + * @param arguments the arguments to convert to the required parameter types + * @param method the target {@code Method} + * @return {@code true} if some kind of conversion occurred on an argument + * @throws SpelEvaluationException if a problem occurs during conversion */ public static boolean convertAllArguments(TypeConverter converter, Object[] arguments, Method method) throws SpelEvaluationException { @@ -256,11 +260,12 @@ public abstract class ReflectionHelper { } /** - * Takes an input set of argument values and converts them to the types specified as the - * required parameter types. The arguments are converted 'in-place' in the input array. - * @param converter the type converter to use for attempting conversions - * @param arguments the actual arguments that need conversion - * @param executable the target Method or Constructor + * Convert the supplied set of arguments into the parameter types of the supplied + * {@link Executable}, taking the varargs position into account. + *

The arguments are converted 'in-place' in the input array. + * @param converter the converter to use for type conversions + * @param arguments the arguments to convert to the required parameter types + * @param executable the target {@code Method} or {@code Constructor} * @param varargsPosition the known position of the varargs argument, if any * ({@code null} if not varargs) * @return {@code true} if some kind of conversion occurred on an argument @@ -288,30 +293,31 @@ public abstract class ReflectionHelper { } MethodParameter methodParam = MethodParameter.forExecutable(executable, varargsPosition); + TypeDescriptor targetType = new TypeDescriptor(methodParam); + TypeDescriptor componentTypeDesc = targetType.getElementTypeDescriptor(); + Assert.state(componentTypeDesc != null, "Component type must not be null for a varargs array"); // If the target is varargs and there is just one more argument, then convert it here. if (varargsPosition == arguments.length - 1) { Object argument = arguments[varargsPosition]; - TypeDescriptor targetType = new TypeDescriptor(methodParam); TypeDescriptor sourceType = TypeDescriptor.forObject(argument); if (argument == null) { // Perform the equivalent of GenericConversionService.convertNullSource() for a single argument. - TypeDescriptor elementDesc = targetType.getElementTypeDescriptor(); - if (elementDesc != null && elementDesc.getObjectType() == Optional.class) { + if (componentTypeDesc.getObjectType() == Optional.class) { arguments[varargsPosition] = Optional.empty(); conversionOccurred = true; } } - // If the argument type is equal to the varargs element type, there is no need to + // If the argument type is equal to the varargs component type, there is no need to // 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. - else if (!sourceType.equals(targetType.getElementTypeDescriptor())) { + else if (!sourceType.equals(componentTypeDesc)) { arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); } // Possible outcomes of the above if-else block: // 1) the input argument was null, and nothing was done. - // 2) the input argument was null; the varargs element type is Optional; and the argument was converted to Optional.empty(). + // 2) the input argument was null; the varargs component type is Optional; and the argument was converted to Optional.empty(). // 3) the input argument was correct type but not wrapped in an array, and nothing was done. // 4) the input argument was already compatible (i.e., array of valid type), and nothing was done. // 5) the input argument was the wrong type and got converted and wrapped in an array. @@ -320,13 +326,12 @@ public abstract class ReflectionHelper { conversionOccurred = true; // case 5 } } - // Otherwise, convert remaining arguments to the varargs element type. + // Otherwise, convert remaining arguments to the varargs component type. else { - TypeDescriptor targetType = new TypeDescriptor(methodParam).getElementTypeDescriptor(); - Assert.state(targetType != null, "No element type"); for (int i = varargsPosition; i < arguments.length; i++) { Object argument = arguments[i]; - arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType); + TypeDescriptor sourceType = TypeDescriptor.forObject(argument); + arguments[i] = converter.convertValue(argument, sourceType, componentTypeDesc); conversionOccurred |= (argument != arguments[i]); } } @@ -335,11 +340,12 @@ public abstract class ReflectionHelper { } /** - * Takes an input set of argument values and converts them to the types specified as the - * required parameter types. The arguments are converted 'in-place' in the input array. - * @param converter the type converter to use for attempting conversions - * @param arguments the actual arguments that need conversion - * @param methodHandle the target MethodHandle + * Convert the supplied set of arguments into the parameter types of the supplied + * {@link MethodHandle}, taking the varargs position into account. + *

The arguments are converted 'in-place' in the input array. + * @param converter the converter to use for type conversions + * @param arguments the arguments to convert to the required parameter types + * @param methodHandle the target {@code MethodHandle} * @param varargsPosition the known position of the varargs argument, if any * ({@code null} if not varargs) * @return {@code true} if some kind of conversion occurred on an argument @@ -350,7 +356,7 @@ public abstract class ReflectionHelper { MethodHandle methodHandle, @Nullable Integer varargsPosition) throws EvaluationException { boolean conversionOccurred = false; - final MethodType methodHandleArgumentTypes = methodHandle.type(); + MethodType methodHandleArgumentTypes = methodHandle.type(); if (varargsPosition == null) { for (int i = 0; i < arguments.length; i++) { Class argumentClass = methodHandleArgumentTypes.parameterType(i); @@ -374,9 +380,12 @@ public abstract class ReflectionHelper { conversionOccurred |= (argument != arguments[i]); } - final Class varArgClass = methodHandleArgumentTypes.lastParameterType().componentType(); + Class varArgClass = methodHandleArgumentTypes.lastParameterType().componentType(); ResolvableType varArgResolvableType = ResolvableType.forClass(varArgClass); - TypeDescriptor varArgContentType = new TypeDescriptor(varArgResolvableType, varArgClass, null); + TypeDescriptor varArgComponentType = new TypeDescriptor(varArgResolvableType, varArgClass, null); + TypeDescriptor componentTypeDesc = varArgComponentType.getElementTypeDescriptor(); + // TODO Determine why componentTypeDesc is null. + // Assert.state(componentTypeDesc != null, "Component type must not be null for a varargs array"); // If the target is varargs and there is just one more argument, then convert it here. if (varargsPosition == arguments.length - 1) { @@ -384,22 +393,21 @@ public abstract class ReflectionHelper { TypeDescriptor sourceType = TypeDescriptor.forObject(argument); if (argument == null) { // Perform the equivalent of GenericConversionService.convertNullSource() for a single argument. - TypeDescriptor elementDesc = varArgContentType.getElementTypeDescriptor(); - if (elementDesc != null && elementDesc.getObjectType() == Optional.class) { + if (componentTypeDesc != null && componentTypeDesc.getObjectType() == Optional.class) { arguments[varargsPosition] = Optional.empty(); conversionOccurred = true; } } - // If the argument type is equal to the varargs element type, there is no need to + // If the argument type is equal to the varargs component type, there is no need to // 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. - else if (!sourceType.equals(varArgContentType.getElementTypeDescriptor())) { - arguments[varargsPosition] = converter.convertValue(argument, sourceType, varArgContentType); + else if (!sourceType.equals(componentTypeDesc)) { + arguments[varargsPosition] = converter.convertValue(argument, sourceType, varArgComponentType); } // Possible outcomes of the above if-else block: // 1) the input argument was null, and nothing was done. - // 2) the input argument was null; the varargs element type is Optional; and the argument was converted to Optional.empty(). + // 2) the input argument was null; the varargs component type is Optional; and the argument was converted to Optional.empty(). // 3) the input argument was correct type but not wrapped in an array, and nothing was done. // 4) the input argument was already compatible (i.e., array of valid type), and nothing was done. // 5) the input argument was the wrong type and got converted and wrapped in an array. @@ -408,11 +416,11 @@ public abstract class ReflectionHelper { conversionOccurred = true; // case 5 } } - // Otherwise, convert remaining arguments to the varargs element type. + // Otherwise, convert remaining arguments to the varargs component type. else { for (int i = varargsPosition; i < arguments.length; i++) { Object argument = arguments[i]; - arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), varArgContentType); + arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), varArgComponentType); conversionOccurred |= (argument != arguments[i]); } } @@ -448,7 +456,7 @@ public abstract class ReflectionHelper { * {@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 set up for the invocation - * @return a repackaged array of arguments where any varargs setup has performed + * @return a repackaged array of arguments where any varargs setup has been performed */ public static Object[] setupArgumentsForVarargsInvocation(Class[] requiredParameterTypes, Object... args) { Assert.notEmpty(requiredParameterTypes, "Required parameter types array must not be empty"); @@ -505,11 +513,9 @@ public abstract class ReflectionHelper { /** - * An instance of ArgumentsMatchInfo describes what kind of match was achieved + * An instance of {@code ArgumentsMatchInfo} describes what kind of match was achieved * between two sets of arguments - the set that a method/constructor is expecting - * and the set that are being supplied at the point of invocation. If the kind - * indicates that conversion is required for some of the arguments then the arguments - * that require conversion are listed in the argsRequiringConversion array. + * and the set that is being supplied at the point of invocation. * * @param kind the kind of match that was achieved */ @@ -529,7 +535,7 @@ public abstract class ReflectionHelper { @Override public String toString() { - return "ArgumentMatchInfo: " + this.kind; + return "ArgumentsMatchInfo: " + this.kind; } } From c55c64427c49f32472004f082402a56be056882d Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 10 Jul 2024 11:51:55 +0200 Subject: [PATCH 2/2] Support single String argument for varargs invocations in SpEL Prior to this commit, the Spring Expression Language (SpEL) incorrectly split single String arguments by comma for Object... varargs method and constructor invocations. This commit addresses this by checking if the single argument type is already "assignable" to the varargs component type instead of "equal" to the varargs component type. Closes gh-33013 --- .../spel/support/ReflectionHelper.java | 10 ++-- .../spel/MethodInvocationTests.java | 40 +++++++++++++++ .../expression/spel/SpelReproTests.java | 18 +++++-- .../expression/spel/TestScenarioCreator.java | 11 ++++- .../spel/VariableAndFunctionTests.java | 49 +++++++++++++++++++ .../spel/testresources/Inventor.java | 5 ++ 6 files changed, 122 insertions(+), 11 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 5fd242044fb..31bd84389a2 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 @@ -308,11 +308,11 @@ public abstract class ReflectionHelper { conversionOccurred = true; } } - // If the argument type is equal to the varargs component type, there is no need to + // If the argument type is assignable to the varargs component type, there is no need to // 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. - else if (!sourceType.equals(componentTypeDesc)) { + else if (!sourceType.isAssignableTo(componentTypeDesc)) { arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); } // Possible outcomes of the above if-else block: @@ -384,7 +384,7 @@ public abstract class ReflectionHelper { ResolvableType varArgResolvableType = ResolvableType.forClass(varArgClass); TypeDescriptor varArgComponentType = new TypeDescriptor(varArgResolvableType, varArgClass, null); TypeDescriptor componentTypeDesc = varArgComponentType.getElementTypeDescriptor(); - // TODO Determine why componentTypeDesc is null. + // TODO Determine why componentTypeDesc can be null. // Assert.state(componentTypeDesc != null, "Component type must not be null for a varargs array"); // If the target is varargs and there is just one more argument, then convert it here. @@ -398,11 +398,11 @@ public abstract class ReflectionHelper { conversionOccurred = true; } } - // If the argument type is equal to the varargs component type, there is no need to + // If the argument type is assignable to the varargs component type, there is no need to // 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. - else if (!sourceType.equals(componentTypeDesc)) { + else if (componentTypeDesc != null && !sourceType.isAssignableTo(componentTypeDesc)) { arguments[varargsPosition] = converter.convertValue(argument, sourceType, varArgComponentType); } // 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 626db90a46b..b84a57498ee 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 @@ -293,6 +293,46 @@ class MethodInvocationTests extends AbstractExpressionTests { evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class); } + @Test // gh-33013 + void testVarargsWithObjectArrayType() { + // Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + + // No var-args and no conversion necessary + evaluate("formatObjectVarargs('x')", "x", String.class); + + // No var-args but conversion necessary + evaluate("formatObjectVarargs(9)", "9", String.class); + + // No conversion necessary + evaluate("formatObjectVarargs('x -> %s', '')", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class); + evaluate("formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class); + + // Conversion necessary + evaluate("formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class); + evaluate("formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class); + + // Individual string contains a comma with multiple varargs arguments + evaluate("formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class); + evaluate("formatObjectVarargs('foo -> %s %s', 'bar', ',baz')", "foo -> bar ,baz", String.class); + evaluate("formatObjectVarargs('foo -> %s %s', 'bar,', 'baz')", "foo -> bar, baz", String.class); + + // Individual string contains a comma with single varargs argument. + evaluate("formatObjectVarargs('foo -> %s', ',')", "foo -> ,", String.class); + evaluate("formatObjectVarargs('foo -> %s', ',bar')", "foo -> ,bar", String.class); + evaluate("formatObjectVarargs('foo -> %s', 'bar,')", "foo -> bar,", String.class); + evaluate("formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", String.class); + } + @Test void testVarargsOptionalInvocation() { // Calling 'public String optionalVarargsMethod(Optional... values)' diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java index 60d0a14a8f2..f8491daae4e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java @@ -69,6 +69,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatException; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.InstanceOfAssertFactories.list; /** * Reproduction tests cornering various reported SpEL issues. @@ -1436,13 +1437,20 @@ class SpelReproTests extends AbstractExpressionTests { assertThat(expression.getValue(new NamedUser())).isEqualTo(NamedUser.class.getName()); } - @Test - void SPR12522() { + @Test // gh-17127, SPR-12522 + void arraysAsListWithNoArguments() { + SpelExpressionParser parser = new SpelExpressionParser(); + Expression expression = parser.parseExpression("T(java.util.Arrays).asList()"); + List value = expression.getValue(List.class); + assertThat(value).isEmpty(); + } + + @Test // gh-33013 + void arraysAsListWithSingleEmptyStringArgument() { SpelExpressionParser parser = new SpelExpressionParser(); Expression expression = parser.parseExpression("T(java.util.Arrays).asList('')"); - Object value = expression.getValue(); - assertThat(value).isInstanceOf(List.class); - assertThat(((List) value)).isEmpty(); + List value = expression.getValue(List.class); + assertThat(value).asInstanceOf(list(String.class)).containsExactly(""); } @Test diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java index 614fcf431d2..b2aec5c02cc 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java @@ -100,7 +100,12 @@ class TestScenarioCreator { MethodHandle messageStaticFullyBound = messageStaticPartiallyBound .bindTo(new String[] { "prerecorded", "3", "Oh Hello World", "ignored"}); testContext.registerFunction("messageStaticBound", messageStaticFullyBound); - } + + // #formatObjectVarargs(format, args...) + MethodHandle formatObjectVarargs = MethodHandles.lookup().findStatic(TestScenarioCreator.class, + "formatObjectVarargs", MethodType.methodType(String.class, String.class, Object[].class)); + testContext.registerFunction("formatObjectVarargs", formatObjectVarargs); +} /** * Register some variables that can be referenced from the tests @@ -154,4 +159,8 @@ class TestScenarioCreator { return template.formatted((Object[]) args); } + public static String formatObjectVarargs(String format, Object... args) { + return String.format(format, args); + } + } 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 0dfa472aea6..fbfb7cc0447 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 @@ -16,6 +16,7 @@ package org.springframework.expression.spel; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -101,6 +102,54 @@ class VariableAndFunctionTests extends AbstractExpressionTests { evaluate("#varargsFunction2(9,'a',null,'b')", "9-[a, null, b]", String.class); } + @Disabled("Disabled until bugs are reported and fixed") + @Test + void functionWithVarargsViaMethodHandle_CurrentlyFailing() { + // Calling 'public static String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + + // No var-args and no conversion necessary + evaluate("#formatObjectVarargs('x')", "x", String.class); + + // No var-args but conversion necessary + evaluate("#formatObjectVarargs(9)", "9", String.class); + + // No conversion necessary + evaluate("#formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class); + evaluate("#formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class); + evaluate("#formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class); + evaluate("#formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class); + evaluate("#formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class); + evaluate("#formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class); + evaluate("#formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class); + evaluate("#formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class); + } + + @Test // gh-33013 + void functionWithVarargsViaMethodHandle() { + // Calling 'public static String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + + // No conversion necessary + evaluate("#formatObjectVarargs('x -> %s', '')", "x -> ", String.class); + evaluate("#formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class); + evaluate("#formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class); + evaluate("#formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class); + + // Conversion necessary + evaluate("#formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class); + evaluate("#formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class); + + // Individual string contains a comma with multiple varargs arguments + evaluate("#formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class); + evaluate("#formatObjectVarargs('foo -> %s %s', 'bar', ',baz')", "foo -> bar ,baz", String.class); + evaluate("#formatObjectVarargs('foo -> %s %s', 'bar,', 'baz')", "foo -> bar, baz", String.class); + + // Individual string contains a comma with single varargs argument. + evaluate("#formatObjectVarargs('foo -> %s', ',')", "foo -> ,", String.class); + evaluate("#formatObjectVarargs('foo -> %s', ',bar')", "foo -> ,bar", String.class); + evaluate("#formatObjectVarargs('foo -> %s', 'bar,')", "foo -> bar,", String.class); + evaluate("#formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", 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 1ef7a29cd37..a40106d2124 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 @@ -217,6 +217,11 @@ public class Inventor { return str1 + "-" + String.join("-", strings); } + public String formatObjectVarargs(String format, Object... args) { + return String.format(format, args); + } + + public Inventor(String... strings) { if (strings.length > 0) { this.name = strings[0];