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 6a29ba3694d..99e9a576869 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 @@ -76,7 +76,6 @@ public class ConstructorReference extends SpelNodeImpl { @Nullable 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. */ @Nullable private volatile ConstructorExecutor cachedExecutor; 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 e82e420d0a0..e0ca1054d1d 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 @@ -82,7 +82,7 @@ public class Projection extends SpelNodeImpl { state.exitScope(); } } - return new ValueRef.TypedValueHolderValueRef(new TypedValue(result), this); // TODO unable to build correct type descriptor + return new ValueRef.TypedValueHolderValueRef(new TypedValue(result), this); } boolean operandIsArray = ObjectUtils.isArray(operand); 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 e33b77e3b62..e26dca0d038 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 @@ -90,7 +90,6 @@ public class Selection extends SpelNodeImpl { SpelNodeImpl selectionCriteria = this.children[0]; if (operand instanceof Map mapdata) { - // TODO don't lose generic info for the new map Map result = new HashMap<>(); Object lastKey = null; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java index 3effcf1858e..a0a67d89af1 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/TypeReference.java @@ -54,7 +54,7 @@ public class TypeReference extends SpelNodeImpl { @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { - // TODO possible optimization here if we cache the discovered type reference, but can we do that? + // TODO Possible optimization: if we cache the discovered type reference, but can we do that? String typeName = (String) this.children[0].getValueInternal(state).getValue(); Assert.state(typeName != null, "No type name"); if (!typeName.contains(".") && Character.isLowerCase(typeName.charAt(0))) { @@ -99,7 +99,7 @@ public class TypeReference extends SpelNodeImpl { @Override public void generateCode(MethodVisitor mv, CodeFlow cf) { - // TODO Future optimization - if followed by a static method call, skip generating code here + // TODO Future optimization: if followed by a static method call, skip generating code here. Assert.state(this.type != null, "No type available"); if (this.type.isPrimitive()) { if (this.type == boolean.class) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java index 9f4482b36c4..886b2090287 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java @@ -733,7 +733,6 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { /** * Eat an identifier, possibly qualified (meaning that it is dotted). - * TODO AndyC Could create complete identifiers (a.b.c) here rather than a sequence of them? (a, b, c) */ private SpelNodeImpl eatPossiblyQualifiedId() { Deque qualifiedIdPieces = new ArrayDeque<>(); @@ -783,7 +782,6 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { // method reference push(new MethodReference(nullSafeNavigation, methodOrPropertyName.stringValue(), methodOrPropertyName.startPos, methodOrPropertyName.endPos, args)); - // TODO what is the end position for a method reference? the name or the last arg? return true; } return false; @@ -826,7 +824,6 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { else { // regular constructor invocation eatConstructorArgs(nodes); - // TODO correct end position? push(new ConstructorReference(newToken.startPos, newToken.endPos, nodes.toArray(new SpelNodeImpl[0]))); } return true; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index 4d2591c3b4d..375f9f6163c 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -233,7 +233,6 @@ public abstract class ReflectionHelper { return (match != null ? new ArgumentsMatchInfo(match) : null); } - // TODO could do with more refactoring around argument handling and varargs /** * Convert the supplied set of arguments into the parameter types specified * by the supplied {@link Method}. 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 9765586a840..5cd5064bb20 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 @@ -64,7 +64,7 @@ import static org.springframework.expression.spel.standard.SpelExpressionTestUti public class SpelCompilationCoverageTests extends AbstractExpressionTests { /* - * Further TODOs for compilation: + * TODO Potential optimizations for SpEL compilation: * * - OpMinus with a single literal operand could be treated as a negative literal. Will save a * pointless loading of 0 and then a subtract instruction in code gen. @@ -1205,12 +1205,12 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(expression); assertThat(expression.getValue(context, new SomeCompareMethod2()).toString()).isEqualTo("xyz"); - // TODO fails due to conversionservice handling of String[] to Object... - // expression = parser.parseExpression("#append2(#stringArray)"); - // assertEquals("xyz", expression.getValue(context).toString()); - // assertTrue(((SpelNodeImpl)((SpelExpression) expression).getAST()).isCompilable()); - // assertCanCompile(expression); - // assertEquals("xyz", expression.getValue(context).toString()); + // TODO Determine why the String[] is passed as the first element of the Object... varargs array instead of the entire varargs array. + // expression = parser.parseExpression("#append2(#stringArray)"); + // assertThat(expression.getValue(context)).hasToString("xyz"); + // assertThat(((SpelNodeImpl) ((SpelExpression) expression).getAST()).isCompilable()).isTrue(); + // assertCanCompile(expression); + // assertThat(expression.getValue(context)).hasToString("xyz"); expression = parser.parseExpression("#sum(1,2,3)"); assertThat(expression.getValue(context)).isEqualTo(6); @@ -3703,16 +3703,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(tc.s).isEqualTo("aaabbbccc"); tc.reset(); - // TODO Fails related to conversion service converting a String[] to satisfy Object... -// expression = parser.parseExpression("sixteen(stringArray)"); -// assertCantCompile(expression); -// expression.getValue(tc); -// assertEquals("aaabbbccc", tc.s); -// assertCanCompile(expression); -// tc.reset(); -// expression.getValue(tc); -// assertEquals("aaabbbccc", tc.s); -// tc.reset(); + // TODO Determine why the String[] is passed as the first element of the Object... varargs array instead of the entire varargs array. + // expression = parser.parseExpression("sixteen(stringArray)"); + // assertCantCompile(expression); + // expression.getValue(tc); + // assertThat(tc.s).isEqualTo("aaabbbccc"); + // assertCanCompile(expression); + // tc.reset(); + // expression.getValue(tc); + // assertThat(tc.s).isEqualTo("aaabbbccc"); + // tc.reset(); // varargs int expression = parser.parseExpression("twelve(1,2,3)");