Browse Source

Support safe navigation operator with void methods in SpEL

Prior to this commit the Spring Expression Language (SpEL) was able to
properly parse an expression that uses the safe navigation operator
(?.) with a method that has a `void` return type (for example,
"myObject?.doSomething()"); however, SpEL was not able to evaluate or
compile such expressions.

This commit addresses the evaluation issue by selectively not boxing
the exit type descriptor (for inclusion in the generated bytecode) when
the method's return type is `void`.

This commit addresses the compilation issue by pushing a null object
reference onto the stack in the generated byte code when the method's
return type is `void`.

Closes gh-27421
pull/31346/head
Sam Brannen 2 years ago
parent
commit
9aab4a60f5
  1. 10
      spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java
  2. 86
      spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

10
spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java

@ -259,7 +259,7 @@ public class MethodReference extends SpelNodeImpl { @@ -259,7 +259,7 @@ public class MethodReference extends SpelNodeImpl {
if (executorToCheck != null && executorToCheck.get() instanceof ReflectiveMethodExecutor reflectiveMethodExecutor) {
Method method = reflectiveMethodExecutor.getMethod();
String descriptor = CodeFlow.toDescriptor(method.getReturnType());
if (this.nullSafe && CodeFlow.isPrimitive(descriptor)) {
if (this.nullSafe && CodeFlow.isPrimitive(descriptor) && (descriptor.charAt(0) != 'V')) {
this.originalPrimitiveExitTypeDescriptor = descriptor.charAt(0);
this.exitTypeDescriptor = CodeFlow.toBoxedDescriptor(descriptor);
}
@ -359,10 +359,16 @@ public class MethodReference extends SpelNodeImpl { @@ -359,10 +359,16 @@ public class MethodReference extends SpelNodeImpl {
if (this.originalPrimitiveExitTypeDescriptor != null) {
// The output of the accessor will be a primitive but from the block above it might be null,
// so to have a 'common stack' element at skipIfNull target we need to box the primitive
// so to have a 'common stack' element at the skipIfNull target we need to box the primitive.
CodeFlow.insertBoxIfNecessary(mv, this.originalPrimitiveExitTypeDescriptor);
}
if (skipIfNull != null) {
if ("V".equals(this.exitTypeDescriptor)) {
// If the method return type is 'void', we need to push a null object
// reference onto the stack to satisfy the needs of the skipIfNull target.
mv.visitInsn(ACONST_NULL);
}
mv.visitLabel(skipIfNull);
}
}

86
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

@ -771,6 +771,34 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @@ -771,6 +771,34 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertThat(expression.getValue(context)).isNull();
}
@Test // gh-27421
public void nullSafeMethodChainingWithNonStaticVoidMethod() throws Exception {
FooObjectHolder foh = new FooObjectHolder();
StandardEvaluationContext context = new StandardEvaluationContext(foh);
SpelExpression expression = (SpelExpression) parser.parseExpression("getFoo()?.doFoo()");
FooObject.doFooInvoked = false;
assertThat(expression.getValue(context)).isNull();
assertThat(FooObject.doFooInvoked).isTrue();
FooObject.doFooInvoked = false;
foh.foo = null;
assertThat(expression.getValue(context)).isNull();
assertThat(FooObject.doFooInvoked).isFalse();
assertCanCompile(expression);
FooObject.doFooInvoked = false;
foh.foo = new FooObject();
assertThat(expression.getValue(context)).isNull();
assertThat(FooObject.doFooInvoked).isTrue();
FooObject.doFooInvoked = false;
foh.foo = null;
assertThat(expression.getValue(context)).isNull();
assertThat(FooObject.doFooInvoked).isFalse();
}
@Test
public void nullsafeMethodChaining_SPR16489() throws Exception {
FooObjectHolder foh = new FooObjectHolder();
@ -3898,37 +3926,71 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @@ -3898,37 +3926,71 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
tc.reset();
}
@Test
void nullSafeInvocationOfNonStaticVoidWrapperMethod() {
@Test // gh-27421
public void nullSafeInvocationOfNonStaticVoidMethod() {
// non-static method, no args, void return
expression = parser.parseExpression("new %s()?.one()".formatted(TestClass5.class.getName()));
assertCantCompile(expression);
TestClass5._i = 0;
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
TestClass5._i = 0;
assertCanCompile(expression);
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
}
@Test // gh-27421
public void nullSafeInvocationOfStaticVoidMethod() {
// static method, no args, void return
expression = parser.parseExpression("T(%s)?.two()".formatted(TestClass5.class.getName()));
assertCantCompile(expression);
TestClass5._i = 0;
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
TestClass5._i = 0;
assertCanCompile(expression);
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
}
@Test // gh-27421
public void nullSafeInvocationOfNonStaticVoidWrapperMethod() {
// non-static method, no args, Void return
expression = parser.parseExpression("new %s()?.oneVoidWrapper()".formatted(TestClass5.class.getName()));
assertCantCompile(expression);
TestClass5._i = 0;
expression.getValue();
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
TestClass5._i = 0;
assertCanCompile(expression);
expression.getValue();
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
}
@Test
void nullSafeInvocationOfStaticVoidWrapperMethod() {
@Test // gh-27421
public void nullSafeInvocationOfStaticVoidWrapperMethod() {
// static method, no args, Void return
expression = parser.parseExpression("T(%s)?.twoVoidWrapper()".formatted(TestClass5.class.getName()));
assertCantCompile(expression);
TestClass5._i = 0;
expression.getValue();
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
TestClass5._i = 0;
assertCanCompile(expression);
expression.getValue();
assertThat(expression.getValue()).isNull();
assertThat(TestClass5._i).isEqualTo(1);
}
@ -5496,7 +5558,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @@ -5496,7 +5558,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
public static class FooObject {
static boolean doFooInvoked = false;
public Object getObject() { return "hello"; }
public void doFoo() { doFooInvoked = true; }
}
@ -5744,7 +5809,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @@ -5744,7 +5809,10 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
field = null;
}
public void one() { i = 1; }
public void one() {
_i = 1;
this.i = 1;
}
public Void oneVoidWrapper() {
_i = 1;

Loading…
Cancel
Save