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 e9b62ec2adb..d7013b57ef0 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -298,11 +298,11 @@ public abstract class ReflectionHelper { conversionOccurred = true; } } - // If the argument type is equal to the varargs element 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(targetType.getElementTypeDescriptor())) { + else if (!sourceType.isAssignableTo(targetType.getElementTypeDescriptor())) { arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); } // 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 7b59007195e..c1293625d93 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 @@ -294,6 +294,50 @@ public 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 Object[]{' '})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class); + + // The following assertions were cherry-picked from 6.1.x; however, they are expected + // to fail on 6.0.x and 5.3.x, since gh-32704 (Support varargs invocations in SpEL for + // varargs array subtype) was not backported. + // evaluate("formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class); + // evaluate("formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class); + // evaluate("formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", 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 public 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 00d9581f381..f7b9c382bb3 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 @@ -66,6 +66,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. @@ -1442,14 +1443,20 @@ class SpelReproTests extends AbstractExpressionTests { assertThat(expression.getValue(new NamedUser())).isEqualTo(NamedUser.class.getName()); } - @Test - @SuppressWarnings("rawtypes") - 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()).isTrue(); + 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/testresources/Inventor.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index 282622cf7d2..3979d919e09 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 @@ -213,6 +213,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) { }