Skip to content

Commit

Permalink
Deprecate local variable support in SpEL's internal ExpressionState
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed Feb 16, 2024
1 parent c1f0faa commit ab48ac3
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ public void enterScope() {
* 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());
Expand All @@ -228,7 +230,9 @@ public void enterScope(String name, Object value) {
* 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<String, Object> variables) {
initVariableScopes().push(new VariableScope(variables));
initScopeRootObjects().push(getActiveContextObject());
Expand All @@ -246,7 +250,9 @@ public void exitScope() {
* 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);
}
Expand All @@ -256,7 +262,9 @@ public void setLocalVariable(String name, Object value) {
* @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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected ValueRef getValueRef(ExpressionState state) throws EvaluationException
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,10 @@ protected ValueRef getValueRef(ExpressionState state) throws EvaluationException
Arrays.asList(ObjectUtils.toObjectArray(operand)));

List<Object> 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) {
Expand All @@ -157,7 +156,6 @@ protected ValueRef getValueRef(ExpressionState state) throws EvaluationException
throw new SpelEvaluationException(selectionCriteria.getStartPosition(),
SpelMessage.RESULT_OF_SELECTION_CRITERIA_IS_NOT_BOOLEAN);
}
index++;
}
finally {
state.exitScope();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ void construction() {
}

@Test
@SuppressWarnings("removal")
void localVariables() {
Object value = state.lookupLocalVariable("foo");
assertThat(value).isNull();
Expand Down Expand Up @@ -86,6 +87,7 @@ void globalVariables() {
}

@Test
@SuppressWarnings("removal")
void noVariableInterference() {
TypedValue typedValue = state.lookupVariable("foo");
assertThat(typedValue).isEqualTo(TypedValue.NULL);
Expand All @@ -99,6 +101,7 @@ void noVariableInterference() {
}

@Test
@SuppressWarnings("removal")
void localVariableNestedScopes() {
assertThat(state.lookupLocalVariable("foo")).isNull();

Expand Down Expand Up @@ -157,6 +160,7 @@ void activeContextObject() {
}

@Test
@SuppressWarnings("removal")
void populatedNestedScopes() {
assertThat(state.lookupLocalVariable("foo")).isNull();

Expand Down Expand Up @@ -186,6 +190,7 @@ void rootObjectConstructor() {
}

@Test
@SuppressWarnings("removal")
void populatedNestedScopesMap() {
assertThat(state.lookupLocalVariable("foo")).isNull();
assertThat(state.lookupLocalVariable("goo")).isNull();
Expand Down

0 comments on commit ab48ac3

Please sign in to comment.