Support compilation of varargs invocations in SpEL for array subtypes

This commit introduces support for compiling SpEL expressions that
contain varargs invocations where the supplied array is a subtype of
the declared varargs array type.

See gh-32804

spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java

@@ -16,9 +16,8 @@
 
 package org.springframework.expression.spel.ast;
 
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Member;
-import java.lang.reflect.Method;
+import java.lang.reflect.Executable;
+import java.util.Objects;
 import java.util.function.Supplier;
 
 import org.springframework.asm.MethodVisitor;
@@ -214,62 +213,67 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes {
 	 * and if it is then the argument values will be appropriately packaged into an array.
 	 * @param mv the method visitor where code should be generated
 	 * @param cf the current codeflow
-	 * @param member the method or constructor for which arguments are being set up
+	 * @param executable the method or constructor for which arguments are being set up
 	 * @param arguments the expression nodes for the expression supplied argument values
 	 */
-	protected static void generateCodeForArguments(MethodVisitor mv, CodeFlow cf, Member member, SpelNodeImpl[] arguments) {
-		String[] paramDescriptors = null;
-		boolean isVarargs = false;
-		if (member instanceof Constructor<?> ctor) {
-			paramDescriptors = CodeFlow.toDescriptors(ctor.getParameterTypes());
-			isVarargs = ctor.isVarArgs();
-		}
-		else { // Method
-			Method method = (Method)member;
-			paramDescriptors = CodeFlow.toDescriptors(method.getParameterTypes());
-			isVarargs = method.isVarArgs();
-		}
-		if (isVarargs) {
+	protected static void generateCodeForArguments(MethodVisitor mv, CodeFlow cf, Executable executable, SpelNodeImpl[] arguments) {
+		String[] paramDescriptors = CodeFlow.toDescriptors(executable.getParameterTypes());
+		int paramCount = paramDescriptors.length;
+
+		if (executable.isVarArgs()) {
 			// The final parameter may or may not need packaging into an array, or nothing may
 			// have been passed to satisfy the varargs and so something needs to be built.
-			int p = 0; // Current supplied argument being processed
-			int childCount = arguments.length;
 
 			// Fulfill all the parameter requirements except the last one
-			for (p = 0; p < paramDescriptors.length - 1; p++) {
-				cf.generateCodeForArgument(mv, arguments[p], paramDescriptors[p]);
+			for (int i = 0; i < paramCount - 1; i++) {
+				cf.generateCodeForArgument(mv, arguments[i], paramDescriptors[i]);
 			}
 
-			SpelNodeImpl lastChild = (childCount == 0 ? null : arguments[childCount - 1]);
-			String arrayType = paramDescriptors[paramDescriptors.length - 1];
-			// Determine if the final passed argument is already suitably packaged in array
-			// form to be passed to the method
-			if (lastChild != null && arrayType.equals(lastChild.getExitDescriptor())) {
-				cf.generateCodeForArgument(mv, lastChild, paramDescriptors[p]);
-			}
-			else {
-				arrayType = arrayType.substring(1); // trim the leading '[', may leave other '['
-				// build array big enough to hold remaining arguments
-				CodeFlow.insertNewArrayCode(mv, childCount - p, arrayType);
-				// Package up the remaining arguments into the array
-				int arrayindex = 0;
-				while (p < childCount) {
-					SpelNodeImpl child = arguments[p];
+			int argCount = arguments.length;
+			String varargsType = paramDescriptors[paramCount - 1];
+			String varargsComponentType = varargsType.substring(1); // trim the leading '[', may leave other '['
+
+			if (needsVarargsArrayWrapping(arguments, paramDescriptors)) {
+				// Package up the remaining arguments into an array
+				CodeFlow.insertNewArrayCode(mv, argCount - paramCount + 1, varargsComponentType);
+				for (int argIndex = paramCount - 1, arrayIndex = 0; argIndex < argCount; argIndex++, arrayIndex++) {
+					SpelNodeImpl child = arguments[argIndex];
 					mv.visitInsn(DUP);
-					CodeFlow.insertOptimalLoad(mv, arrayindex++);
-					cf.generateCodeForArgument(mv, child, arrayType);
-					CodeFlow.insertArrayStore(mv, arrayType);
-					p++;
+					CodeFlow.insertOptimalLoad(mv, arrayIndex);
+					cf.generateCodeForArgument(mv, child, varargsComponentType);
+					CodeFlow.insertArrayStore(mv, varargsComponentType);
 				}
 			}
+			else if (varargsType.equals(arguments[argCount - 1].getExitDescriptor())) {
+				// varargs type matches type of last argument
+				cf.generateCodeForArgument(mv, arguments[argCount - 1], paramDescriptors[paramCount - 1]);
+			}
+			else {
+				// last argument is an array and varargs component type is a supertype of argument component type
+				cf.generateCodeForArgument(mv, arguments[argCount - 1], varargsComponentType);
+			}
 		}
 		else {
-			for (int i = 0; i < paramDescriptors.length;i++) {
+			for (int i = 0; i < paramCount; i++) {
 				cf.generateCodeForArgument(mv, arguments[i], paramDescriptors[i]);
 			}
 		}
 	}
 
+	private static boolean needsVarargsArrayWrapping(SpelNodeImpl[] arguments, String[] paramDescriptors) {
+		if (arguments.length == 0) {
+			return true;
+		}
+
+		String lastExitTypeDescriptor = arguments[arguments.length - 1].exitTypeDescriptor;
+		String lastParamDescriptor = paramDescriptors[paramDescriptors.length - 1];
+		return countLeadingOpeningBrackets(Objects.requireNonNull(lastExitTypeDescriptor)) != countLeadingOpeningBrackets(lastParamDescriptor);
+	}
+
+	private static long countLeadingOpeningBrackets(String lastExitTypeDescriptor) {
+		return lastExitTypeDescriptor.chars().takeWhile(c -> c == '[').count();
+	}
+
 	/**
 	 * Ask an argument to generate its bytecode and then follow it up
 	 * with any boxing/unboxing/checkcasting to ensure it matches the expected parameter descriptor.

spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

@@ -28,8 +28,10 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.StringTokenizer;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import example.Color;
@@ -2267,6 +2269,12 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		return a+b;
 	}
 
+	public static String concat2(Object... args) {
+		return Arrays.stream(args)
+				.map(Objects::toString)
+				.collect(Collectors.joining());
+	}
+
 	public static String join(String...strings) {
 		StringBuilder buf = new StringBuilder();
 		for (String string: strings) {
@@ -2279,7 +2287,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 	void compiledExpressionShouldWorkWhenUsingCustomFunctionWithVarargs() throws Exception {
 		StandardEvaluationContext context;
 
-		// Here the target method takes Object... and we are passing a string
+		// single string argument
 		expression = parser.parseExpression("#doFormat('hey %s', 'there')");
 		context = new StandardEvaluationContext();
 		context.registerFunction("doFormat",
@@ -2291,6 +2299,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertThat(expression.getValue(String.class)).isEqualTo("hey there");
 
+		// single string argument from root array access
 		expression = parser.parseExpression("#doFormat([0], 'there')");
 		context = new StandardEvaluationContext(new Object[] {"hey %s"});
 		context.registerFunction("doFormat",
@@ -2302,6 +2311,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertThat(expression.getValue(String.class)).isEqualTo("hey there");
 
+		// single string from variable
 		expression = parser.parseExpression("#doFormat([0], #arg)");
 		context = new StandardEvaluationContext(new Object[] {"hey %s"});
 		context.registerFunction("doFormat",
@@ -2313,13 +2323,29 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertThat(((SpelExpression) expression).getAST().isCompilable()).isTrue();
 		assertCanCompile(expression);
 		assertThat(expression.getValue(String.class)).isEqualTo("hey there");
+
+		// string array argument
+		expression = parser.parseExpression("#doFormat('hey %s', #arg)");
+		context = new StandardEvaluationContext();
+		context.registerFunction("doFormat",
+				DelegatingStringFormat.class.getDeclaredMethod("format", String.class, Object[].class));
+		context.setVariable("arg", new String[] { "there" });
+		((SpelExpression) expression).setEvaluationContext(context);
+		assertThat(expression.getValue(String.class)).isEqualTo("hey there");
+		assertThat(((SpelExpression) expression).getAST().isCompilable()).isTrue();
+		assertCanCompile(expression);
+		assertThat(expression.getValue(String.class)).isEqualTo("hey there");
 	}
 
 	@Test
 	void functionReference() throws Exception {
 		EvaluationContext ctx = new StandardEvaluationContext();
 		Method m = getClass().getDeclaredMethod("concat", String.class, String.class);
-		ctx.setVariable("concat",m);
+		ctx.setVariable("concat", m);
+		Method m2 = getClass().getDeclaredMethod("concat2", Object[].class);
+		ctx.setVariable("concat2", m2);
+		Method m3 = getClass().getDeclaredMethod("join", String[].class);
+		ctx.setVariable("join", m3);
 
 		expression = parser.parseExpression("#concat('a','b')");
 		assertThat(expression.getValue(ctx)).isEqualTo("ab");
@@ -2331,6 +2357,20 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertThat(expression.getValue(ctx)).isEqualTo('b');
 
+		// varargs
+		expression = parser.parseExpression("#join(#stringArray)");
+		ctx.setVariable("stringArray", new String[] { "a", "b", "c" });
+		assertThat(expression.getValue(ctx)).isEqualTo("abc");
+		assertCanCompile(expression);
+		assertThat(expression.getValue(ctx)).isEqualTo("abc");
+
+		// varargs with argument component type that is a subtype of the varargs component type.
+		expression = parser.parseExpression("#concat2(#stringArray)");
+		ctx.setVariable("stringArray", new String[] { "a", "b", "c" });
+		assertThat(expression.getValue(ctx)).isEqualTo("abc");
+		assertCanCompile(expression);
+		assertThat(expression.getValue(ctx)).isEqualTo("abc");
+
 		expression = parser.parseExpression("#concat(#a,#b)");
 		ctx.setVariable("a", "foo");
 		ctx.setVariable("b", "bar");
@@ -2524,12 +2564,11 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertThat(expression.getValue(context, new SomeCompareMethod2()).toString()).isEqualTo("xyz");
 
-		// TODO Determine why the String[] is passed as the first element of the Object... varargs array instead of the entire varargs array.
-		// expression = parser.parseExpression("#append2(#stringArray)");
-		// assertThat(expression.getValue(context)).hasToString("xyz");
-		// assertThat(((SpelExpression) expression).getAST().isCompilable()).isTrue();
-		// assertCanCompile(expression);
-		// assertThat(expression.getValue(context)).hasToString("xyz");
+		expression = parser.parseExpression("#append2(#stringArray)");
+		assertThat(expression.getValue(context)).hasToString("xyz");
+		assertThat(((SpelExpression) expression).getAST().isCompilable()).isTrue();
+		assertCanCompile(expression);
+		assertThat(expression.getValue(context)).hasToString("xyz");
 
 		expression = parser.parseExpression("#sum(1,2,3)");
 		assertThat(expression.getValue(context)).isEqualTo(6);
@@ -4878,6 +4917,25 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		tc8 = (TestClass8) o;
 		assertThat(tc8.i).isEqualTo(42);
 
+		// varargs
+		expression = parser.parseExpression("new " + testclass8 + "(#root)");
+		Object[] objectArray = { "a", "b", "c" };
+		assertThat(expression.getValue(objectArray).getClass().getName()).isEqualTo(testclass8);
+		assertCanCompile(expression);
+		o = expression.getValue(objectArray);
+		assertThat(o.getClass().getName()).isEqualTo(testclass8);
+		tc8 = (TestClass8) o;
+		assertThat(tc8.args).containsExactly("a", "b", "c");
+
+		// varargs with argument component type that is a subtype of the varargs component type.
+		expression = parser.parseExpression("new " + testclass8 + "(#root)");
+		assertThat(expression.getValue(objectArray).getClass().getName()).isEqualTo(testclass8);
+		assertCanCompile(expression);
+		o = expression.getValue(new String[] { "a", "b", "c" });
+		assertThat(o.getClass().getName()).isEqualTo(testclass8);
+		tc8 = (TestClass8) o;
+		assertThat(tc8.args).containsExactly("a", "b", "c");
+
 		// private class, can't compile it
 		String testclass9 = "org.springframework.expression.spel.SpelCompilationCoverageTests$TestClass9";
 		expression = parser.parseExpression("new " + testclass9 + "(42)");
@@ -4984,6 +5042,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertThat(tc.s).isEqualTo("aaabbbccc");
 		tc.reset();
 
+		// varargs object
 		expression = parser.parseExpression("sixteen('aaa','bbb','ccc')");
 		assertCannotCompile(expression);
 		expression.getValue(tc);
@@ -4994,27 +5053,38 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertThat(tc.s).isEqualTo("aaabbbccc");
 		tc.reset();
 
+		// string array from property in varargs object
 		expression = parser.parseExpression("sixteen(seventeen)");
 		assertCannotCompile(expression);
 		expression.getValue(tc);
 		assertThat(tc.s).isEqualTo("aaabbbccc");
 		assertCanCompile(expression);
 		tc.reset();
-		// see TODO below
-		// expression.getValue(tc);
-		// assertThat(tc.s).isEqualTo("aaabbbccc");
-		// tc.reset();
-
-		// TODO Determine why the String[] is passed as the first element of the Object... varargs array instead of the entire varargs array.
-		// expression = parser.parseExpression("sixteen(stringArray)");
-		// assertCannotCompile(expression);
-		// expression.getValue(tc);
-		// assertThat(tc.s).isEqualTo("aaabbbccc");
-		// assertCanCompile(expression);
-		// tc.reset();
-		// expression.getValue(tc);
-		// assertThat(tc.s).isEqualTo("aaabbbccc");
-		// tc.reset();
+		expression.getValue(tc);
+		assertThat(tc.s).isEqualTo("aaabbbccc");
+		tc.reset();
+
+		// string array from variable in varargs object
+		expression = parser.parseExpression("sixteen(stringArray)");
+		assertCannotCompile(expression);
+		expression.getValue(tc);
+		assertThat(tc.s).isEqualTo("aaabbbccc");
+		assertCanCompile(expression);
+		tc.reset();
+		expression.getValue(tc);
+		assertThat(tc.s).isEqualTo("aaabbbccc");
+		tc.reset();
+
+		// string array in varargs object with other parameter
+		expression = parser.parseExpression("eighteen('AAA', stringArray)");
+		assertCannotCompile(expression);
+		expression.getValue(tc);
+		assertThat(tc.s).isEqualTo("AAA::aaabbbccc");
+		assertCanCompile(expression);
+		tc.reset();
+		expression.getValue(tc);
+		assertThat(tc.s).isEqualTo("AAA::aaabbbccc");
+		tc.reset();
 
 		// varargs int
 		expression = parser.parseExpression("twelve(1,2,3)");
@@ -6863,6 +6933,18 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		public String[] seventeen() {
 			return new String[] { "aaa", "bbb", "ccc" };
 		}
+
+		public void eighteen(String a, Object... vargs) {
+			if (vargs == null) {
+				s = a + "::";
+			}
+			else {
+				s = a+"::";
+				for (Object varg: vargs) {
+					s += varg;
+				}
+			}
+		}
 	}
 
 
@@ -6911,6 +6993,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		public String s;
 		public double d;
 		public boolean z;
+		public Object[] args;
 
 		public TestClass8(int i, String s, double d, boolean z) {
 			this.i = i;
@@ -6926,6 +7009,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 			this.i = i;
 		}
 
+		public TestClass8(Object... args) {
+			this.args = args;
+		}
+
 		@SuppressWarnings("unused")
 		private TestClass8(String a, String b) {
 			this.s = a+b;