Skip to content

Commit 4de2aad

Browse files
sbrannensbrannen
committed
Merge branch '6.1.x'
2 parents 8cfdaaa + 83ca2c0 commit 4de2aad

File tree

3 files changed

+40
-30
lines changed

3 files changed

+40
-30
lines changed

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

+24-5
Original file line numberDiff line numberDiff line change
@@ -220,18 +220,37 @@ else if (spelParamCount != declaredParamCount) {
220220
}
221221
}
222222

223-
// more complex case, we need to look at conversion and vararg repacking
223+
// more complex case, we need to look at conversion and varargs repackaging
224224
Integer varArgPosition = null;
225225
if (isSuspectedVarargs) {
226226
varArgPosition = declaredParamCount - 1;
227227
}
228228
TypeConverter converter = state.getEvaluationContext().getTypeConverter();
229229
ReflectionHelper.convertAllMethodHandleArguments(converter, functionArgs, methodHandle, varArgPosition);
230230

231-
if (isSuspectedVarargs && declaredParamCount == 1) {
232-
// we only repack the varargs if it is the ONLY argument
233-
functionArgs = ReflectionHelper.setupArgumentsForVarargsInvocation(
234-
methodHandle.type().parameterArray(), functionArgs);
231+
if (isSuspectedVarargs) {
232+
if (declaredParamCount == 1) {
233+
// We only repackage the varargs if it is the ONLY argument -- for example,
234+
// when we are dealing with a bound MethodHandle.
235+
functionArgs = ReflectionHelper.setupArgumentsForVarargsInvocation(
236+
methodHandle.type().parameterArray(), functionArgs);
237+
}
238+
else if (spelParamCount == declaredParamCount) {
239+
// If the varargs were supplied already packaged in an array, we have to create
240+
// a new array, add the non-varargs arguments to the beginning of that array,
241+
// and add the unpackaged varargs arguments to the end of that array. The reason
242+
// is that MethodHandle.invokeWithArguments(Object...) does not expect varargs
243+
// to be packaged in an array, in contrast to how method invocation works with
244+
// reflection.
245+
int actualVarargsIndex = functionArgs.length - 1;
246+
if (actualVarargsIndex >= 0 && functionArgs[actualVarargsIndex].getClass().isArray()) {
247+
Object[] argsToUnpack = (Object[]) functionArgs[actualVarargsIndex];
248+
Object[] newArgs = new Object[actualVarargsIndex + argsToUnpack.length];
249+
System.arraycopy(functionArgs, 0, newArgs, 0, actualVarargsIndex);
250+
System.arraycopy(argsToUnpack, 0, newArgs, actualVarargsIndex, argsToUnpack.length);
251+
functionArgs = newArgs;
252+
}
253+
}
235254
}
236255

237256
try {

‎spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java

+8-8
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,10 @@ public static boolean convertAllMethodHandleArguments(TypeConverter converter, O
355355
MethodHandle methodHandle, @Nullable Integer varargsPosition) throws EvaluationException {
356356

357357
boolean conversionOccurred = false;
358-
MethodType methodHandleArgumentTypes = methodHandle.type();
358+
MethodType methodHandleType = methodHandle.type();
359359
if (varargsPosition == null) {
360360
for (int i = 0; i < arguments.length; i++) {
361-
Class<?> argumentClass = methodHandleArgumentTypes.parameterType(i);
361+
Class<?> argumentClass = methodHandleType.parameterType(i);
362362
ResolvableType resolvableType = ResolvableType.forClass(argumentClass);
363363
TypeDescriptor targetType = new TypeDescriptor(resolvableType, argumentClass, null);
364364

@@ -370,7 +370,7 @@ public static boolean convertAllMethodHandleArguments(TypeConverter converter, O
370370
else {
371371
// Convert everything up to the varargs position
372372
for (int i = 0; i < varargsPosition; i++) {
373-
Class<?> argumentClass = methodHandleArgumentTypes.parameterType(i);
373+
Class<?> argumentClass = methodHandleType.parameterType(i);
374374
ResolvableType resolvableType = ResolvableType.forClass(argumentClass);
375375
TypeDescriptor targetType = new TypeDescriptor(resolvableType, argumentClass, null);
376376

@@ -379,10 +379,10 @@ public static boolean convertAllMethodHandleArguments(TypeConverter converter, O
379379
conversionOccurred |= (argument != arguments[i]);
380380
}
381381

382-
Class<?> varArgClass = methodHandleArgumentTypes.lastParameterType().componentType();
382+
Class<?> varArgClass = methodHandleType.lastParameterType().componentType();
383383
ResolvableType varArgResolvableType = ResolvableType.forClass(varArgClass);
384-
TypeDescriptor varArgComponentType = new TypeDescriptor(varArgResolvableType, varArgClass, null);
385-
TypeDescriptor componentTypeDesc = varArgComponentType.getElementTypeDescriptor();
384+
TypeDescriptor targetType = new TypeDescriptor(varArgResolvableType, varArgClass, null);
385+
TypeDescriptor componentTypeDesc = targetType.getElementTypeDescriptor();
386386
// TODO Determine why componentTypeDesc can be null.
387387
// Assert.state(componentTypeDesc != null, "Component type must not be null for a varargs array");
388388

@@ -402,7 +402,7 @@ public static boolean convertAllMethodHandleArguments(TypeConverter converter, O
402402
// convert a String containing a comma would result in the String being split and
403403
// repackaged in an array when it should be used as-is.
404404
else if (componentTypeDesc != null && !sourceType.isAssignableTo(componentTypeDesc)) {
405-
arguments[varargsPosition] = converter.convertValue(argument, sourceType, varArgComponentType);
405+
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
406406
}
407407
// Possible outcomes of the above if-else block:
408408
// 1) the input argument was null, and nothing was done.
@@ -419,7 +419,7 @@ else if (componentTypeDesc != null && !sourceType.isAssignableTo(componentTypeDe
419419
else {
420420
for (int i = varargsPosition; i < arguments.length; i++) {
421421
Object argument = arguments[i];
422-
arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), varArgComponentType);
422+
arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType);
423423
conversionOccurred |= (argument != arguments[i]);
424424
}
425425
}

‎spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java

+8-17
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.expression.spel;
1818

19-
import org.junit.jupiter.api.Disabled;
2019
import org.junit.jupiter.api.Test;
2120

2221
import org.springframework.expression.spel.standard.SpelExpressionParser;
@@ -106,22 +105,6 @@ void functionWithVarargs() {
106105
evaluate("#varargsFunction2(9,'a',null,'b')", "9-[a, null, b]", String.class);
107106
}
108107

109-
@Disabled("Disabled until bugs are reported and fixed")
110-
@Test
111-
void functionWithVarargsViaMethodHandle_CurrentlyFailing() {
112-
// Calling 'public static String formatObjectVarargs(String format, Object... args)' -> String.format(format, args)
113-
114-
// No conversion necessary
115-
evaluate("#formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class);
116-
evaluate("#formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class);
117-
evaluate("#formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class);
118-
evaluate("#formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class);
119-
evaluate("#formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class);
120-
evaluate("#formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class);
121-
evaluate("#formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class);
122-
evaluate("#formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class);
123-
}
124-
125108
@Test // gh-33013
126109
void functionWithVarargsViaMethodHandle() {
127110
// Calling 'public static String formatObjectVarargs(String format, Object... args)' -> String.format(format, args)
@@ -138,6 +121,14 @@ void functionWithVarargsViaMethodHandle() {
138121
evaluate("#formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class);
139122
evaluate("#formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class);
140123
evaluate("#formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class);
124+
evaluate("#formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class);
125+
evaluate("#formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class);
126+
evaluate("#formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class);
127+
evaluate("#formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class);
128+
evaluate("#formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class);
129+
evaluate("#formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class);
130+
evaluate("#formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class);
131+
evaluate("#formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class);
141132

142133
// Conversion necessary
143134
evaluate("#add('2', 5.0)", 7, Integer.class);

0 commit comments

Comments
 (0)