When using methods with interface/superclass parameters, SpEL will compile with the first actual argument and narrow the argument type, then fail on the other implementations. The following unit test will reproduce the problem:

import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.expression.Expression;
import org.springframework.expression.spel.SpelCompilerMode;
import org.springframework.expression.spel.SpelParserConfiguration;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.util.ReflectionUtils;

class Tests {

    @Test
    void invocationOnInterfaceParameter() {
        StandardEvaluationContext context = new StandardEvaluationContext();
        // register a function which accept a parameter of interface/superclass
        context.setVariable("reverseList", ReflectionUtils.findMethod(Collections.class, "reverse", List.class));
        // use immediate compilation mode to compile the expression
        SpelExpressionParser immediateCompileParser = new SpelExpressionParser(new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null));
        // parse the expression
        Expression expression = immediateCompileParser.parseExpression("#reverseList(#root)");
        // evaluate the expression twice to trigger the compilation use one of the implementation
        expression.getValue(context, new LinkedList<>());
        expression.getValue(context, new LinkedList<>());
        // evaluate the expression with the other implementation
        expression.getValue(context, new ArrayList<>());
    }

}

I think https://github.com/spring-projects/spring-framework/blob/92ede84ea37e6b419d501293fedfb05459f30fbe/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java#L282 should pass the requiredTypeDesc in and cast to this type, or do not cast at https://github.com/spring-projects/spring-framework/blob/92ede84ea37e6b419d501293fedfb05459f30fbe/spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java#L152 ? I don't know the underlying implementation of SpEL, I'm just describing what I saw.

Comment From: TigerZCoder

I see the code at https://github.com/spring-projects/spring-framework/blob/92ede84ea37e6b419d501293fedfb05459f30fbe/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java#L297 that tries to solve it, but should the cast be replaced rather than inserted?

Comment From: sbrannen

Thanks for reporting the issue and providing the reproducer.

I have confirmed that fails on 6.2.x with the following.

org.springframework.expression.spel.SpelEvaluationException: EL1072E: An exception occurred whilst evaluating a compiled expression
    at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:332)
    at example.Tests.invocationOnInterfaceParameter(Tests.java:48)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.ClassCastException: class java.util.ArrayList cannot be cast to class java.util.LinkedList (java.util.ArrayList and java.util.LinkedList are in module java.base of loader 'bootstrap')
    at org.springframework.expression.spel.generated.CompiledExpression00001.getValue(Unknown Source)
    at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:322)
    ... 4 more

We might be able to optimize the SpEL compiler to cast to the declared parameter type for the method instead of the concrete argument type for a given evaluation of the expression.

As a side note, with which version of Spring Framework did you encounter this, and did it ever work for you with a previous Spring Framework version?

Comment From: TigerZCoder

I encountered this issue with spring-framework version 5.2.24.RELEASE and the reproducer worked.

Comment From: TigerZCoder

I found the same problem with calling instance methods on reference:

import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.expression.Expression;
import org.springframework.expression.spel.SpelCompilerMode;
import org.springframework.expression.spel.SpelParserConfiguration;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.util.ReflectionUtils;

class Tests {

    @Test
    void invocationOnInterfaceMethod() {
        StandardEvaluationContext context = new StandardEvaluationContext();
        // use immediate compilation mode to compile the expression
        SpelExpressionParser immediateCompileParser =
                new SpelExpressionParser(new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null));
        // parse the expression
        Expression expression = immediateCompileParser.parseExpression("#root.clear()");
        // evaluate the expression twice to trigger the compilation use one of the implementation
        expression.getValue(context, new LinkedList<>());
        expression.getValue(context, new LinkedList<>());
        // evaluate the expression with the other implementation
        expression.getValue(context, new ArrayList<>());
    }

}

Maybe the reference should be cast to the topmost declaring class in the method hierarchy.

Comment From: sbrannen

I encountered this issue with spring-framework version 5.2.24.RELEASE and the reproducer worked.

Thanks for the feedback.

We'll look into it.

Comment From: sbrannen

I found the same problem with calling instance methods on reference:

That is the expected behavior and is documented in the SpEL Compilation section of the reference manual:

