Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Support compilation of null-safe index operations in SpEL

Browse Source 
See gh-29847
This commit is contained in:
Sam Brannen
2024-03-22 17:44:37 +01:00
parent d2bd0d5716
commit 38c473fd05
2 changed files with 251 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java
View File

@@ -22,6 +22,7 @@ import java.util.List;
import java.util.Map;
import java.util.function.Supplier;
import org.springframework.asm.Label;
import org.springframework.asm.MethodVisitor;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.expression.AccessException;
@@ -73,6 +74,9 @@ public class Indexer extends SpelNodeImpl {
@Nullable
private IndexedType indexedType;
@Nullable
private String originalPrimitiveExitTypeDescriptor;
@Nullable
private volatile String arrayTypeDescriptor;
@@ -271,6 +275,17 @@ public class Indexer extends SpelNodeImpl {
cf.loadTarget(mv);
}
Label skipIfNull = null;
if (this.nullSafe) {
mv.visitInsn(DUP);
skipIfNull = new Label();
Label continueLabel = new Label();
mv.visitJumpInsn(IFNONNULL, continueLabel);
CodeFlow.insertCheckCast(mv, exitTypeDescriptor);
mv.visitJumpInsn(GOTO, skipIfNull);
mv.visitLabel(continueLabel);
}
SpelNodeImpl index = this.children[0];
if (this.indexedType == IndexedType.ARRAY) {
@@ -333,6 +348,16 @@ public class Indexer extends SpelNodeImpl {
}
cf.pushDescriptor(exitTypeDescriptor);
if (skipIfNull != null) {
if (this.originalPrimitiveExitTypeDescriptor != null) {
// The output of the indexer is a primitive, but from the logic above it
// might be null. So, to have a common stack element type at the skipIfNull
// target, it is necessary to box the primitive.
CodeFlow.insertBoxIfNecessary(mv, this.originalPrimitiveExitTypeDescriptor);
}
mv.visitLabel(skipIfNull);
}
}
@Override
@@ -396,56 +421,56 @@ public class Indexer extends SpelNodeImpl {
if (arrayComponentType == boolean.class) {
boolean[] array = (boolean[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "Z";
setExitTypeDescriptor("Z");
this.arrayTypeDescriptor = "[Z";
return array[idx];
}
else if (arrayComponentType == byte.class) {
byte[] array = (byte[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "B";
setExitTypeDescriptor("B");
this.arrayTypeDescriptor = "[B";
return array[idx];
}
else if (arrayComponentType == char.class) {
char[] array = (char[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "C";
setExitTypeDescriptor("C");
this.arrayTypeDescriptor = "[C";
return array[idx];
}
else if (arrayComponentType == double.class) {
double[] array = (double[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "D";
setExitTypeDescriptor("D");
this.arrayTypeDescriptor = "[D";
return array[idx];
}
else if (arrayComponentType == float.class) {
float[] array = (float[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "F";
setExitTypeDescriptor("F");
this.arrayTypeDescriptor = "[F";
return array[idx];
}
else if (arrayComponentType == int.class) {
int[] array = (int[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "I";
setExitTypeDescriptor("I");
this.arrayTypeDescriptor = "[I";
return array[idx];
}
else if (arrayComponentType == long.class) {
long[] array = (long[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "J";
setExitTypeDescriptor("J");
this.arrayTypeDescriptor = "[J";
return array[idx];
}
else if (arrayComponentType == short.class) {
short[] array = (short[]) ctx;
checkAccess(array.length, idx);
this.exitTypeDescriptor = "S";
setExitTypeDescriptor("S");
this.arrayTypeDescriptor = "[S";
return array[idx];
}
@@ -453,7 +478,7 @@ public class Indexer extends SpelNodeImpl {
Object[] array = (Object[]) ctx;
checkAccess(array.length, idx);
Object retValue = array[idx];
this.exitTypeDescriptor = CodeFlow.toDescriptor(arrayComponentType);
setExitTypeDescriptor(CodeFlow.toDescriptor(arrayComponentType));
this.arrayTypeDescriptor = CodeFlow.toDescriptor(array.getClass());
return retValue;
}
@@ -466,6 +491,19 @@ public class Indexer extends SpelNodeImpl {
}
}
private void setExitTypeDescriptor(String descriptor) {
// If this indexer would return a primitive - and yet it is also marked
// null-safe - then the exit type descriptor must be promoted to the box
// type to allow a null value to be passed on.
if (this.nullSafe && CodeFlow.isPrimitive(descriptor)) {
this.originalPrimitiveExitTypeDescriptor = descriptor;
this.exitTypeDescriptor = CodeFlow.toBoxedDescriptor(descriptor);
}
else {
this.exitTypeDescriptor = descriptor;
}
}
@SuppressWarnings("unchecked")
private <T> T convertValue(TypeConverter converter, @Nullable Object value, Class<T> targetType) {
T result = (T) converter.convertValue(
@@ -602,7 +640,7 @@ public class Indexer extends SpelNodeImpl {
Indexer.this.cachedReadName = this.name;
Indexer.this.cachedReadTargetType = targetObjectRuntimeClass;
if (accessor instanceof CompilablePropertyAccessor compilablePropertyAccessor) {
Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType());
setExitTypeDescriptor(CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType()));
}
return accessor.read(this.evaluationContext, this.targetObject, this.name);
}

203
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java
View File

@@ -725,6 +725,198 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
}
@Nested
class NullSafeIndexTests { // gh-29847
private final RootContextWithIndexedProperties rootContext = new RootContextWithIndexedProperties();
private final StandardEvaluationContext context = new StandardEvaluationContext(rootContext);
@Test
void nullSafeIndexIntoPrimitiveIntArray() {
expression = parser.parseExpression("intArray?.[0]");
// Cannot compile before the array type is known.
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.intArray = new int[] { 8, 9, 10 };
assertThat(expression.getValue(context)).isEqualTo(8);
assertCanCompile(expression);
assertThat(expression.getValue(context)).isEqualTo(8);
// Normally we would expect the exit type descriptor to be "I" for an
// element of an int[]. However, with null-safe indexing support the
// only way for it to evaluate to null is to box the 'int' to an 'Integer'.
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer");
// Null-safe support should have been compiled once the array type is known.
rootContext.intArray = null;
assertThat(expression.getValue(context)).isNull();
assertCanCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer");
}
@Test
void nullSafeIndexIntoNumberArray() {
expression = parser.parseExpression("numberArray?.[0]");
// Cannot compile before the array type is known.
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.numberArray = new Number[] { 8, 9, 10 };
assertThat(expression.getValue(context)).isEqualTo(8);
assertCanCompile(expression);
assertThat(expression.getValue(context)).isEqualTo(8);
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Number");
// Null-safe support should have been compiled once the array type is known.
rootContext.numberArray = null;
assertThat(expression.getValue(context)).isNull();
assertCanCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Number");
}
@Test
void nullSafeIndexIntoList() {
expression = parser.parseExpression("list?.[0]");
// Cannot compile before the list type is known.
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.list = List.of(42);
assertThat(expression.getValue(context)).isEqualTo(42);
assertCanCompile(expression);
assertThat(expression.getValue(context)).isEqualTo(42);
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object");
// Null-safe support should have been compiled once the list type is known.
rootContext.list = null;
assertThat(expression.getValue(context)).isNull();
assertCanCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object");
}
@Test
void nullSafeIndexIntoSetCannotBeCompiled() {
expression = parser.parseExpression("set?.[0]");
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.set = Set.of(42);
assertThat(expression.getValue(context)).isEqualTo(42);
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isEqualTo(42);
assertThat(getAst().getExitDescriptor()).isNull();
}
@Test
void nullSafeIndexIntoStringCannotBeCompiled() {
expression = parser.parseExpression("string?.[0]");
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.string = "XYZ";
assertThat(expression.getValue(context)).isEqualTo("X");
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isEqualTo("X");
assertThat(getAst().getExitDescriptor()).isNull();
}
@Test
void nullSafeIndexIntoMap() {
expression = parser.parseExpression("map?.['enigma']");
// Cannot compile before the map type is known.
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.map = Map.of("enigma", 42);
assertThat(expression.getValue(context)).isEqualTo(42);
assertCanCompile(expression);
assertThat(expression.getValue(context)).isEqualTo(42);
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object");
// Null-safe support should have been compiled once the map type is known.
rootContext.map = null;
assertThat(expression.getValue(context)).isNull();
assertCanCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object");
}
@Test
void nullSafeIndexIntoObjectViaPrimitiveProperty() {
expression = parser.parseExpression("person?.['age']");
// Cannot compile before the Person type is known.
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.person = new Person("Jane");
rootContext.person.setAge(42);
assertThat(expression.getValue(context)).isEqualTo(42);
assertCanCompile(expression);
assertThat(expression.getValue(context)).isEqualTo(42);
// Normally we would expect the exit type descriptor to be "I" for
// an int. However, with null-safe indexing support the only way
// for it to evaluate to null is to box the 'int' to an 'Integer'.
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer");
// Null-safe support should have been compiled once the Person type is known.
rootContext.person = null;
assertThat(expression.getValue(context)).isNull();
assertCanCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Integer");
}
@Test
void nullSafeIndexIntoObjectViaStringProperty() {
expression = parser.parseExpression("person?.['name']");
// Cannot compile before the Person type is known.
assertThat(expression.getValue(context)).isNull();
assertCannotCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isNull();
rootContext.person = new Person("Jane");
assertThat(expression.getValue(context)).isEqualTo("Jane");
assertCanCompile(expression);
assertThat(expression.getValue(context)).isEqualTo("Jane");
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/String");
// Null-safe support should have been compiled once the Person type is known.
rootContext.person = null;
assertThat(expression.getValue(context)).isNull();
assertCanCompile(expression);
assertThat(expression.getValue(context)).isNull();
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/String");
}
}
@Nested
class PropertyVisibilityTests {
@@ -6736,4 +6928,15 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
}
}
// Must be public with public fields/properties.
public static class RootContextWithIndexedProperties {
public int[] intArray;
public Number[] numberArray;
public List<Integer> list;
public Set<Integer> set;
public String string;
public Map<String, Integer> map;
public Person person;
}
}