Support Collection target types in custom IndexAccessors

Prior to this commit, an IndexAccessor could not provide support for a
Collection target type, since the built-in support for indexing into a
Collection in SpEL's Indexer took precedence.

This commit allows an IndexAccessor to support custom Collection target
types by separating the built-in List and Collection support and
applying the built-in Collection support after custom index accessors
have been applied.

Closes gh-32736

Author: sbrannen

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

@@ -212,6 +212,22 @@ private ValueRef getValueRef(ExpressionState state, AccessMode accessMode) throw
 		// At this point, we need a TypeDescriptor for a non-null target object
 		Assert.state(targetDescriptor != null, "No type descriptor");
 
+		// Indexing into an array
+		if (target.getClass().isArray()) {
+			int intIndex = convertIndexToInt(state, index);
+			this.indexedType = IndexedType.ARRAY;
+			return new ArrayIndexingValueRef(state.getTypeConverter(), target, intIndex, targetDescriptor);
+		}
+
+		// Indexing into a List
+		if (target instanceof List<?> list) {
+			int intIndex = convertIndexToInt(state, index);
+			this.indexedType = IndexedType.LIST;
+			return new CollectionIndexingValueRef(list, intIndex, targetDescriptor,
+					state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(),
+					state.getConfiguration().getMaximumAutoGrowSize());
+		}
+
 		// Indexing into a Map
 		if (target instanceof Map<?, ?> map) {
 			Object key = index;
@@ -223,26 +239,11 @@ private ValueRef getValueRef(ExpressionState state, AccessMode accessMode) throw
 			return new MapIndexingValueRef(state.getTypeConverter(), map, key, targetDescriptor);
 		}
 
-		// If the object is something that looks indexable by an integer,
-		// attempt to treat the index value as a number
-		if (target.getClass().isArray() || target instanceof Collection || target instanceof String) {
-			int idx = (Integer) state.convertValue(index, TypeDescriptor.valueOf(Integer.class));
-			if (target.getClass().isArray()) {
-				this.indexedType = IndexedType.ARRAY;
-				return new ArrayIndexingValueRef(state.getTypeConverter(), target, idx, targetDescriptor);
-			}
-			else if (target instanceof Collection<?> collection) {
-				if (target instanceof List) {
-					this.indexedType = IndexedType.LIST;
-				}
-				return new CollectionIndexingValueRef(collection, idx, targetDescriptor,
-						state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(),
-						state.getConfiguration().getMaximumAutoGrowSize());
-			}
-			else {
-				this.indexedType = IndexedType.STRING;
-				return new StringIndexingValueRef((String) target, idx, targetDescriptor);
-			}
+		// Indexing into a String
+		if (target instanceof String string) {
+			int intIndex = convertIndexToInt(state, index);
+			this.indexedType = IndexedType.STRING;
+			return new StringIndexingValueRef(string, intIndex, targetDescriptor);
 		}
 
 		// Check for a custom IndexAccessor.
@@ -280,6 +281,14 @@ else if (target instanceof Collection<?> collection) {
 			}
 		}
 
+		// Fallback indexing support for collections
+		if (target instanceof Collection<?> collection) {
+			int intIndex = convertIndexToInt(state, index);
+			return new CollectionIndexingValueRef(collection, intIndex, targetDescriptor,
+					state.getTypeConverter(), state.getConfiguration().isAutoGrowCollections(),
+					state.getConfiguration().getMaximumAutoGrowSize());
+		}
+
 		// As a last resort, try to treat the index value as a property of the context object.
 		TypeDescriptor valueType = indexValue.getTypeDescriptor();
 		if (valueType != null && String.class == valueType.getType()) {
@@ -458,6 +467,10 @@ private void setExitTypeDescriptor(String descriptor) {
 		}
 	}
 
+	private static int convertIndexToInt(ExpressionState state, Object index) {
+		return (Integer) state.convertValue(index, TypeDescriptor.valueOf(Integer.class));
+	}
+
 	private static Class<?> getObjectType(Object obj) {
 		return (obj instanceof Class<?> clazz ? clazz : obj.getClass());
 	}

spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java

@@ -21,9 +21,12 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.math.BigDecimal;
+import java.util.AbstractCollection;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
@@ -44,6 +47,7 @@
 import org.springframework.expression.PropertyAccessor;
 import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.ReflectiveIndexAccessor;
 import org.springframework.expression.spel.support.SimpleEvaluationContext;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.expression.spel.testresources.Person;
@@ -735,6 +739,25 @@ void readIndexWithStringIndexType() {
 					.havingCause().withMessage("unknown bird: property");
 		}
 
+		@Test  // gh-32736
+		void readIndexWithCollectionTargetType() {
+			context.addIndexAccessor(new ColorCollectionIndexAccessor());
+
+			Expression expression = parser.parseExpression("[0]");
+
+			// List.of() relies on built-in list support.
+			assertThat(expression.getValue(context, List.of(Color.RED))).isEqualTo(Color.RED);
+
+			ColorCollection colorCollection = new ColorCollection();
+
+			// Preconditions for this use case.
+			assertThat(colorCollection).isInstanceOf(Collection.class);
+			assertThat(colorCollection).isNotInstanceOf(List.class);
+
+			// ColorCollection relies on custom ColorCollectionIndexAccessor.
+			assertThat(expression.getValue(context, colorCollection)).isEqualTo(Color.RED);
+		}
+
 		static class BirdNameToColorMappings {
 
 			public final String property = "enigma";
@@ -755,6 +778,34 @@ static class BirdNameToColorMappingsIndexAccessor extends ReflectiveIndexAccesso
 			}
 		}
 
+		static class ColorCollection extends AbstractCollection<Color> {
+
+			public Color get(int index) {
+				return switch (index) {
+					case 0 -> Color.RED;
+					case 1 -> Color.BLUE;
+					default -> throw new NoSuchElementException("No color at index " + index);
+				};
+			}
+
+			@Override
+			public Iterator<Color> iterator() {
+				throw new UnsupportedOperationException();
+			}
+
+			@Override
+			public int size() {
+				throw new UnsupportedOperationException();
+			}
+		}
+
+		static class ColorCollectionIndexAccessor extends ReflectiveIndexAccessor {
+
+			ColorCollectionIndexAccessor() {
+				super(ColorCollection.class, int.class, "get");
+			}
+		}
+
 		/**
 		 * {@link IndexAccessor} that knows how to read and write indexes in a
 		 * Jackson {@link ArrayNode}.