Due to the lack of typing around expressions, the compiler uses information gathered during the interpreted evaluations of an expression when performing compilation. For example, it does not know the type of a property reference purely from the expression, but during the first interpreted evaluation, it finds out what it is. Of course, basing compilation on such derived information can cause trouble later if the types of the various expression elements change over time. For this reason, compilation is best suited to expressions whose type information is not going to change on repeated evaluations.

In immediate mode, expressions are compiled as soon as possible, typically after the first interpreted evaluation. If evaluation of the compiled expression fails (for example, due to a type changing, as described earlier), the caller of the expression evaluation receives an exception. If the types of various expression elements change over time, consider switching to MIXED mode or turning off the compiler.

Thus, we do not have any plans to change that behavior.

However, for the specific case of method/constructor invocations, I can see how it could be beneficial to cast each concrete argument to the actual declared parameter type for the method/constructor, and I will investigate options along those lines.

Comment From: sbrannen

I encountered this issue with spring-framework version 5.2.24.RELEASE and the reproducer worked.

With which Spring Framework version did your original invocationOnInterfaceParameter() test pass?

Comment From: sbrannen

I think https://github.com/spring-projects/spring-framework/blob/92ede84ea37e6b419d501293fedfb05459f30fbe/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java#L282 should pass the requiredTypeDesc in and cast to this type,

As far as I can tell, that would only work if we reliably knew upfront that the argument's type is immediately assignable to the requiredTypeDesc without boxing or unboxing.

Unfortunately, within the SpEL compiler and AST nodes, we do not always work directly with types (i.e., java.lang.Class) but rather with String-based type descriptors. So there is no easy way to determine if one type is assignable to another without parsing a type descriptor and using reflection to load the types, which is something which might not even be possible due to ClassLoader restrictions. So, that does not appear to be an option.

or do not cast at https://github.com/spring-projects/spring-framework/blob/92ede84ea37e6b419d501293fedfb05459f30fbe/spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java#L152 ?

That unfortunately breaks several existing use cases.

I see the code at https://github.com/spring-projects/spring-framework/blob/92ede84ea37e6b419d501293fedfb05459f30fbe/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java#L297 that tries to solve it,

That's correct. That code performs the proper CHECKCAST to java.util.List, but that occurs after the CHECKCAST to java.util.LinkedList (generated by VariableReference.generateCode(...)), which can be seen in the resulting byte code:

// Method descriptor #10 (Ljava/lang/Object;Lorg/springframework/expression/EvaluationContext;)Ljava/lang/Object;
// Stack: 1, Locals: 3
public java.lang.Object getValue(java.lang.Object arg0, org.springframework.expression.EvaluationContext arg1)
        throws org.springframework.expression.EvaluationException;
 0  aload_1 [arg0]
 1  checkcast java.util.LinkedList [14]
 4  checkcast java.util.List [16]
 7  invokestatic java.util.Collections.reverse(java.util.List) : void [22]
10  aconst_null
11  areturn

but should the cast be replaced rather than inserted?

As far as I know, there is no way to replace an instruction in the internal ByteVector used in ASM. So, although it would seem to be a logical solution for this particular use case, there does not seem to be a supported mechanism for achieving that.

I don't know the underlying implementation of SpEL, I'm just describing what I saw.

I understand, and thanks for sharing your thoughts.

Please note as well that we have a test which verifies the exact behavior you reported.

https://github.com/spring-projects/spring-framework/blob/6be811119efb742970862ef1faeeef71be3a7d8a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java#L214-L219

In light of the above, I am closing this issue as "works as designed", and please keep in mind what the reference manual states in this regard.

If the types of various expression elements change over time, consider switching to MIXED mode or turning off the compiler.

Cheers,

Sam

Comment From: TigerZCoder

Thank you very much for responding to this question point by point in such detail. Actually, I read the reference and chose IMMEDIATE mode for performance, and avoided MIXED mode to avoid re-run side effects, and consider type change warnings for unrelated types with the same literal signature. But when I encountered the argument cast problem, I mainly thought about it from the perspective of a strong type system like Java with complete information, without realizing that it was a type change problem. In my real case I used the superclass for a harmless warm-up, and I will evaluate my code for a more appropriate solution. Thanks a lot again.