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
This commit is contained in:
Sam Brannen 2024-02-16 18:26:17 +01:00
parent c1f0faade7
commit ab48ac36e9
4 changed files with 15 additions and 4 deletions

View File

@ -217,7 +217,9 @@ public class ExpressionState {
* name/value pair. * name/value pair.
* @param name the name of the local variable * @param name the name of the local variable
* @param value the value 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) { public void enterScope(String name, Object value) {
initVariableScopes().push(new VariableScope(name, value)); initVariableScopes().push(new VariableScope(name, value));
initScopeRootObjects().push(getActiveContextObject()); initScopeRootObjects().push(getActiveContextObject());
@ -228,7 +230,9 @@ public class ExpressionState {
* context object} and a new local variable scope containing the supplied * context object} and a new local variable scope containing the supplied
* name/value pairs. * name/value pairs.
* @param variables a map containing name/value pairs for local variables * @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<String, Object> variables) { public void enterScope(@Nullable Map<String, Object> variables) {
initVariableScopes().push(new VariableScope(variables)); initVariableScopes().push(new VariableScope(variables));
initScopeRootObjects().push(getActiveContextObject()); initScopeRootObjects().push(getActiveContextObject());
@ -246,7 +250,9 @@ public class ExpressionState {
* overwritten. * overwritten.
* @param name the name of the local variable * @param name the name of the local variable
* @param value the value 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) { public void setLocalVariable(String name, Object value) {
initVariableScopes().element().setVariable(name, value); initVariableScopes().element().setVariable(name, value);
} }
@ -256,7 +262,9 @@ public class ExpressionState {
* @param name the name of the local variable * @param name the name of the local variable
* @return the value of the local variable, or {@code null} if the variable * @return the value of the local variable, or {@code null} if the variable
* does not exist in the current scope * 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 @Nullable
public Object lookupLocalVariable(String name) { public Object lookupLocalVariable(String name) {
for (VariableScope scope : initVariableScopes()) { for (VariableScope scope : initVariableScopes()) {

View File

@ -93,7 +93,7 @@ public class Projection extends SpelNodeImpl {
for (Object element : data) { for (Object element : data) {
try { try {
state.pushActiveContextObject(new TypedValue(element)); state.pushActiveContextObject(new TypedValue(element));
state.enterScope("index", result.size()); state.enterScope();
Object value = this.children[0].getValueInternal(state).getValue(); Object value = this.children[0].getValueInternal(state).getValue();
if (value != null && operandIsArray) { if (value != null && operandIsArray) {
arrayElementType = determineCommonType(arrayElementType, value.getClass()); arrayElementType = determineCommonType(arrayElementType, value.getClass());

View File

@ -139,11 +139,10 @@ public class Selection extends SpelNodeImpl {
Arrays.asList(ObjectUtils.toObjectArray(operand))); Arrays.asList(ObjectUtils.toObjectArray(operand)));
List<Object> result = new ArrayList<>(); List<Object> result = new ArrayList<>();
int index = 0;
for (Object element : data) { for (Object element : data) {
try { try {
state.pushActiveContextObject(new TypedValue(element)); state.pushActiveContextObject(new TypedValue(element));
state.enterScope("index", index); state.enterScope();
Object val = selectionCriteria.getValueInternal(state).getValue(); Object val = selectionCriteria.getValueInternal(state).getValue();
if (val instanceof Boolean b) { if (val instanceof Boolean b) {
if (b) { if (b) {
@ -157,7 +156,6 @@ public class Selection extends SpelNodeImpl {
throw new SpelEvaluationException(selectionCriteria.getStartPosition(), throw new SpelEvaluationException(selectionCriteria.getStartPosition(),
SpelMessage.RESULT_OF_SELECTION_CRITERIA_IS_NOT_BOOLEAN); SpelMessage.RESULT_OF_SELECTION_CRITERIA_IS_NOT_BOOLEAN);
} }
index++;
} }
finally { finally {
state.exitScope(); state.exitScope();

View File

@ -56,6 +56,7 @@ class ExpressionStateTests extends AbstractExpressionTests {
} }
@Test @Test
@SuppressWarnings("removal")
void localVariables() { void localVariables() {
Object value = state.lookupLocalVariable("foo"); Object value = state.lookupLocalVariable("foo");
assertThat(value).isNull(); assertThat(value).isNull();
@ -86,6 +87,7 @@ class ExpressionStateTests extends AbstractExpressionTests {
} }
@Test @Test
@SuppressWarnings("removal")
void noVariableInterference() { void noVariableInterference() {
TypedValue typedValue = state.lookupVariable("foo"); TypedValue typedValue = state.lookupVariable("foo");
assertThat(typedValue).isEqualTo(TypedValue.NULL); assertThat(typedValue).isEqualTo(TypedValue.NULL);
@ -99,6 +101,7 @@ class ExpressionStateTests extends AbstractExpressionTests {
} }
@Test @Test
@SuppressWarnings("removal")
void localVariableNestedScopes() { void localVariableNestedScopes() {
assertThat(state.lookupLocalVariable("foo")).isNull(); assertThat(state.lookupLocalVariable("foo")).isNull();
@ -157,6 +160,7 @@ class ExpressionStateTests extends AbstractExpressionTests {
} }
@Test @Test
@SuppressWarnings("removal")
void populatedNestedScopes() { void populatedNestedScopes() {
assertThat(state.lookupLocalVariable("foo")).isNull(); assertThat(state.lookupLocalVariable("foo")).isNull();
@ -186,6 +190,7 @@ class ExpressionStateTests extends AbstractExpressionTests {
} }
@Test @Test
@SuppressWarnings("removal")
void populatedNestedScopesMap() { void populatedNestedScopesMap() {
assertThat(state.lookupLocalVariable("foo")).isNull(); assertThat(state.lookupLocalVariable("foo")).isNull();
assertThat(state.lookupLocalVariable("goo")).isNull(); assertThat(state.lookupLocalVariable("goo")).isNull();