Fix SpEL compilation for non trivial elvis operand

Issue: SPR-17214

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -79,10 +79,12 @@ public class Elvis extends SpelNodeImpl {
 	public void generateCode(MethodVisitor mv, CodeFlow cf) {
 		// exit type descriptor can be null if both components are literal expressions
 		computeExitTypeDescriptor();
+		cf.enterCompilationScope();
 		this.children[0].generateCode(mv, cf);
 		String lastDesc = cf.lastDescriptor();
 		Assert.state(lastDesc != null, "No last descriptor");
 		CodeFlow.insertBoxIfNecessary(mv, lastDesc.charAt(0));
+		cf.exitCompilationScope();
 		Label elseTarget = new Label();
 		Label endOfIf = new Label();
 		mv.visitInsn(DUP);
@@ -95,12 +97,14 @@ public class Elvis extends SpelNodeImpl {
 		mv.visitJumpInsn(IFEQ, endOfIf);  // if not empty, drop through to elseTarget
 		mv.visitLabel(elseTarget);
 		mv.visitInsn(POP);
+		cf.enterCompilationScope();
 		this.children[1].generateCode(mv, cf);
 		if (!CodeFlow.isPrimitive(this.exitTypeDescriptor)) {
 			lastDesc = cf.lastDescriptor();
 			Assert.state(lastDesc != null, "No last descriptor");
 			CodeFlow.insertBoxIfNecessary(mv, lastDesc.charAt(0));
 		}
+		cf.exitCompilationScope();
 		mv.visitLabel(endOfIf);
 		cf.pushDescriptor(this.exitTypeDescriptor);
 	}

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

@@ -692,7 +692,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 	}
 
 	@Test
-	public void ternaryWithBooleanReturn() { // SPR-12271
+	public void ternaryWithBooleanReturn_SPR12271() {
 		expression = parser.parseExpression("T(Boolean).TRUE?'abc':'def'");
 		assertEquals("abc", expression.getValue());
 		assertCanCompile(expression);
@@ -703,7 +703,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertEquals("def", expression.getValue());
 	}
-	
+
 	@Test
 	public void nullsafeFieldPropertyDereferencing_SPR16489() throws Exception {
 		FooObjectHolder foh = new FooObjectHolder();
@@ -715,7 +715,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertEquals("hello",expression.getValue(context));
 		foh.foo = null;
 		assertNull(expression.getValue(context));
-		
+
 		// Now revert state of foh and try compiling it:
 		foh.foo = new FooObject();
 		assertEquals("hello",expression.getValue(context));
@@ -723,9 +723,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertEquals("hello",expression.getValue(context));
 		foh.foo = null;
 		assertNull(expression.getValue(context));
-		
+
 		// Static references
-		expression = (SpelExpression)parser.parseExpression("#var?.propertya");
+		expression = (SpelExpression) parser.parseExpression("#var?.propertya");
 		context.setVariable("var", StaticsHelper.class);
 		assertEquals("sh",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -737,7 +737,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertNull(expression.getValue(context));
 
 		// Single size primitive (boolean)
-		expression = (SpelExpression)parser.parseExpression("#var?.a");
+		expression = (SpelExpression) parser.parseExpression("#var?.a");
 		context.setVariable("var", new TestClass4());
 		assertFalse((Boolean)expression.getValue(context));
 		context.setVariable("var", null);
@@ -749,7 +749,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertNull(expression.getValue(context));
 
 		// Double slot primitives
-		expression = (SpelExpression)parser.parseExpression("#var?.four");
+		expression = (SpelExpression) parser.parseExpression("#var?.four");
 		context.setVariable("var", new Three());
 		assertEquals("0.04",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -760,7 +760,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		context.setVariable("var", null);
 		assertNull(expression.getValue(context));
 	}
-	
+
 	@Test
 	public void nullsafeMethodChaining_SPR16489() throws Exception {
 		FooObjectHolder foh = new FooObjectHolder();
@@ -777,9 +777,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertEquals("hello",expression.getValue(context));
 		foh.foo = null;
 		assertNull(expression.getValue(context));
-		
+
 		// Static method references
-		expression = (SpelExpression)parser.parseExpression("#var?.methoda()");
+		expression = (SpelExpression) parser.parseExpression("#var?.methoda()");
 		context.setVariable("var", StaticsHelper.class);
 		assertEquals("sh",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -789,9 +789,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertEquals("sh",expression.getValue(context).toString());
 		context.setVariable("var", null);
 		assertNull(expression.getValue(context));
-		
+
 		// Nullsafe guard on expression element evaluating to primitive/null
-		expression = (SpelExpression)parser.parseExpression("#var?.intValue()");
+		expression = (SpelExpression) parser.parseExpression("#var?.intValue()");
 		context.setVariable("var", 4);
 		assertEquals("4",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -802,9 +802,8 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		context.setVariable("var", null);
 		assertNull(expression.getValue(context));
 
-		
 		// Nullsafe guard on expression element evaluating to primitive/null
-		expression = (SpelExpression)parser.parseExpression("#var?.booleanValue()");
+		expression = (SpelExpression) parser.parseExpression("#var?.booleanValue()");
 		context.setVariable("var", false);
 		assertEquals("false",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -816,7 +815,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertNull(expression.getValue(context));
 
 		// Nullsafe guard on expression element evaluating to primitive/null
-		expression = (SpelExpression)parser.parseExpression("#var?.booleanValue()");
+		expression = (SpelExpression) parser.parseExpression("#var?.booleanValue()");
 		context.setVariable("var", true);
 		assertEquals("true",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -828,7 +827,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertNull(expression.getValue(context));
 
 		// Nullsafe guard on expression element evaluating to primitive/null
-		expression = (SpelExpression)parser.parseExpression("#var?.longValue()");
+		expression = (SpelExpression) parser.parseExpression("#var?.longValue()");
 		context.setVariable("var", 5L);
 		assertEquals("5",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -840,7 +839,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertNull(expression.getValue(context));
 
 		// Nullsafe guard on expression element evaluating to primitive/null
-		expression = (SpelExpression)parser.parseExpression("#var?.floatValue()");
+		expression = (SpelExpression) parser.parseExpression("#var?.floatValue()");
 		context.setVariable("var", 3f);
 		assertEquals("3.0",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -852,7 +851,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertNull(expression.getValue(context));
 
 		// Nullsafe guard on expression element evaluating to primitive/null
-		expression = (SpelExpression)parser.parseExpression("#var?.shortValue()");
+		expression = (SpelExpression) parser.parseExpression("#var?.shortValue()");
 		context.setVariable("var", (short)8);
 		assertEquals("8",expression.getValue(context).toString());
 		context.setVariable("var", null);
@@ -863,7 +862,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		context.setVariable("var", null);
 		assertNull(expression.getValue(context));
 	}
-	
+
 	@Test
 	public void elvis() throws Exception {
 		Expression expression = parser.parseExpression("'a'?:'b'");
@@ -1575,7 +1574,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertTrue((Boolean) expression.getValue(f));
 
-		long l = 300l;
+		long l = 300L;
 		expression = parse("#root==300l");
 		assertTrue((Boolean) expression.getValue(l));
 		assertCanCompile(expression);
@@ -3236,15 +3235,15 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertTrue(expression.getValue(Boolean.class));
 		context.setVariable("it", null);
 		assertNull(expression.getValue(Boolean.class));
-		
+
 		assertCanCompile(expression);
-		
+
 		context.setVariable("it", 3);
 		assertTrue(expression.getValue(Boolean.class));
 		context.setVariable("it", null);
 		assertNull(expression.getValue(Boolean.class));
 	}
-	
+
 	@Test
 	public void failsWhenSettingContextForExpression_SPR12326() {
 		SpelExpressionParser parser = new SpelExpressionParser(
@@ -3259,9 +3258,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertTrue(expression.getValue(Boolean.class));
 		context.setVariable("it", null);
 		assertNull(expression.getValue(Boolean.class));
-		
+
 		assertCanCompile(expression);
-		
+
 		context.setVariable("it", person);
 		assertTrue(expression.getValue(Boolean.class));
 		context.setVariable("it", null);
@@ -4199,7 +4198,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 	}
 
 	@Test
-	public void propertyReferenceVisibility() { // SPR-12771
+	public void propertyReferenceVisibility_SPR12771() {
 		StandardEvaluationContext ctx = new StandardEvaluationContext();
 		ctx.setVariable("httpServletRequest", HttpServlet3RequestFactory.getOne());
 		// Without a fix compilation was inserting a checkcast to a private type
@@ -4813,46 +4812,46 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertEquals(3, expression.getValue(root));
 		assertEquals(3, expression.getValue(root));
 	}
-	
+
 	@Test
 	public void elvisOperator_SPR15192() {
 		SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null);
 		Expression exp;
-		
+
 		exp = new SpelExpressionParser(configuration).parseExpression("bar()");
 		assertEquals("BAR", exp.getValue(new Foo(), String.class));
 		assertCanCompile(exp);
 		assertEquals("BAR", exp.getValue(new Foo(), String.class));
 		assertIsCompiled(exp);
- 
+
 		exp = new SpelExpressionParser(configuration).parseExpression("bar('baz')");
 		assertEquals("BAZ", exp.getValue(new Foo(), String.class));
 		assertCanCompile(exp);
 		assertEquals("BAZ", exp.getValue(new Foo(), String.class));
 		assertIsCompiled(exp);
- 
+
 		StandardEvaluationContext context = new StandardEvaluationContext();
 		context.setVariable("map", Collections.singletonMap("foo", "qux"));
- 
+
 		exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'])");
 		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
 		assertCanCompile(exp);
 		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
- 
+
 		exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'] ?: 'qux')");
 		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
 		assertCanCompile(exp);
 		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// When the condition is a primitive
 		exp = new SpelExpressionParser(configuration).parseExpression("3?:'foo'");
 		assertEquals("3", exp.getValue(context, new Foo(), String.class));
 		assertCanCompile(exp);
 		assertEquals("3", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// When the condition is a double slot primitive
 		exp = new SpelExpressionParser(configuration).parseExpression("3L?:'foo'");
 		assertEquals("3", exp.getValue(context, new Foo(), String.class));
@@ -4866,7 +4865,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(exp);
 		assertEquals("4", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// null condition
 		exp = new SpelExpressionParser(configuration).parseExpression("null?:4L");
 		assertEquals("4", exp.getValue(context, new Foo(), String.class));
@@ -4888,7 +4887,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(exp);
 		assertEquals("foo", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// variable access returning array
 		exp = new SpelExpressionParser(configuration).parseExpression("#x?:'foo'");
 		context.setVariable("x",new int[]{1,2,3});
@@ -4898,19 +4897,67 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertIsCompiled(exp);
 	}
 
+	@Test
+	public void elvisOperator_SPR17214() throws Exception {
+		SpelParserConfiguration spc = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null);
+		SpelExpressionParser sep = new SpelExpressionParser(spc);
+
+		RecordHolder rh = null;
+
+		expression = sep.parseExpression("record.get('abc')?:record.put('abc',expression.someLong?.longValue())");
+		rh = new RecordHolder();
+		assertNull(expression.getValue(rh));
+		assertEquals(3L,expression.getValue(rh));
+		assertCanCompile(expression);
+		rh = new RecordHolder();
+		assertNull(expression.getValue(rh));
+		assertEquals(3L,expression.getValue(rh));
+
+		expression = sep.parseExpression("record.get('abc')?:record.put('abc',3L.longValue())");
+		rh = new RecordHolder();
+		assertNull(expression.getValue(rh));
+		assertEquals(3L,expression.getValue(rh));
+		assertCanCompile(expression);
+		rh = new RecordHolder();
+		assertNull(expression.getValue(rh));
+		assertEquals(3L,expression.getValue(rh));
+
+		expression = sep.parseExpression("record.get('abc')?:record.put('abc',3L.longValue())");
+		rh = new RecordHolder();
+		assertNull(expression.getValue(rh));
+		assertEquals(3L,expression.getValue(rh));
+		assertCanCompile(expression);
+		rh = new RecordHolder();
+		assertNull(expression.getValue(rh));
+		assertEquals(3L,expression.getValue(rh));
+
+		expression = sep.parseExpression("record.get('abc')==null?record.put('abc',expression.someLong?.longValue()):null");
+		rh = new RecordHolder();
+		rh.expression.someLong=6L;
+		assertNull(expression.getValue(rh));
+		assertEquals(6L,rh.get("abc"));
+		assertNull(expression.getValue(rh));
+		assertCanCompile(expression);
+		rh = new RecordHolder();
+		rh.expression.someLong=6L;
+		assertNull(expression.getValue(rh));
+		assertEquals(6L,rh.get("abc"));
+		assertNull(expression.getValue(rh));
+	}
+
 	@Test
 	public void ternaryOperator_SPR15192() {
 		SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null);
 		Expression exp;
 		StandardEvaluationContext context = new StandardEvaluationContext();
 		context.setVariable("map", Collections.singletonMap("foo", "qux"));
- 
+
 		exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'] != null ? #map['foo'] : 'qux')");
 		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
 		assertCanCompile(exp);
 		assertEquals("QUX", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		exp = new SpelExpressionParser(configuration).parseExpression("3==3?3:'foo'");
 		assertEquals("3", exp.getValue(context, new Foo(), String.class));
 		assertCanCompile(exp);
@@ -4921,7 +4968,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(exp);
 		assertEquals("foo", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// When the condition is a double slot primitive
 		exp = new SpelExpressionParser(configuration).parseExpression("3==3?3L:'foo'");
 		assertEquals("3", exp.getValue(context, new Foo(), String.class));
@@ -4940,7 +4987,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(exp);
 		assertEquals("abc", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// null condition
 		exp = new SpelExpressionParser(configuration).parseExpression("3==3?null:4L");
 		assertEquals(null, exp.getValue(context, new Foo(), String.class));
@@ -4962,7 +5009,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(exp);
 		assertEquals("foo", exp.getValue(context, new Foo(), String.class));
 		assertIsCompiled(exp);
-		
+
 		// variable access returning array
 		exp = new SpelExpressionParser(configuration).parseExpression("#x==#x?'1,2,3':'foo'");
 		context.setVariable("x",new int[]{1,2,3});
@@ -5289,9 +5336,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 	}
 
 	public static class FooObjectHolder {
-		
+
 		private FooObject foo = new FooObject();
-		
+
 		public FooObject getFoo() {
 			return foo;
 		}
@@ -6060,4 +6107,26 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		}
 	}
 
+
+	public static class RecordHolder {
+
+		public Map<String,Long> record = new HashMap<>();
+
+		public LongHolder expression = new LongHolder();
+
+		public void add(String key, Long value) {
+			record.put(key, value);
+		}
+
+		public long get(String key) {
+			return record.get(key);
+		}
+	}
+
+
+	public static class LongHolder {
+
+		public Long someLong = 3L;
+	}
+
 }