From 9f4d46fe3374cc1e2ca8b2616107680432650a2e Mon Sep 17 00:00:00 2001 From: Grigory Stepanov Date: Wed, 18 Jan 2023 20:14:06 +0300 Subject: [PATCH 1/5] Introduce null-safe index operator in SpEL See gh-29847 --- .../expression/spel/ast/Indexer.java | 15 +++++++------ .../InternalSpelExpressionParser.java | 12 ++++++----- .../expression/spel/IndexingTests.java | 21 +++++++++++++++++++ 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 723e80b1ed3..1477724b1b8 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -103,12 +103,12 @@ public class Indexer extends SpelNodeImpl { private PropertyAccessor cachedWriteAccessor; - /** - * Create an {@code Indexer} with the given start position, end position, and - * index expression. - */ - public Indexer(int startPos, int endPos, SpelNodeImpl indexExpression) { - super(startPos, endPos, indexExpression); + private final boolean nullSafe; + + + public Indexer(boolean nullSafe, int startPos, int endPos, SpelNodeImpl expr) { + super(startPos, endPos, expr); + this.nullSafe = nullSafe; } @@ -161,6 +161,9 @@ public class Indexer extends SpelNodeImpl { // Raise a proper exception in case of a null target if (target == null) { + if (this.nullSafe) { + return ValueRef.NullValueRef.INSTANCE; + } throw new SpelEvaluationException(getStartPosition(), SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java index 33af0c254fc..c998c350f1a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java @@ -399,7 +399,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { @Nullable private SpelNodeImpl eatNonDottedNode() { if (peekToken(TokenKind.LSQUARE)) { - if (maybeEatIndexer()) { + if (maybeEatIndexer(false)) { return pop(); } } @@ -419,7 +419,8 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { Token t = takeToken(); // it was a '.' or a '?.' boolean nullSafeNavigation = (t.kind == TokenKind.SAFE_NAVI); if (maybeEatMethodOrProperty(nullSafeNavigation) || maybeEatFunctionOrVar() || - maybeEatProjection(nullSafeNavigation) || maybeEatSelection(nullSafeNavigation)) { + maybeEatProjection(nullSafeNavigation) || maybeEatSelection(nullSafeNavigation) || + maybeEatIndexer(nullSafeNavigation)) { return pop(); } if (peekToken() == null) { @@ -537,7 +538,8 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { else if (maybeEatBeanReference()) { return pop(); } - else if (maybeEatProjection(false) || maybeEatSelection(false) || maybeEatIndexer()) { + else if (maybeEatProjection(false) || maybeEatSelection(false) || + maybeEatIndexer(false)) { return pop(); } else if (maybeEatInlineListOrMap()) { @@ -699,7 +701,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { return true; } - private boolean maybeEatIndexer() { + private boolean maybeEatIndexer(boolean nullSafeNavigation) { Token t = peekToken(); if (t == null || !peekToken(TokenKind.LSQUARE, true)) { return false; @@ -709,7 +711,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { throw internalException(t.startPos, SpelMessage.MISSING_SELECTION_EXPRESSION); } eatToken(TokenKind.RSQUARE); - this.constructedNodes.push(new Indexer(t.startPos, t.endPos, expr)); + this.constructedNodes.push(new Indexer(nullSafeNavigation, t.startPos, t.endPos, expr)); return true; } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java index 5ee8a2d2b0a..af57312a9b1 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java @@ -376,6 +376,20 @@ class IndexingTests { assertThat(expression.getValue(this, String.class)).isEqualTo("apple"); } + @Test + void nullSafeIndex() { + ContextWithNullCollections testContext = new ContextWithNullCollections(); + StandardEvaluationContext context = new StandardEvaluationContext(testContext); + Expression expr = new SpelExpressionParser().parseRaw("nullList?.[4]"); + assertThat(expr.getValue(context)).isNull(); + + expr = new SpelExpressionParser().parseRaw("nullArray?.[4]"); + assertThat(expr.getValue(context)).isNull(); + + expr = new SpelExpressionParser().parseRaw("nullMap:?.[4]"); + assertThat(expr.getValue(context)).isNull(); + } + @Target({ElementType.FIELD}) @Retention(RetentionPolicy.RUNTIME) @@ -436,4 +450,11 @@ class IndexingTests { } + + static class ContextWithNullCollections { + public List nullList = null; + public String[] nullArray = null; + public Map nullMap = null; + } + } From 4d433174eb23b34f514350422b348147b75a314a Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 21 Feb 2024 17:28:50 +0100 Subject: [PATCH 2/5] Revise null-safe index operator support in SpEL See gh-29847 --- .../expression/spel/ast/Indexer.java | 40 ++++++--- .../InternalSpelExpressionParser.java | 3 +- .../expression/spel/IndexingTests.java | 86 +++++++++++++++---- 3 files changed, 98 insertions(+), 31 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 1477724b1b8..59c4fdaf21a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -68,6 +68,8 @@ public class Indexer extends SpelNodeImpl { private enum IndexedType {ARRAY, LIST, MAP, STRING, OBJECT} + private final boolean nullSafe; + @Nullable private IndexedType indexedType; @@ -103,11 +105,24 @@ public class Indexer extends SpelNodeImpl { private PropertyAccessor cachedWriteAccessor; - private final boolean nullSafe; - + /** + * Create an {@code Indexer} with the given start position, end position, and + * index expression. + * @see #Indexer(boolean, int, int, SpelNodeImpl) + * @deprecated as of Spring Framework 6.2, in favor of {@link #Indexer(boolean, int, int, SpelNodeImpl)} + */ + @Deprecated(since = "6.2", forRemoval = true) + public Indexer(int startPos, int endPos, SpelNodeImpl indexExpression) { + this(false, startPos, endPos, indexExpression); + } - public Indexer(boolean nullSafe, int startPos, int endPos, SpelNodeImpl expr) { - super(startPos, endPos, expr); + /** + * Create an {@code Indexer} with the given null-safe flag, start position, + * end position, and index expression. + * @since 6.2 + */ + public Indexer(boolean nullSafe, int startPos, int endPos, SpelNodeImpl indexExpression) { + super(startPos, endPos, indexExpression); this.nullSafe = nullSafe; } @@ -136,6 +151,15 @@ public class Indexer extends SpelNodeImpl { protected ValueRef getValueRef(ExpressionState state) throws EvaluationException { TypedValue context = state.getActiveContextObject(); Object target = context.getValue(); + + if (target == null) { + if (this.nullSafe) { + return ValueRef.NullValueRef.INSTANCE; + } + // Raise a proper exception in case of a null target + throw new SpelEvaluationException(getStartPosition(), SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE); + } + TypeDescriptor targetDescriptor = context.getTypeDescriptor(); TypedValue indexValue; Object index; @@ -159,14 +183,6 @@ public class Indexer extends SpelNodeImpl { } } - // Raise a proper exception in case of a null target - if (target == null) { - if (this.nullSafe) { - return ValueRef.NullValueRef.INSTANCE; - } - throw new SpelEvaluationException(getStartPosition(), SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE); - } - // At this point, we need a TypeDescriptor for a non-null target object Assert.state(targetDescriptor != null, "No type descriptor"); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java index c998c350f1a..16770056104 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java @@ -538,8 +538,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { else if (maybeEatBeanReference()) { return pop(); } - else if (maybeEatProjection(false) || maybeEatSelection(false) || - maybeEatIndexer(false)) { + else if (maybeEatProjection(false) || maybeEatSelection(false) || maybeEatIndexer(false)) { return pop(); } else if (maybeEatInlineListOrMap()) { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java index af57312a9b1..ab424b3b53a 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java @@ -26,7 +26,9 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.expression.EvaluationContext; @@ -35,6 +37,7 @@ import org.springframework.expression.PropertyAccessor; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.testresources.Person; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -376,18 +379,74 @@ class IndexingTests { assertThat(expression.getValue(this, String.class)).isEqualTo("apple"); } - @Test - void nullSafeIndex() { - ContextWithNullCollections testContext = new ContextWithNullCollections(); - StandardEvaluationContext context = new StandardEvaluationContext(testContext); - Expression expr = new SpelExpressionParser().parseRaw("nullList?.[4]"); - assertThat(expr.getValue(context)).isNull(); + @Nested + class NullSafeIndexTests { // gh-29847 + + private final RootContextWithIndexedProperties rootContext = new RootContextWithIndexedProperties(); + + private final StandardEvaluationContext context = new StandardEvaluationContext(rootContext); - expr = new SpelExpressionParser().parseRaw("nullArray?.[4]"); - assertThat(expr.getValue(context)).isNull(); + private final SpelExpressionParser parser = new SpelExpressionParser(); + + private Expression expression; + + @Test + void nullSafeIndexIntoArray() { + expression = parser.parseExpression("array?.[0]"); + assertThat(expression.getValue(context)).isNull(); + rootContext.array = new int[] {42}; + assertThat(expression.getValue(context)).isEqualTo(42); + } + + @Test + void nullSafeIndexIntoList() { + expression = parser.parseExpression("list?.[0]"); + assertThat(expression.getValue(context)).isNull(); + rootContext.list = List.of(42); + assertThat(expression.getValue(context)).isEqualTo(42); + } + + @Test + void nullSafeIndexIntoSet() { + expression = parser.parseExpression("set?.[0]"); + assertThat(expression.getValue(context)).isNull(); + rootContext.set = Set.of(42); + assertThat(expression.getValue(context)).isEqualTo(42); + } + + @Test + void nullSafeIndexIntoString() { + expression = parser.parseExpression("string?.[0]"); + assertThat(expression.getValue(context)).isNull(); + rootContext.string = "XYZ"; + assertThat(expression.getValue(context)).isEqualTo("X"); + } + + @Test + void nullSafeIndexIntoMap() { + expression = parser.parseExpression("map?.['enigma']"); + assertThat(expression.getValue(context)).isNull(); + rootContext.map = Map.of("enigma", 42); + assertThat(expression.getValue(context)).isEqualTo(42); + } + + @Test + void nullSafeIndexIntoObject() { + expression = parser.parseExpression("person?.['name']"); + assertThat(expression.getValue(context)).isNull(); + rootContext.person = new Person("Jane"); + assertThat(expression.getValue(context)).isEqualTo("Jane"); + } + + static class RootContextWithIndexedProperties { + public int[] array; + public List list; + public Set set; + public String string; + public Map map; + public Person person; + } - expr = new SpelExpressionParser().parseRaw("nullMap:?.[4]"); - assertThat(expr.getValue(context)).isNull(); } @@ -450,11 +509,4 @@ class IndexingTests { } - - static class ContextWithNullCollections { - public List nullList = null; - public String[] nullArray = null; - public Map nullMap = null; - } - } From d2bd0d57160624f0c5246faa665bb0b882fc431b Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 22 Mar 2024 17:41:14 +0100 Subject: [PATCH 3/5] Retain null-safe syntax in AST representation of SpEL indexers Prior to this commit, SpEL's CompoundExpression omitted the null-safe syntax in AST string representations of indexing operations. To address this, this commit implements isNullSafe() in Indexer. See gh-29847 --- .../org/springframework/expression/spel/ast/Indexer.java | 9 +++++++++ .../springframework/expression/spel/ParsingTests.java | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 59c4fdaf21a..2236868a6f9 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -127,6 +127,15 @@ public class Indexer extends SpelNodeImpl { } + /** + * Does this node represent a null-safe index operation? + * @since 6.2 + */ + @Override + public final boolean isNullSafe() { + return this.nullSafe; + } + @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { return getValueRef(state).getValue(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java index 4eebccab494..2a0ce84bbcb 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java @@ -60,8 +60,8 @@ class ParsingTests { parseCheck("property1?.property2?.methodOne()"); parseCheck("property1?.methodOne('enigma')?.methodTwo(42)"); parseCheck("property1?.methodOne()?.property2?.methodTwo()"); - parseCheck("property1[0]?.property2['key']?.methodTwo()"); - parseCheck("property1[0][1]?.property2['key'][42]?.methodTwo()"); + parseCheck("property1?.[0]?.property2?.['key']?.methodTwo()"); + parseCheck("property1?.[0]?.[1]?.property2?.['key']?.[42]?.methodTwo()"); } @Test From 38c473fd055bb9c1f02a49a58c29289a2e32e909 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 22 Mar 2024 17:44:37 +0100 Subject: [PATCH 4/5] Support compilation of null-safe index operations in SpEL See gh-29847 --- .../expression/spel/ast/Indexer.java | 58 ++++- .../spel/SpelCompilationCoverageTests.java | 203 ++++++++++++++++++ 2 files changed, 251 insertions(+), 10 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 2236868a6f9..033817bd944 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -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 convertValue(TypeConverter converter, @Nullable Object value, Class 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); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 72edfb42542..77da11dea14 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -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 list; + public Set set; + public String string; + public Map map; + public Person person; + } + } From 218a148898914955bbb8a0abbc190af51dea43c3 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sat, 23 Mar 2024 14:24:29 +0100 Subject: [PATCH 5/5] Document null-safe index operator in SpEL See gh-29847 --- .../operator-safe-navigation.adoc | 60 ++++++++++++++++++- .../language-ref/properties-arrays.adoc | 4 ++ .../expression/spel/ast/Indexer.java | 7 +++ .../spel/SpelDocumentationTests.java | 18 ++++++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/framework-docs/modules/ROOT/pages/core/expressions/language-ref/operator-safe-navigation.adoc b/framework-docs/modules/ROOT/pages/core/expressions/language-ref/operator-safe-navigation.adoc index d914a700296..d8367f70df8 100644 --- a/framework-docs/modules/ROOT/pages/core/expressions/language-ref/operator-safe-navigation.adoc +++ b/framework-docs/modules/ROOT/pages/core/expressions/language-ref/operator-safe-navigation.adoc @@ -1,7 +1,7 @@ [[expressions-operator-safe-navigation]] = Safe Navigation Operator -The safe navigation operator (`?`) is used to avoid a `NullPointerException` and comes +The safe navigation operator (`?.`) is used to avoid a `NullPointerException` and comes from the https://www.groovy-lang.org/operators.html#_safe_navigation_operator[Groovy] language. Typically, when you have a reference to an object, you might need to verify that it is not `null` before accessing methods or properties of the object. To avoid @@ -81,6 +81,64 @@ For example, the expression `#calculator?.max(4, 2)` evaluates to `null` if the `max(int, int)` method will be invoked on the `#calculator`. ==== +[[expressions-operator-safe-navigation-indexing]] +== Safe Index Access + +Since Spring Framework 6.2, the Spring Expression Language supports safe navigation for +indexing into the following types of structures. + +* xref:core/expressions/language-ref/properties-arrays.adoc#expressions-indexing-arrays-and-collections[arrays and collections] +* xref:core/expressions/language-ref/properties-arrays.adoc#expressions-indexing-strings[strings] +* xref:core/expressions/language-ref/properties-arrays.adoc#expressions-indexing-maps[maps] +* xref:core/expressions/language-ref/properties-arrays.adoc#expressions-indexing-objects[objects] + +The following example shows how to use the safe navigation operator for indexing into +a list (`?.[]`). + +[tabs] +====== +Java:: ++ +[source,java,indent=0,subs="verbatim,quotes",role="primary"] +---- + ExpressionParser parser = new SpelExpressionParser(); + IEEE society = new IEEE(); + EvaluationContext context = new StandardEvaluationContext(society); + + // evaluates to Inventor("Nikola Tesla") + Inventor inventor = parser.parseExpression("members?.[0]") // <1> + .getValue(context, Inventor.class); + + society.members = null; + + // evaluates to null - does not throw an exception + inventor = parser.parseExpression("members?.[0]") // <2> + .getValue(context, Inventor.class); +---- +<1> Use null-safe index operator on a non-null `members` list +<2> Use null-safe index operator on a null `members` list + +Kotlin:: ++ +[source,kotlin,indent=0,subs="verbatim,quotes",role="secondary"] +---- + val parser = SpelExpressionParser() + val society = IEEE() + val context = StandardEvaluationContext(society) + + // evaluates to Inventor("Nikola Tesla") + var inventor = parser.parseExpression("members?.[0]") // <1> + .getValue(context, Inventor::class.java) + + society.members = null + + // evaluates to null - does not throw an exception + inventor = parser.parseExpression("members?.[0]") // <2> + .getValue(context, Inventor::class.java) +---- +<1> Use null-safe index operator on a non-null `members` list +<2> Use null-safe index operator on a null `members` list +====== [[expressions-operator-safe-navigation-selection-and-projection]] == Safe Collection Selection and Projection diff --git a/framework-docs/modules/ROOT/pages/core/expressions/language-ref/properties-arrays.adoc b/framework-docs/modules/ROOT/pages/core/expressions/language-ref/properties-arrays.adoc index 8e8360d1da3..32e6fd54e0e 100644 --- a/framework-docs/modules/ROOT/pages/core/expressions/language-ref/properties-arrays.adoc +++ b/framework-docs/modules/ROOT/pages/core/expressions/language-ref/properties-arrays.adoc @@ -7,6 +7,10 @@ into various structures. NOTE: Numerical index values are zero-based, such as when accessing the n^th^ element of an array in Java. +TIP: See the xref:core/expressions/language-ref/operator-safe-navigation.adoc[Safe Navigation Operator] +section for details on how to navigate object graphs and index into various structures +using the null-safe operator. + [[expressions-property-navigation]] == Property Navigation diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 033817bd944..5ead18c349a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -58,6 +58,13 @@ import org.springframework.util.ReflectionUtils; *
  • Objects: the property with the specified name
    • * * + *

    Null-safe Indexing

    + * + *

    As of Spring Framework 6.2, null-safe indexing is supported via the {@code '?.'} + * operator. For example, {@code 'colors?.[0]'} will evaluate to {@code null} if + * {@code colors} is {@code null} and will otherwise evaluate to the 0th + * color. + * * @author Andy Clement * @author Phillip Webb * @author Stephane Nicoll diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java index 07c9bd3b79f..b512cf54518 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelDocumentationTests.java @@ -688,6 +688,24 @@ class SpelDocumentationTests extends AbstractExpressionTests { assertThat(city).isNull(); } + @Test + void nullSafeIndexing() { + IEEE society = new IEEE(); + EvaluationContext context = new StandardEvaluationContext(society); + + // evaluates to Inventor("Nikola Tesla") + Inventor inventor = parser.parseExpression("members?.[0]") // <1> + .getValue(context, Inventor.class); + assertThat(inventor).extracting(Inventor::getName).isEqualTo("Nikola Tesla"); + + society.members = null; + + // evaluates to null - does not throw an Exception + inventor = parser.parseExpression("members?.[0]") // <2> + .getValue(context, Inventor.class); + assertThat(inventor).isNull(); + } + @Test @SuppressWarnings("unchecked") void nullSafeSelection() {