Polishing

spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,7 +43,7 @@ import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
- * Represents the invocation of a constructor. Either a constructor on a regular type or
+ * 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.
  *
  * <h4>Examples</h4>
@@ -83,8 +83,9 @@ public class ConstructorReference extends SpelNodeImpl {
 
 
 	/**
-	 * Create a constructor reference. The first argument is the type, the rest are the parameters to the constructor
-	 * call
+	 * Create a constructor reference for a regular type.
+	 * <p>The first argument is the type. The rest are the arguments to the
+	 * constructor.
 	 */
 	public ConstructorReference(int startPos, int endPos, SpelNodeImpl... arguments) {
 		super(startPos, endPos, arguments);
@@ -93,8 +94,10 @@ public class ConstructorReference extends SpelNodeImpl {
 	}
 
 	/**
-	 * Create a constructor reference. The first argument is the type, the rest are the parameters to the constructor
-	 * call
+	 * Create a constructor reference for an array.
+	 * <p>The first argument is the array component type. The second argument is
+	 * an {@link InlineList} representing the array initializer, if an initializer
+	 * was supplied in the expression.
 	 */
 	public ConstructorReference(int startPos, int endPos, SpelNodeImpl[] dimensions, SpelNodeImpl... arguments) {
 		super(startPos, endPos, arguments);
@@ -139,11 +142,11 @@ public class ConstructorReference extends SpelNodeImpl {
 			}
 			catch (AccessException ex) {
 				// Two reasons this can occur:
-				// 1. the method invoked actually threw a real exception
-				// 2. the method invoked was not passed the arguments it expected and has become 'stale'
+				// 1. the constructor invoked actually threw a real exception
+				// 2. the constructor invoked was not passed the arguments it expected and has become 'stale'
 
 				// In the first case we should not retry, in the second case we should see if there is a
-				// better suited method.
+				// better suited constructor.
 
 				// To determine which situation it is, the AccessException will contain a cause.
 				// If the cause is an InvocationTargetException, a user exception was thrown inside the constructor.
@@ -167,7 +170,7 @@ public class ConstructorReference extends SpelNodeImpl {
 			}
 		}
 
-		// Either there was no accessor or it no longer exists
+		// Either there was no ConstructorExecutor or it no longer exists
 		String typeName = (String) this.children[0].getValueInternal(state).getValue();
 		Assert.state(typeName != null, "No type name");
 		executorToUse = findExecutorForConstructor(typeName, argumentTypes, state);
@@ -317,8 +320,8 @@ public class ConstructorReference extends SpelNodeImpl {
 		else {
 			// There is an initializer
 			if (this.dimensions == null || this.dimensions.length > 1) {
-				// There is an initializer but this is a multidimensional array (e.g. new int[][]{{1,2},{3,4}})
-				// - this is not currently supported
+				// There is an initializer, but this is a multidimensional array
+				// (e.g. new int[][]{{1,2},{3,4}}), which is not supported.
 				throw new SpelEvaluationException(getStartPosition(),
 						SpelMessage.MULTIDIM_ARRAY_INITIALIZER_NOT_SUPPORTED);
 			}
@@ -450,11 +453,9 @@ public class ConstructorReference extends SpelNodeImpl {
 			return false;
 		}
 
-		if (getChildCount() > 1) {
-			for (int c = 1, max = getChildCount(); c < max; c++) {
-				if (!this.children[c].isCompilable()) {
-					return false;
-				}
+		for (int i = 1; i < this.children.length; i++) {
+			if (!this.children[i].isCompilable()) {
+				return false;
 			}
 		}
 

spring-expression/src/main/java/org/springframework/expression/spel/ast/Projection.java

@@ -65,10 +65,9 @@ public class Projection extends SpelNodeImpl {
 		TypedValue op = state.getActiveContextObject();
 		Object operand = op.getValue();
 
-		// When the input is a map, we push a special context object on the stack
-		// before calling the specified operation. This special context object
-		// has two fields 'key' and 'value' that refer to the map entry's key
-		// and value, and they can be referenced in the operation -- for example,
+		// When the input is a map, we push a Map.Entry on the stack before calling
+		// the specified operation. Map.Entry has two properties 'key' and 'value'
+		// that can be referenced in the operation -- for example,
 		// {'a':'y', 'b':'n'}.![value == 'y' ? key : null] evaluates to ['a', null].
 		if (operand instanceof Map<?, ?> mapData) {
 			List<Object> result = new ArrayList<>();

spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -811,6 +811,8 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
 						dimensions.add(eatExpression());
 					}
 					else {
+						// A missing array dimension is tracked as null and will be
+						// rejected later during evaluation.
 						dimensions.add(null);
 					}
 					eatToken(TokenKind.RSQUARE);

spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java

@@ -38,7 +38,7 @@ import org.springframework.util.MethodInvoker;
 
 /**
  * Utility methods used by the reflection resolver code to discover the appropriate
- * methods/constructors and fields that should be used in expressions.
+ * methods, constructors, and fields that should be used in expressions.
  *
  * @author Andy Clement
  * @author Juergen Hoeller
@@ -49,7 +49,7 @@ public abstract class ReflectionHelper {
 
 	/**
 	 * Compare argument arrays and return information about whether they match.
-	 * A supplied type converter and conversionAllowed flag allow for matches to take
+	 * <p>A supplied type converter and conversionAllowed flag allow for matches to take
 	 * into account that a type may be transformed into a different type by the converter.
 	 * @param expectedArgTypes the types the method/constructor is expecting
 	 * @param suppliedArgTypes the types that are being supplied at the point of invocation
@@ -68,7 +68,7 @@ public abstract class ReflectionHelper {
 		for (int i = 0; i < expectedArgTypes.size() && match != null; i++) {
 			TypeDescriptor suppliedArg = suppliedArgTypes.get(i);
 			TypeDescriptor expectedArg = expectedArgTypes.get(i);
-			// The user may supply null - and that will be ok unless a primitive is expected
+			// The user may supply null, and that will be OK unless a primitive is expected.
 			if (suppliedArg == null) {
 				if (expectedArg.isPrimitive()) {
 					match = null;
@@ -136,9 +136,9 @@ public abstract class ReflectionHelper {
 
 	/**
 	 * Compare argument arrays and return information about whether they match.
-	 * A supplied type converter and conversionAllowed flag allow for matches to
+	 * <p>A supplied type converter and conversionAllowed flag allow for matches to
 	 * take into account that a type may be transformed into a different type by the
-	 * converter. This variant of compareArguments also allows for a varargs match.
+	 * converter. This variant of {@link #compareArguments} also allows for a varargs match.
 	 * @param expectedArgTypes the types the method/constructor is expecting
 	 * @param suppliedArgTypes the types that are being supplied at the point of invocation
 	 * @param typeConverter a registered type converter
@@ -233,19 +233,26 @@ public abstract class ReflectionHelper {
 		return (match != null ? new ArgumentsMatchInfo(match) : null);
 	}
 
-
 	// TODO could do with more refactoring around argument handling and varargs
 	/**
-	 * Convert a supplied set of arguments into the requested types. If the parameterTypes are related to
-	 * a varargs method then the final entry in the parameterTypes array is going to be an array itself whose
-	 * component type should be used as the conversion target for extraneous arguments. (For example, if the
-	 * parameterTypes are {Integer, String[]} and the input arguments are {Integer, boolean, float} then both
-	 * the boolean and float must be converted to strings). This method does *not* repackage the arguments
-	 * into a form suitable for the varargs invocation - a subsequent call to setupArgumentsForVarargsInvocation handles that.
+	 * Convert the supplied set of arguments into the parameter types specified
+	 * by the supplied {@link Method}.
+	 * <p>The arguments are converted 'in-place' in the input array.
+	 * <p>If the method accepts varargs, the final entry in its parameterTypes
+	 * array is going to be an array itself whose component type will be used as
+	 * the conversion target for any additional arguments. For example, if the
+	 * parameterTypes are {Integer, String[]} and the input arguments are
+	 * {Integer, boolean, float}, then both the boolean and float must be converted
+	 * to strings.
+	 * <p>This method does <strong>not</strong> repackage the arguments into a
+	 * form suitable for the varargs invocation. A subsequent call to
+	 * {@link #setupArgumentsForVarargsInvocation(Class[], Object...)} must be
+	 * used for that.
 	 * @param converter the converter to use for type conversions
-	 * @param arguments the arguments to convert to the requested parameter types
-	 * @param method the target Method
-	 * @return true if some kind of conversion occurred on the argument
+	 * @param arguments the arguments to convert to the parameter types of the
+	 * target method
+	 * @param method the target method
+	 * @return true if some kind of conversion occurred on an argument
 	 * @throws SpelEvaluationException if there is a problem with conversion
 	 */
 	public static boolean convertAllArguments(TypeConverter converter, Object[] arguments, Method method)
@@ -256,8 +263,9 @@ public abstract class ReflectionHelper {
 	}
 
 	/**
-	 * Takes an input set of argument values and converts them to the types specified as the
-	 * required parameter types. The arguments are converted 'in-place' in the input array.
+	 * Takes an input set of argument values and converts them to the parameter
+	 * types of the supplied {@link Executable} (i.e., constructor or method).
+	 * <p>The arguments are converted 'in-place' in the input array.
 	 * @param converter the type converter to use for attempting conversions
 	 * @param arguments the actual arguments that need conversion
 	 * @param executable the target Method or Constructor
@@ -334,8 +342,9 @@ public abstract class ReflectionHelper {
 	}
 
 	/**
-	 * Takes an input set of argument values and converts them to the types specified as the
-	 * required parameter types. The arguments are converted 'in-place' in the input array.
+	 * Takes an input set of argument values and converts them to the parameter
+	 * types of the supplied {@link MethodHandle}.
+	 * <p>The arguments are converted 'in-place' in the input array.
 	 * @param converter the type converter to use for attempting conversions
 	 * @param arguments the actual arguments that need conversion
 	 * @param methodHandle the target MethodHandle