From ab48ac36e9adef1b40d42484f0620dea782b9eac Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 16 Feb 2024 18:26:17 +0100 Subject: [PATCH] Deprecate local variable support in SpEL's internal ExpressionState Since the Spring Expression Language does not actually support local variables in expressions, this commit deprecates all public APIs related to local variables in ExpressionState (namely, the two enterScope(...) variants that accept local variable data, setLocalVariable(), and lookupLocalVariable()). In addition, we no longer invoke `state.enterScope("index", ...)` in the Projection and Selection AST nodes since the $index local variable was never accessible within expressions anyway. See gh-23202 Closes gh-32004 --- .../springframework/expression/spel/ExpressionState.java | 8 ++++++++ .../springframework/expression/spel/ast/Projection.java | 2 +- .../springframework/expression/spel/ast/Selection.java | 4 +--- .../expression/spel/ExpressionStateTests.java | 5 +++++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java b/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java index 15797db806d..32247c95b94 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ExpressionState.java @@ -217,7 +217,9 @@ public class ExpressionState { * name/value pair. * @param name the name of the local variable * @param value the value of the local variable + * @deprecated as of 6.2 with no replacement; to be removed in 7.0 */ + @Deprecated(since = "6.2", forRemoval = true) public void enterScope(String name, Object value) { initVariableScopes().push(new VariableScope(name, value)); initScopeRootObjects().push(getActiveContextObject()); @@ -228,7 +230,9 @@ public class ExpressionState { * context object} and a new local variable scope containing the supplied * name/value pairs. * @param variables a map containing name/value pairs for local variables + * @deprecated as of 6.2 with no replacement; to be removed in 7.0 */ + @Deprecated(since = "6.2", forRemoval = true) public void enterScope(@Nullable Map variables) { initVariableScopes().push(new VariableScope(variables)); initScopeRootObjects().push(getActiveContextObject()); @@ -246,7 +250,9 @@ public class ExpressionState { * overwritten. * @param name the name of the local variable * @param value the value of the local variable + * @deprecated as of 6.2 with no replacement; to be removed in 7.0 */ + @Deprecated(since = "6.2", forRemoval = true) public void setLocalVariable(String name, Object value) { initVariableScopes().element().setVariable(name, value); } @@ -256,7 +262,9 @@ public class ExpressionState { * @param name the name of the local variable * @return the value of the local variable, or {@code null} if the variable * does not exist in the current scope + * @deprecated as of 6.2 with no replacement; to be removed in 7.0 */ + @Deprecated(since = "6.2", forRemoval = true) @Nullable public Object lookupLocalVariable(String name) { for (VariableScope scope : initVariableScopes()) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java index 4c87dd05203..b81e050284c 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java @@ -93,7 +93,7 @@ public class Projection extends SpelNodeImpl { for (Object element : data) { try { state.pushActiveContextObject(new TypedValue(element)); - state.enterScope("index", result.size()); + state.enterScope(); Object value = this.children[0].getValueInternal(state).getValue(); if (value != null && operandIsArray) { arrayElementType = determineCommonType(arrayElementType, value.getClass()); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java index 374a9da4d93..ce8910cc01a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Selection.java @@ -139,11 +139,10 @@ public class Selection extends SpelNodeImpl { Arrays.asList(ObjectUtils.toObjectArray(operand))); List result = new ArrayList<>(); - int index = 0; for (Object element : data) { try { state.pushActiveContextObject(new TypedValue(element)); - state.enterScope("index", index); + state.enterScope(); Object val = selectionCriteria.getValueInternal(state).getValue(); if (val instanceof Boolean b) { if (b) { @@ -157,7 +156,6 @@ public class Selection extends SpelNodeImpl { throw new SpelEvaluationException(selectionCriteria.getStartPosition(), SpelMessage.RESULT_OF_SELECTION_CRITERIA_IS_NOT_BOOLEAN); } - index++; } finally { state.exitScope(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java index c81ca08446d..f827eab9e9c 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java @@ -56,6 +56,7 @@ class ExpressionStateTests extends AbstractExpressionTests { } @Test + @SuppressWarnings("removal") void localVariables() { Object value = state.lookupLocalVariable("foo"); assertThat(value).isNull(); @@ -86,6 +87,7 @@ class ExpressionStateTests extends AbstractExpressionTests { } @Test + @SuppressWarnings("removal") void noVariableInterference() { TypedValue typedValue = state.lookupVariable("foo"); assertThat(typedValue).isEqualTo(TypedValue.NULL); @@ -99,6 +101,7 @@ class ExpressionStateTests extends AbstractExpressionTests { } @Test + @SuppressWarnings("removal") void localVariableNestedScopes() { assertThat(state.lookupLocalVariable("foo")).isNull(); @@ -157,6 +160,7 @@ class ExpressionStateTests extends AbstractExpressionTests { } @Test + @SuppressWarnings("removal") void populatedNestedScopes() { assertThat(state.lookupLocalVariable("foo")).isNull(); @@ -186,6 +190,7 @@ class ExpressionStateTests extends AbstractExpressionTests { } @Test + @SuppressWarnings("removal") void populatedNestedScopesMap() { assertThat(state.lookupLocalVariable("foo")).isNull(); assertThat(state.lookupLocalVariable("goo")).isNull();