Polish SpEL internals and remove duplicate code

Author: sbrannen

Diff for: spring-expression/src/main/java/org/springframework/expression/ConstructorExecutor.java

@@ -34,7 +34,7 @@
  * @author Andy Clement
  * @author Sam Brannen
  * @since 3.0
- * @see MethodResolver
+ * @see ConstructorResolver
  * @see MethodExecutor
  */
 @FunctionalInterface

Diff for: spring-expression/src/main/java/org/springframework/expression/PropertyAccessor.java

@@ -45,7 +45,7 @@ public interface PropertyAccessor {
 	/**
 	 * Return an array of classes for which this property accessor should be called.
 	 * <p>Returning {@code null} indicates this is a general property accessor that
-	 * can be called in an attempt to resolve a property on any type.
+	 * can be called in an attempt to access a property on any type.
 	 * @return an array of classes that this property accessor is suitable for
 	 * (or {@code null} if a general property accessor)
 	 */

Diff for: spring-expression/src/main/java/org/springframework/expression/spel/ast/AstUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 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.
@@ -21,55 +21,59 @@
 
 import org.springframework.expression.PropertyAccessor;
 import org.springframework.lang.Nullable;
+import org.springframework.util.ObjectUtils;
 
 /**
- * Utilities methods for use in the Ast classes.
+ * Utility methods for use in the AST classes.
  *
  * @author Andy Clement
  * @since 3.0.2
  */
 public abstract class AstUtils {
 
 	/**
-	 * Determines the set of property resolvers that should be used to try and access a
-	 * property on the specified target type. The resolvers are considered to be in an
-	 * ordered list, however in the returned list any that are exact matches for the input
-	 * target type (as opposed to 'general' resolvers that could work for any type) are
-	 * placed at the start of the list. In addition, there are specific resolvers that
-	 * exactly name the class in question and resolvers that name a specific class but it
-	 * is a supertype of the class we have. These are put at the end of the specific resolvers
-	 * set and will be tried after exactly matching accessors but before generic accessors.
+	 * Determine the set of property accessors that should be used to try to
+	 * access a property on the specified target type.
+	 * <p>The accessors are considered to be in an ordered list; however, in the
+	 * returned list any accessors that are exact matches for the input target
+	 * type (as opposed to 'general' accessors that could work for any type) are
+	 * placed at the start of the list. In addition, if there are specific
+	 * accessors that exactly name the class in question and accessors that name
+	 * a specific class which is a supertype of the class in question, the latter
+	 * are put at the end of the specific accessors set and will be tried after
+	 * exactly matching accessors but before generic accessors.
 	 * @param targetType the type upon which property access is being attempted
-	 * @return a list of resolvers that should be tried in order to access the property
+	 * @param propertyAccessors the list of property accessors to process
+	 * @return a list of accessors that should be tried in order to access the property
 	 */
 	public static List<PropertyAccessor> getPropertyAccessorsToTry(
 			@Nullable Class<?> targetType, List<PropertyAccessor> propertyAccessors) {
 
 		List<PropertyAccessor> specificAccessors = new ArrayList<>();
 		List<PropertyAccessor> generalAccessors = new ArrayList<>();
-		for (PropertyAccessor resolver : propertyAccessors) {
-			Class<?>[] targets = resolver.getSpecificTargetClasses();
-			if (targets == null) {  // generic resolver that says it can be used for any type
-				generalAccessors.add(resolver);
+		for (PropertyAccessor accessor : propertyAccessors) {
+			Class<?>[] targets = accessor.getSpecificTargetClasses();
+			if (ObjectUtils.isEmpty(targets)) {
+				// generic accessor that says it can be used for any type
+				generalAccessors.add(accessor);
 			}
-			else {
-				if (targetType != null) {
-					for (Class<?> clazz : targets) {
-						if (clazz == targetType) {  // put exact matches on the front to be tried first?
-							specificAccessors.add(resolver);
-						}
-						else if (clazz.isAssignableFrom(targetType)) {  // put supertype matches at the end of the
-							// specificAccessor list
-							generalAccessors.add(resolver);
-						}
+			else if (targetType != null) {
+				for (Class<?> clazz : targets) {
+					if (clazz == targetType) {
+						// add exact matches to the specificAccessors list
+						specificAccessors.add(accessor);
+					}
+					else if (clazz.isAssignableFrom(targetType)) {
+						// add supertype matches to the front of the generalAccessors list
+						generalAccessors.add(0, accessor);
 					}
 				}
 			}
 		}
-		List<PropertyAccessor> resolvers = new ArrayList<>(specificAccessors.size() + generalAccessors.size());
-		resolvers.addAll(specificAccessors);
-		resolvers.addAll(generalAccessors);
-		return resolvers;
+		List<PropertyAccessor> accessors = new ArrayList<>(specificAccessors.size() + generalAccessors.size());
+		accessors.addAll(specificAccessors);
+		accessors.addAll(generalAccessors);
+		return accessors;
 	}
 
 }

Diff for: spring-expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java

@@ -298,46 +298,17 @@ public boolean isWritableProperty(String name, TypedValue contextObject, Evaluat
 	}
 
 	/**
-	 * Determines the set of property resolvers that should be used to try and access a property
-	 * on the specified target type. The resolvers are considered to be in an ordered list,
-	 * however in the returned list any that are exact matches for the input target type (as
-	 * opposed to 'general' resolvers that could work for any type) are placed at the start of the
-	 * list. In addition, there are specific resolvers that exactly name the class in question
-	 * and resolvers that name a specific class but it is a supertype of the class we have.
-	 * These are put at the end of the specific resolvers set and will be tried after exactly
-	 * matching accessors but before generic accessors.
+	 * Determine the set of property accessors that should be used to try to
+	 * access a property on the specified context object.
+	 * <p>Delegates to {@link AstUtils#getPropertyAccessorsToTry(Class, List)}.
 	 * @param contextObject the object upon which property access is being attempted
-	 * @return a list of resolvers that should be tried in order to access the property
+	 * @return a list of accessors that should be tried in order to access the property
 	 */
 	private List<PropertyAccessor> getPropertyAccessorsToTry(
 			@Nullable Object contextObject, List<PropertyAccessor> propertyAccessors) {
 
 		Class<?> targetType = (contextObject != null ? contextObject.getClass() : null);
-
-		List<PropertyAccessor> specificAccessors = new ArrayList<>();
-		List<PropertyAccessor> generalAccessors = new ArrayList<>();
-		for (PropertyAccessor resolver : propertyAccessors) {
-			Class<?>[] targets = resolver.getSpecificTargetClasses();
-			if (targets == null) {
-				// generic resolver that says it can be used for any type
-				generalAccessors.add(resolver);
-			}
-			else if (targetType != null) {
-				for (Class<?> clazz : targets) {
-					if (clazz == targetType) {
-						specificAccessors.add(resolver);
-						break;
-					}
-					else if (clazz.isAssignableFrom(targetType)) {
-						generalAccessors.add(resolver);
-					}
-				}
-			}
-		}
-		List<PropertyAccessor> resolvers = new ArrayList<>(specificAccessors);
-		generalAccessors.removeAll(specificAccessors);
-		resolvers.addAll(generalAccessors);
-		return resolvers;
+		return AstUtils.getPropertyAccessorsToTry(targetType, propertyAccessors);
 	}
 
 	@Override