Compiled SpEL expression is bound to first evaluated argument type instead of declared parameter type for method and constructor invocations

[TigerZCoder]

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

spring-framework/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java

Line 282 in 92ede84

argument.generateCode(methodVisitor, this);

should pass the requiredTypeDesc in and cast to this type, or do not cast at

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

Line 152 in 92ede84

CodeFlow.insertCheckCast(mv, this.exitTypeDescriptor);

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

[TigerZCoder]

I see the code at

spring-framework/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java

Line 297 in 92ede84

insertCheckCast(methodVisitor, requiredTypeDesc);

that tries to solve it, but should the cast be replaced rather than inserted?

[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?

[TigerZCoder]

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

[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.

[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.

[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.