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];