Skip to content

Commit

Permalink
Support compilation of varargs invocations in SpEL for array subtypes
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@sbrannen
Mikaël Francoeur authored and sbrannen committed May 14, 2024
1 parent 29bb7b9 commit 12727a2
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 64 deletions.
86 changes: 45 additions & 41 deletions spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -214,62 +213,67 @@ protected ValueRef getValueRef(ExpressionState state) throws EvaluationException
* 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.
Expand Down
133 changes: 110 additions & 23 deletions ...ssion/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
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;
Expand Down Expand Up @@ -2267,6 +2269,12 @@ public static String concat(String a, String b) {
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) {
Expand All @@ -2279,7 +2287,7 @@ public static String join(String...strings) {
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",
Expand All @@ -2291,6 +2299,7 @@ void compiledExpressionShouldWorkWhenUsingCustomFunctionWithVarargs() throws Exc
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",
Expand All @@ -2302,6 +2311,7 @@ void compiledExpressionShouldWorkWhenUsingCustomFunctionWithVarargs() throws Exc
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",
Expand All @@ -2313,13 +2323,29 @@ void compiledExpressionShouldWorkWhenUsingCustomFunctionWithVarargs() throws Exc
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");
Expand All @@ -2331,6 +2357,20 @@ void functionReference() throws Exception {
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");
Expand Down Expand Up @@ -2524,12 +2564,11 @@ void functionReferenceVarargs_SPR12359() throws Exception {
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);
Expand Down Expand Up @@ -4878,6 +4917,25 @@ void constructorReference() {
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)");
Expand Down Expand Up @@ -4984,6 +5042,7 @@ void methodReferenceVarargs() {
assertThat(tc.s).isEqualTo("aaabbbccc");
tc.reset();

// varargs object
expression = parser.parseExpression("sixteen('aaa','bbb','ccc')");
assertCannotCompile(expression);
expression.getValue(tc);
Expand All @@ -4994,27 +5053,38 @@ void methodReferenceVarargs() {
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)");
Expand Down Expand Up @@ -6863,6 +6933,18 @@ public void sixteen(Object... vargs) {
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;
}
}
}
}


Expand Down Expand Up @@ -6911,6 +6993,7 @@ public static class TestClass8 {
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;
Expand All @@ -6926,6 +7009,10 @@ public TestClass8(Integer i) {
this.i = i;
}

public TestClass8(Object... args) {
this.args = args;
}

@SuppressWarnings("unused")
private TestClass8(String a, String b) {
this.s = a+b;
Expand Down

0 comments on commit 12727a2

Please sign in to comment.