From 52af5c2b3831b1c27608563dd5bd29b72ea08dab Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 8 Dec 2022 18:17:41 -0500 Subject: [PATCH] Support arrays in AST string representations of SpEL expressions Prior to this commit, SpEL's ConstructorReference did not provide support for arrays when generating a string representation of the internal AST. For example, 'new String[3]' was represented as 'new String()' instead of 'new String[3]'. This commit introduces support for standard array construction and array construction with initializers in ConstructorReference's toStringAST() implementation. Closes gh-29665 --- .../spel/ast/ConstructorReference.java | 50 +++++++++++---- .../expression/spel/ParsingTests.java | 64 ++++++++++++++----- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java index c9a68578ae0..8ea9f0c76f6 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java @@ -22,6 +22,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.List; +import java.util.StringJoiner; import org.springframework.asm.MethodVisitor; import org.springframework.core.convert.TypeDescriptor; @@ -46,10 +47,15 @@ import org.springframework.util.Assert; * Represents the invocation of a constructor. Either a constructor on a regular type or * construction of an array. When an array is constructed, an initializer can be specified. * - *

Examples:
- * new String('hello world')
- * new int[]{1,2,3,4}
- * new int[3] new int[3]{1,2,3} + *

Examples

+ * * * @author Andy Clement * @author Juergen Hoeller @@ -68,7 +74,7 @@ public class ConstructorReference extends SpelNodeImpl { private final boolean isArrayConstructor; @Nullable - private SpelNodeImpl[] dimensions; + private final SpelNodeImpl[] dimensions; // TODO is this caching safe - passing the expression around will mean this executor is also being passed around /** The cached executor that may be reused on subsequent evaluations. */ @@ -83,6 +89,7 @@ public class ConstructorReference extends SpelNodeImpl { public ConstructorReference(int startPos, int endPos, SpelNodeImpl... arguments) { super(startPos, endPos, arguments); this.isArrayConstructor = false; + this.dimensions = null; } /** @@ -214,16 +221,33 @@ public class ConstructorReference extends SpelNodeImpl { @Override public String toStringAST() { StringBuilder sb = new StringBuilder("new "); - int index = 0; - sb.append(getChild(index++).toStringAST()); - sb.append('('); - for (int i = index; i < getChildCount(); i++) { - if (i > index) { - sb.append(','); + sb.append(getChild(0).toStringAST()); // constructor or array type + + // Arrays + if (this.isArrayConstructor) { + if (hasInitializer()) { + // new int[] {1, 2, 3, 4, 5}, etc. + InlineList initializer = (InlineList) getChild(1); + sb.append("[] ").append(initializer.toStringAST()); + } + else { + // new int[3], new java.lang.String[3][4], etc. + for (SpelNodeImpl dimension : this.dimensions) { + sb.append('[').append(dimension.toStringAST()).append(']'); + } } - sb.append(getChild(i).toStringAST()); } - sb.append(')'); + // Constructors + else { + // new String('hello'), new org.example.Person('Jane', 32), etc. + StringJoiner sj = new StringJoiner(",", "(", ")"); + int count = getChildCount(); + for (int i = 1; i < count; i++) { + sj.add(getChild(i).toStringAST()); + } + sb.append(sj.toString()); + } + return sb.toString(); } 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 c0e85480fd1..652dcdc0022 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 @@ -67,37 +67,31 @@ class ParsingTests { parseCheck("#var1='value1'"); } - @Disabled("toStringAST() is broken for array construction") @Test void collectionProcessorsCountStringArray() { parseCheck("new String[] {'abc','def','xyz'}.count()"); } - @Disabled("toStringAST() is broken for array construction") @Test void collectionProcessorsCountIntArray() { parseCheck("new int[] {1,2,3}.count()"); } - @Disabled("toStringAST() is broken for array construction") @Test void collectionProcessorsMax() { parseCheck("new int[] {1,2,3}.max()"); } - @Disabled("toStringAST() is broken for array construction") @Test void collectionProcessorsMin() { parseCheck("new int[] {1,2,3}.min()"); } - @Disabled("toStringAST() is broken for array construction") @Test void collectionProcessorsAverage() { parseCheck("new int[] {1,2,3}.average()"); } - @Disabled("toStringAST() is broken for array construction") @Test void collectionProcessorsSort() { parseCheck("new int[] {3,2,1}.sort()"); @@ -444,32 +438,70 @@ class ParsingTests { @Test void methods() { + parseCheck("echo()"); parseCheck("echo(12)"); parseCheck("echo(name)"); + parseCheck("echo('Jane')"); + parseCheck("echo('Jane',32)"); + parseCheck("echo('Jane', 32)", "echo('Jane',32)"); parseCheck("age.doubleItAndAdd(12)"); } @Test - void constructors() { - parseCheck("new String('hello')"); + void constructorWithNoArguments() { + parseCheck("new Foo()"); + parseCheck("new example.Foo()"); } - @Disabled("toStringAST() is broken for array construction") @Test - void arrayConstruction01() { + void constructorWithOneArgument() { + parseCheck("new String('hello')"); + parseCheck("new String( 'hello' )", "new String('hello')"); + parseCheck("new String(\"hello\" )", "new String('hello')"); + } + + @Test + void constructorWithMultipleArguments() { + parseCheck("new example.Person('Jane',32,true)"); + parseCheck("new example.Person('Jane', 32, true)", "new example.Person('Jane',32,true)"); + parseCheck("new example.Person('Jane', 2 * 16, true)", "new example.Person('Jane',(2 * 16),true)"); + } + + @Test + void arrayConstructionWithOneDimensionalReferenceType() { parseCheck("new String[3]"); } - @Disabled("toStringAST() is broken for array construction") @Test - void arrayConstruction02() { - parseCheck("new int[] {1, 2, 3, 4, 5}", "new int[] {1,2,3,4,5}"); + void arrayConstructionWithOneDimensionalFullyQualifiedReferenceType() { + parseCheck("new java.lang.String[3]"); } - @Disabled("toStringAST() is broken for array construction") @Test - void arrayConstruction03() { - parseCheck("new String[] {'abc','xyz'}", "new String[] {'abc','xyz'}"); + void arrayConstructionWithOneDimensionalPrimitiveType() { + parseCheck("new int[3]"); + } + + @Test + void arrayConstructionWithMultiDimensionalReferenceType() { + parseCheck("new Float[3][4]"); + } + + @Test + void arrayConstructionWithMultiDimensionalPrimitiveType() { + parseCheck("new int[3][4]"); + } + + @Test + void arrayConstructionWithOneDimensionalReferenceTypeWithInitializer() { + parseCheck("new String[] {'abc','xyz'}"); + parseCheck("new String[] {'abc', 'xyz'}", "new String[] {'abc','xyz'}"); + } + + @Test + void arrayConstructionWithOneDimensionalPrimitiveTypeWithInitializer() { + parseCheck("new int[] {1,2,3,4,5}"); + parseCheck("new int[] {1, 2, 3, 4, 5}", "new int[] {1,2,3,4,5}"); } }