diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java index 618cbda2a41..d9d3bbcbbfa 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java @@ -957,7 +957,7 @@ public class CodeFlow implements Opcodes { */ public static void insertOptimalLoad(MethodVisitor mv, int value) { if (value < 6) { - mv.visitInsn(ICONST_0+value); + mv.visitInsn(ICONST_0 + value); } else if (value < Byte.MAX_VALUE) { mv.visitIntInsn(BIPUSH, value); @@ -971,15 +971,16 @@ public class CodeFlow implements Opcodes { } /** - * Produce appropriate bytecode to store a stack item in an array. The - * instruction to use varies depending on whether the type - * is a primitive or reference type. + * Produce appropriate bytecode to store a stack item in an array. + *

The instruction to use varies depending on whether the type is a + * primitive or reference type. * @param mv where to insert the bytecode - * @param arrayElementType the type of the array elements + * @param arrayComponentType the component type of the array */ - public static void insertArrayStore(MethodVisitor mv, String arrayElementType) { - if (arrayElementType.length() == 1) { - switch (arrayElementType.charAt(0)) { + public static void insertArrayStore(MethodVisitor mv, String arrayComponentType) { + if (arrayComponentType.length() == 1) { + char componentType = arrayComponentType.charAt(0); + switch (componentType) { case 'B', 'Z' -> mv.visitInsn(BASTORE); case 'I' -> mv.visitInsn(IASTORE); case 'J' -> mv.visitInsn(LASTORE); @@ -987,7 +988,7 @@ public class CodeFlow implements Opcodes { case 'D' -> mv.visitInsn(DASTORE); case 'C' -> mv.visitInsn(CASTORE); case 'S' -> mv.visitInsn(SASTORE); - default -> throw new IllegalArgumentException("Unexpected array type " + arrayElementType.charAt(0)); + default -> throw new IllegalArgumentException("Unexpected array component type " + componentType); } } else { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java index bc71a1c243c..9d70815776a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java @@ -211,18 +211,22 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { /** - * Generate code that handles building the argument values for the specified method. - *

This method will take into account whether the invoked method is a varargs method, - * and if it is then the argument values will be appropriately packaged into an array. + * Generate code that handles building the argument values for the specified + * {@link Member} (method or constructor). + *

This method takes into account whether the method or constructor was + * declared to accept varargs, and if it was then the argument values will be + * appropriately packaged into an array. * @param mv the method visitor where code should be generated - * @param cf the current codeflow + * @param cf the current {@link CodeFlow} * @param member the method or constructor for which arguments are being set up * @param arguments the expression nodes for the expression supplied argument values * @deprecated As of Spring Framework 6.2, in favor of * {@link #generateCodeForArguments(MethodVisitor, CodeFlow, Executable, SpelNodeImpl[])} */ @Deprecated(since = "6.2") - protected static void generateCodeForArguments(MethodVisitor mv, CodeFlow cf, Member member, SpelNodeImpl[] arguments) { + protected static void generateCodeForArguments( + MethodVisitor mv, CodeFlow cf, Member member, SpelNodeImpl[] arguments) { + if (member instanceof Executable executable) { generateCodeForArguments(mv, cf, executable, arguments); } @@ -233,7 +237,7 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { /** * Generate code that handles building the argument values for the specified * {@link Executable} (method or constructor). - *

This method takes into account whether the invoked executable was + *

This method takes into account whether the method or constructor was * declared to accept varargs, and if it was then the argument values will be * appropriately packaged into an array. * @param mv the method visitor where code should be generated @@ -244,52 +248,56 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { * values * @since 6.2 */ - protected static void generateCodeForArguments(MethodVisitor mv, CodeFlow cf, Executable executable, SpelNodeImpl[] arguments) { + protected static void generateCodeForArguments( + MethodVisitor mv, CodeFlow cf, Executable executable, SpelNodeImpl[] arguments) { + Class[] parameterTypes = executable.getParameterTypes(); - String[] paramDescriptors = CodeFlow.toDescriptors(parameterTypes); + String[] parameterDescriptors = CodeFlow.toDescriptors(parameterTypes); + int parameterCount = parameterTypes.length; if (executable.isVarArgs()) { // The final parameter may or may not need packaging into an array, or nothing may - // have been passed to satisfy the varargs and so something needs to be built. - int p = 0; // Current supplied argument being processed - int childCount = arguments.length; + // have been passed to satisfy the varargs which means something needs to be built. + + int varargsIndex = parameterCount - 1; + int argumentCount = arguments.length; + int p = 0; // Current supplied argument being processed - // Fulfill all the parameter requirements except the last one - for (p = 0; p < paramDescriptors.length - 1; p++) { - cf.generateCodeForArgument(mv, arguments[p], paramDescriptors[p]); + // Fulfill all the parameter requirements except the last one (the varargs array). + for (p = 0; p < varargsIndex; p++) { + cf.generateCodeForArgument(mv, arguments[p], parameterDescriptors[p]); } - SpelNodeImpl lastChild = (childCount == 0 ? null : arguments[childCount - 1]); + SpelNodeImpl lastArgument = (argumentCount != 0 ? arguments[argumentCount - 1] : null); ClassLoader classLoader = executable.getDeclaringClass().getClassLoader(); - Class lastChildType = (lastChild != null ? - loadClassForExitDescriptor(lastChild.getExitDescriptor(), classLoader) : null); - Class lastParameterType = parameterTypes[parameterTypes.length - 1]; + Class lastArgumentType = (lastArgument != null ? + loadClassForExitDescriptor(lastArgument.getExitDescriptor(), classLoader) : null); + Class lastParameterType = parameterTypes[varargsIndex]; // Determine if the final passed argument is already suitably packaged in array - // form to be passed to the method - if (lastChild != null && lastChildType != null && lastParameterType.isAssignableFrom(lastChildType)) { - cf.generateCodeForArgument(mv, lastChild, paramDescriptors[p]); + // form to be passed to the method. + if (lastArgument != null && lastArgumentType != null && lastParameterType.isAssignableFrom(lastArgumentType)) { + cf.generateCodeForArgument(mv, lastArgument, parameterDescriptors[p]); } else { - String arrayType = paramDescriptors[paramDescriptors.length - 1]; - arrayType = arrayType.substring(1); // trim the leading '[', may leave other '[' - // build array big enough to hold remaining arguments - CodeFlow.insertNewArrayCode(mv, childCount - p, arrayType); - // Package up the remaining arguments into the array - int arrayindex = 0; - while (p < childCount) { - SpelNodeImpl child = arguments[p]; + String arrayComponentType = parameterDescriptors[varargsIndex]; + // Trim the leading '[', potentially leaving other '[' characters. + arrayComponentType = arrayComponentType.substring(1); + // Build array big enough to hold remaining arguments. + CodeFlow.insertNewArrayCode(mv, argumentCount - p, arrayComponentType); + // Package up the remaining arguments into the array. + int arrayIndex = 0; + while (p < argumentCount) { mv.visitInsn(DUP); - CodeFlow.insertOptimalLoad(mv, arrayindex++); - cf.generateCodeForArgument(mv, child, arrayType); - CodeFlow.insertArrayStore(mv, arrayType); - p++; + CodeFlow.insertOptimalLoad(mv, arrayIndex++); + cf.generateCodeForArgument(mv, arguments[p++], arrayComponentType); + CodeFlow.insertArrayStore(mv, arrayComponentType); } } } else { - for (int i = 0; i < paramDescriptors.length;i++) { - cf.generateCodeForArgument(mv, arguments[i], paramDescriptors[i]); + for (int i = 0; i < parameterCount; i++) { + cf.generateCodeForArgument(mv, arguments[i], parameterDescriptors[i]); } } } @@ -308,8 +316,10 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { } /** - * Ask an argument to generate its bytecode and then follow it up - * with any boxing/unboxing/checkcasting to ensure it matches the expected parameter descriptor. + * Generate bytecode that loads the supplied argument onto the stack. + *

This method also performs any boxing, unboxing, or check-casting + * necessary to ensure that the type of the argument on the stack matches the + * supplied {@code paramDesc}. * @deprecated As of Spring Framework 6.2, in favor of * {@link CodeFlow#generateCodeForArgument(MethodVisitor, SpelNode, String)} */ 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 688d517334a..f8e8096b721 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 @@ -2642,14 +2642,12 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(expression); assertThat(expression.getValue(context)).isEqualTo(20); - expression = parser.parseExpression("#appendChar('abc'.charAt(0),'abc'.charAt(1))"); assertThat(expression.getValue(context)).isEqualTo("ab"); assertThat(((SpelExpression) expression).getAST().isCompilable()).isTrue(); assertCanCompile(expression); assertThat(expression.getValue(context)).isEqualTo("ab"); - expression = parser.parseExpression("#append4('a','b','c')"); assertThat(expression.getValue(context).toString()).isEqualTo("a::bc"); assertThat(((SpelExpression) expression).getAST().isCompilable()).isTrue(); @@ -3259,7 +3257,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(b).isTrue(); assertThat(aa.gotComparedTo).isEqualTo(bb); - List ls = new ArrayList<>(); ls.add("foo"); StandardEvaluationContext context = new StandardEvaluationContext(ls); @@ -4682,7 +4679,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(expression.getValue(Boolean.class)).isNull(); } - /** * Test variants of using T(...) and static/non-static method/property/field references. */ @@ -4775,7 +4771,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(expression.getValue(StaticsHelper.sh)).isEqualTo("mb"); assertCanCompile(expression); assertThat(expression.getValue(StaticsHelper.sh)).isEqualTo("mb"); - } @Test @@ -4863,86 +4858,89 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { context = new StandardEvaluationContext(new Object[] {person.getAge()}); context.setVariable("it", person); assertThat(ex.getValue(context, Boolean.class)).isTrue(); - assertThat(ex.getValue(context, Boolean.class)).isTrue(); PersonInOtherPackage person2 = new PersonInOtherPackage(1); ex = parser.parseRaw("#it?.age.equals([0])"); context = new StandardEvaluationContext(new Object[] {person2.getAge()}); context.setVariable("it", person2); assertThat(ex.getValue(context, Boolean.class)).isTrue(); - assertThat(ex.getValue(context, Boolean.class)).isTrue(); ex = parser.parseRaw("#it?.age.equals([0])"); context = new StandardEvaluationContext(new Object[] {person2.getAge()}); context.setVariable("it", person2); assertThat((Boolean) ex.getValue(context)).isTrue(); - assertThat((Boolean) ex.getValue(context)).isTrue(); } @Test void constructorReference() { - // simple ctor + // simple constructor expression = parser.parseExpression("new String('123')"); assertThat(expression.getValue()).isEqualTo("123"); assertCanCompile(expression); assertThat(expression.getValue()).isEqualTo("123"); - String testclass8 = "org.springframework.expression.spel.SpelCompilationCoverageTests$TestClass8"; - // multi arg ctor that includes primitives + String testclass8 = TestClass8.class.getName(); + Object result; + + // multi arg constructor that includes primitives expression = parser.parseExpression("new " + testclass8 + "(42,'123',4.0d,true)"); - assertThat(expression.getValue().getClass().getName()).isEqualTo(testclass8); + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass8.class); assertCanCompile(expression); - Object o = expression.getValue(); - assertThat(o.getClass().getName()).isEqualTo(testclass8); - TestClass8 tc8 = (TestClass8) o; + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass8.class); + TestClass8 tc8 = (TestClass8) result; assertThat(tc8.i).isEqualTo(42); assertThat(tc8.s).isEqualTo("123"); assertThat(tc8.d).isCloseTo(4.0d, within(0.5d)); assertThat(tc8.z).isTrue(); - // no-arg ctor + // no-arg constructor expression = parser.parseExpression("new " + testclass8 + "()"); - assertThat(expression.getValue().getClass().getName()).isEqualTo(testclass8); + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass8.class); assertCanCompile(expression); - o = expression.getValue(); - assertThat(o.getClass().getName()).isEqualTo(testclass8); + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass8.class); - // pass primitive to reference type ctor + // pass primitive to reference type constructor expression = parser.parseExpression("new " + testclass8 + "(42)"); - assertThat(expression.getValue().getClass().getName()).isEqualTo(testclass8); + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass8.class); assertCanCompile(expression); - o = expression.getValue(); - assertThat(o.getClass().getName()).isEqualTo(testclass8); - tc8 = (TestClass8) o; + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass8.class); + tc8 = (TestClass8) result; assertThat(tc8.i).isEqualTo(42); // varargs expression = parser.parseExpression("new " + testclass8 + "(#root)"); Object[] objectArray = { "a", "b", "c" }; - o = expression.getValue(objectArray); - assertThat(o).isExactlyInstanceOf(TestClass8.class); + result = expression.getValue(objectArray); + assertThat(result).isExactlyInstanceOf(TestClass8.class); assertCanCompile(expression); - o = expression.getValue(objectArray); - assertThat(o).isExactlyInstanceOf(TestClass8.class); - tc8 = (TestClass8) o; + result = expression.getValue(objectArray); + assertThat(result).isExactlyInstanceOf(TestClass8.class); + tc8 = (TestClass8) result; assertThat(tc8.args).containsExactly("a", "b", "c"); // varargs with argument component type that is a subtype of the varargs component type. expression = parser.parseExpression("new " + testclass8 + "(#root)"); String[] stringArray = { "a", "b", "c" }; - o = expression.getValue(stringArray); - assertThat(o).isExactlyInstanceOf(TestClass8.class); + result = expression.getValue(stringArray); + assertThat(result).isExactlyInstanceOf(TestClass8.class); assertCanCompile(expression); - o = expression.getValue(stringArray); - assertThat(o).isExactlyInstanceOf(TestClass8.class); - tc8 = (TestClass8) o; + result = expression.getValue(stringArray); + assertThat(result).isExactlyInstanceOf(TestClass8.class); + tc8 = (TestClass8) result; assertThat(tc8.args).containsExactly("a", "b", "c"); // private class, can't compile it - String testclass9 = "org.springframework.expression.spel.SpelCompilationCoverageTests$TestClass9"; + String testclass9 = TestClass9.class.getName(); expression = parser.parseExpression("new " + testclass9 + "(42)"); - assertThat(expression.getValue().getClass().getName()).isEqualTo(testclass9); + result = expression.getValue(); + assertThat(result).isExactlyInstanceOf(TestClass9.class); assertCannotCompile(expression); }