From a0f5c16627e265298739d261a6000b91c26121fe Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 10 Jul 2024 13:39:48 +0200 Subject: [PATCH] Support MethodHandle function invocation with zero varargs in SpEL Prior to this commit, the Spring Expression Language (SpEL) could not invoke a varargs MethodHandle function with zero variable arguments, even though the variable arguments are not required. Attempting to do so resulted in a SpelEvaluationException with an INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION message. This commit addresses this by updating the executeFunctionViaMethodHandle(...) method in FunctionReference so that it properly checks the required number of arguments for both varargs and non-varargs MethodHandle invocations. This commit also improves the error message for varargs invocations with too few arguments. For example, if the MethodHandle requires at least 1 argument plus a variable number of additional arguments and 0 arguments were supplied, the error message now states: "Incorrect number of arguments for function 'myFunc': 0 supplied but function takes 1 or more" Instead of: "Incorrect number of arguments for function 'myFunc': 0 supplied but function takes 2" Closes gh-33190 --- .../spel/ast/FunctionReference.java | 23 ++++++++++++++---- .../expression/spel/TestScenarioCreator.java | 11 ++++++++- .../spel/VariableAndFunctionTests.java | 24 ++++++++++++------- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java index 0387b65ef33f..9f715e29870b 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java @@ -177,13 +177,28 @@ private TypedValue executeFunctionViaMethodHandle(ExpressionState state, MethodH int spelParamCount = functionArgs.length; int declaredParamCount = declaredParams.parameterCount(); + // We don't use methodHandle.isVarargsCollector(), because a MethodHandle created via + // MethodHandle#bindTo() is "never a variable-arity method handle, even if the original + // target method handle was." Thus, we merely assume/suspect that varargs are supported + // if the last parameter type is an array. boolean isSuspectedVarargs = declaredParams.lastParameterType().isArray(); - if (spelParamCount < declaredParamCount || (spelParamCount > declaredParamCount && !isSuspectedVarargs)) { - // incorrect number, including more arguments and not a vararg - // perhaps a subset of arguments was provided but the MethodHandle wasn't bound? + if (isSuspectedVarargs) { + if (spelParamCount < declaredParamCount - 1) { + // Varargs, but the number of provided arguments (potentially 0) is insufficient + // for a varargs invocation for the number of declared parameters. + // + // As stated in the Javadoc for MethodHandle#asVarargsCollector(), "the caller + // must supply, at a minimum, N-1 arguments, where N is the arity of the target." + throw new SpelEvaluationException(SpelMessage.INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, + this.name, spelParamCount, (declaredParamCount - 1) + " or more"); + } + } + else if (spelParamCount != declaredParamCount) { + // Incorrect number and not varargs. Perhaps a subset of arguments was provided, + // but the MethodHandle wasn't bound? throw new SpelEvaluationException(SpelMessage.INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, - this.name, functionArgs.length, declaredParamCount); + this.name, spelParamCount, declaredParamCount); } // simplest case: the MethodHandle is fully bound or represents a static method with no params: diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java index b2aec5c02ccd..4c3442a4d844 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java @@ -105,7 +105,12 @@ private static void populateMethodHandles(StandardEvaluationContext testContext) MethodHandle formatObjectVarargs = MethodHandles.lookup().findStatic(TestScenarioCreator.class, "formatObjectVarargs", MethodType.methodType(String.class, String.class, Object[].class)); testContext.registerFunction("formatObjectVarargs", formatObjectVarargs); -} + + // #add(int, int) + MethodHandle add = MethodHandles.lookup().findStatic(TestScenarioCreator.class, + "add", MethodType.methodType(int.class, int.class, int.class)); + testContext.registerFunction("add", add); + } /** * Register some variables that can be referenced from the tests @@ -163,4 +168,8 @@ public static String formatObjectVarargs(String format, Object... args) { return String.format(format, args); } + public static int add(int x, int y) { + return x + y; + } + } 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 index fbfb7cc04471..ea543ad26ccb 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java @@ -54,9 +54,13 @@ void functionInvocationWithIncorrectNumberOfArguments() { evaluateAndCheckError("#reverseInt(1,2)", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "reverseInt", 2, 3); evaluateAndCheckError("#reverseInt(1,2,3,4)", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "reverseInt", 4, 3); - // MethodHandle: #message(template, args...) - evaluateAndCheckError("#message()", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "message", 0, 2); - evaluateAndCheckError("#message('%s')", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "message", 1, 2); + // MethodHandle: #message(String, Object...) + evaluateAndCheckError("#message()", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "message", 0, "1 or more"); + + // MethodHandle: #add(int, int) + evaluateAndCheckError("#add()", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "add", 0, 2); + evaluateAndCheckError("#add(1)", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "add", 1, 2); + evaluateAndCheckError("#add(1, 2, 3)", INCORRECT_NUMBER_OF_ARGUMENTS_TO_FUNCTION, 0, "add", 3, 2); } @Test @@ -107,12 +111,6 @@ void functionWithVarargs() { void functionWithVarargsViaMethodHandle_CurrentlyFailing() { // Calling 'public static 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', new Object[]{''})", "x -> ", String.class); evaluate("#formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class); @@ -128,13 +126,21 @@ void functionWithVarargsViaMethodHandle_CurrentlyFailing() { void functionWithVarargsViaMethodHandle() { // Calling 'public static 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("#add(3, 4)", 7, Integer.class); 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); // Conversion necessary + evaluate("#add('2', 5.0)", 7, Integer.class); 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);