Fix compilation of SpEL Indexer nodes involving map references

There is special handling for SpEL expressions involving a map
and an unquoted string literal key (e.g. mymap[key1]). SpEL does
not require key1 to be quoted. This special handling which is done
in Indexer getValueRef() was not being also done in the Indexer
generateCode() method that compiles the expression. Also fixed
a problem where the key was not being compiled in a new
sub scope. Without the new scope the key expression was failing
to reload the relevant context object when it needed it.

Issue: SPR-12045

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

@@ -180,8 +180,17 @@ public class Indexer extends SpelNodeImpl {
 		if (this.indexedType == IndexedType.array) {
 			return exitTypeDescriptor != null;
 		}
-		else if (this.indexedType == IndexedType.list || this.indexedType == IndexedType.map) {
-			return true;
+		else if (this.indexedType == IndexedType.list) {
+			return this.children[0].isCompilable();
+		} 
+		else if (this.indexedType == IndexedType.map) {
+			if (this.children[0] instanceof PropertyOrFieldReference) {
+				// Only the name will be used, so that is OK
+				return true;
+			}
+			else {
+				return this.children[0].isCompilable();
+			}
 		}
 		else if (this.indexedType == IndexedType.object) {
 			// If the string name is changing the accessor is clearly going to change (so compilation is not possible)
@@ -207,61 +216,89 @@ public class Indexer extends SpelNodeImpl {
 			if (exitTypeDescriptor == "I") {
 				mv.visitTypeInsn(CHECKCAST,"[I");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(IALOAD);
 			} 
 			else if (exitTypeDescriptor == "D") {
 				mv.visitTypeInsn(CHECKCAST,"[D");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
 				mv.visitInsn(DALOAD);
 			}
 			else if (exitTypeDescriptor == "J") {
 				mv.visitTypeInsn(CHECKCAST,"[J");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(LALOAD);
 			}
 			else if (exitTypeDescriptor == "F") {
 				mv.visitTypeInsn(CHECKCAST,"[F");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(FALOAD);
 			}
 			else if (exitTypeDescriptor == "S") {
 				mv.visitTypeInsn(CHECKCAST,"[S");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(SALOAD);
 			}
 			else if (exitTypeDescriptor == "B") {
 				mv.visitTypeInsn(CHECKCAST,"[B");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(BALOAD);
 			}
 			else if (exitTypeDescriptor == "C") {
 				mv.visitTypeInsn(CHECKCAST,"[C");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(CALOAD);
 			}
 			else { 
 				mv.visitTypeInsn(CHECKCAST,"["+exitTypeDescriptor+(CodeFlow.isPrimitiveArray(exitTypeDescriptor)?"":";"));//depthPlusOne(exitTypeDescriptor)+"Ljava/lang/Object;");
 				SpelNodeImpl index = this.children[0];
+				codeflow.enterCompilationScope();
 				index.generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
 				mv.visitInsn(AALOAD);
 			}
 		}
 		else if (this.indexedType == IndexedType.list) {
 			mv.visitTypeInsn(CHECKCAST,"java/util/List");
+			codeflow.enterCompilationScope();
 			this.children[0].generateCode(mv, codeflow);
+			codeflow.exitCompilationScope();
 			mv.visitMethodInsn(INVOKEINTERFACE,"java/util/List","get","(I)Ljava/lang/Object;", true);
 			CodeFlow.insertCheckCast(mv,exitTypeDescriptor);
 		}
 		else if (this.indexedType == IndexedType.map) {
 			mv.visitTypeInsn(CHECKCAST,"java/util/Map");
-			this.children[0].generateCode(mv, codeflow);
+			// Special case when the key is an unquoted string literal that will be parsed as
+			// a property/field reference
+			if ((this.children[0] instanceof PropertyOrFieldReference)) {
+				PropertyOrFieldReference reference = (PropertyOrFieldReference) this.children[0];
+				String mapKeyName = reference.getName();
+				mv.visitLdcInsn(mapKeyName);
+			}
+			else {
+				codeflow.enterCompilationScope();
+				this.children[0].generateCode(mv, codeflow);
+				codeflow.exitCompilationScope();
+			}
 			mv.visitMethodInsn(INVOKEINTERFACE,"java/util/Map","get","(Ljava/lang/Object;)Ljava/lang/Object;", true);
 			CodeFlow.insertCheckCast(mv,exitTypeDescriptor);
 		} 

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

@@ -37,6 +37,7 @@ import org.springframework.expression.spel.ast.SpelNodeImpl;
 import org.springframework.expression.spel.ast.Ternary;
 import org.springframework.expression.spel.standard.SpelCompiler;
 import org.springframework.expression.spel.standard.SpelExpression;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 
 import static org.junit.Assert.*;
@@ -2468,8 +2469,69 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertTrue(expression.getValue(new GenericMessageTestHelper2<Integer>(6),Boolean.TYPE));
 	}
 
+	@Test
+	public void indexerMapAccessor_12045() throws Exception {
+		SpelParserConfiguration spc = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE,this.getClass().getClassLoader());
+		SpelExpressionParser sep = new SpelExpressionParser(spc);
+		expression=sep.parseExpression("headers[command]");
+		MyMessage root = new MyMessage();
+		assertEquals("wibble",expression.getValue(root));
+		// This next call was failing because the isCompilable check in Indexer did not check on the key being compilable
+		// (and also generateCode in the Indexer was missing the optimization that it didn't need necessarily need to call
+		// generateCode for that accessor)
+		assertEquals("wibble",expression.getValue(root));
+		assertCanCompile(expression);
+
+		// What about a map key that is an expression - ensure the getKey() is evaluated in the right scope
+		expression=sep.parseExpression("headers[getKey()]");
+		assertEquals("wobble",expression.getValue(root));
+		assertEquals("wobble",expression.getValue(root));
+		
+		expression=sep.parseExpression("list[getKey2()]");
+		assertEquals("wobble",expression.getValue(root));
+		assertEquals("wobble",expression.getValue(root));
+		
+		expression = sep.parseExpression("ia[getKey2()]");
+		assertEquals(3,expression.getValue(root));
+		assertEquals(3,expression.getValue(root));
+	}
+
 	// ---
 
+	public static interface Message<T> {
+		MessageHeaders getHeaders();
+		@SuppressWarnings("rawtypes")
+		List getList();
+		int[] getIa();
+	}
+	
+	public static class MyMessage implements Message<String> {
+		public MessageHeaders getHeaders() {
+			MessageHeaders mh = new MessageHeaders();
+			mh.put("command", "wibble");
+			mh.put("command2", "wobble");
+			return mh;
+		}
+		public int[] getIa() { return new int[]{5,3}; }
+		@SuppressWarnings({ "rawtypes", "unchecked" })
+		public List getList() {
+			List l = new ArrayList();
+			l.add("wibble");
+			l.add("wobble");
+			return l;
+		}
+		
+		public String getKey() {
+			return "command2";
+		}
+		
+		public int getKey2() {
+			return 1;
+		}
+	}
+	
+	public static class MessageHeaders extends HashMap<String,Object> {	}
+
 	public static class GenericMessageTestHelper<T> {
 		private T payload;