Merge branch '6.1.x'

sbrannen · sbrannen · commit 1e804d8d4f3b · 2024-08-05T16:37:57.000+03:00
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
@@ -313,11 +313,14 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Exe
 				// 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 static boolean convertAllMethodHandleArguments(TypeConverter converter, O
 				// 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
@@ -32,6 +32,8 @@
 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,45 +50,46 @@
 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);
 		evaluate("'   abcba '.trim()", "abcba", String.class);
 	}
 
 	@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);
 		evaluate("new Double(3.0d).compareTo(2)", 1, Integer.class);
 	}
 
 	@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 @@ void testMethodThrowingException_SPR6760() {
 		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 @@ void testMethodThrowingException_SPR6760() {
 	/**
 	 * 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 @@ void testMethodThrowingException_SPR6941() {
 			.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 @@ void testMethodThrowingException_SPR6941_2() {
 		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 @@ void testMethodFiltering_SPR6764() {
 	}
 
 	@Test
-	void testAddingMethodResolvers() {
+	void addingMethodResolvers() {
 		StandardEvaluationContext ctx = new StandardEvaluationContext();
 
 		// reflective method accessor is the only one by default
@@ -235,7 +238,7 @@ void testAddingMethodResolvers() {
 	}
 
 	@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 @@ void testVarargsInvocation01() {
 	}
 
 	@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 @@ void testVarargsInvocation02() {
 	}
 
 	@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 @@ void testVarargsInvocation03() {
 	}
 
 	@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 @@ void testVarargsWithObjectArrayType() {
 	}
 
 	@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 @@ void testVarargsWithPrimitiveArrayType() {
 	}
 
 	@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<String>... values)'
 		evaluate("optionalVarargsMethod()", "[]", String.class);
 		evaluate("optionalVarargsMethod(new String[0])", "[]", String.class);
@@ -379,12 +397,12 @@ void testVarargsOptionalInvocation() {
 	}
 
 	@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
@@ -20,6 +20,7 @@
 
 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 @@ void functionFromMethodHandleWithVarargsAndPrimitiveArrayToObjectArrayConversion
 		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
@@ -119,20 +119,16 @@ public PlaceOfBirth getPlaceOfBirth() {
 
 	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();
