From 59080ff2b21e0e442238ae7a861351b9df76ba0f Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Tue, 29 Jul 2014 10:48:06 -0700 Subject: [PATCH] Cope with generic methods during SpEL compilation This commit allows the SpEL compiler to cope with generic methods being used in expressions involving numeric operands. Due to the use of unbound type variables the methods may look like they return Object but in fact they are returning objects of a numeric type that are suitable for compilation. The changes here ensure the runtime types are looked at if the discovered declared types are not providing enough information. This impacts all the operands involving numerics (mathematical and relational). Issue: SPR-12040 --- .../expression/spel/CodeFlow.java | 22 +- .../expression/spel/ast/OpDivide.java | 6 +- .../expression/spel/ast/OpEQ.java | 25 +- .../expression/spel/ast/OpGE.java | 4 + .../expression/spel/ast/OpGT.java | 3 + .../expression/spel/ast/OpLE.java | 4 + .../expression/spel/ast/OpLT.java | 3 + .../expression/spel/ast/OpMinus.java | 6 +- .../expression/spel/ast/OpMultiply.java | 6 +- .../expression/spel/ast/OpNE.java | 24 +- .../expression/spel/ast/OpPlus.java | 6 +- .../expression/spel/ast/Operator.java | 87 ++++++- .../spel/SpelCompilationCoverageTests.java | 222 ++++++++++++++++++ 13 files changed, 368 insertions(+), 50 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java index 98a68ef579..d374e483e1 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java @@ -106,61 +106,59 @@ public class CodeFlow implements Opcodes { } } - /** * Insert any necessary cast and value call to convert from a boxed type to a * primitive value * @param mv the method visitor into which instructions should be inserted * @param ch the primitive type desired as output - * @param isObject indicates whether the type on the stack is being thought of - * as Object (and so requires a cast) + * @param stackDescriptor the descriptor of the type on top of the stack */ - public static void insertUnboxInsns(MethodVisitor mv, char ch, boolean isObject) { + public static void insertUnboxInsns(MethodVisitor mv, char ch, String stackDescriptor) { switch (ch) { case 'I': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Integer")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Integer"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Integer", "intValue", "()I", false); break; case 'Z': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Boolean")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Boolean"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Boolean", "booleanValue", "()Z", false); break; case 'B': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Byte")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Byte"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Byte", "byteValue", "()B", false); break; case 'C': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Character")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Character"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Character", "charValue", "()C", false); break; case 'D': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Double")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Double"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Double", "doubleValue", "()D", false); break; case 'S': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Short")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Short"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Short", "shortValue", "()S", false); break; case 'F': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Float")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Float"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Float", "floatValue", "()F", false); break; case 'J': - if (isObject) { + if (!stackDescriptor.equals("Ljava/lang/Long")) { mv.visitTypeInsn(CHECKCAST, "java/lang/Long"); } mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Long", "longValue", "()J", false); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java index 7f7c13ac9f..b1a6162d48 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpDivide.java @@ -107,13 +107,15 @@ public class OpDivide extends Operator { getLeftOperand().generateCode(mv, codeflow); String leftdesc = getLeftOperand().getExitDescriptor(); if (!CodeFlow.isPrimitive(leftdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftdesc); } if (this.children.length > 1) { + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); String rightdesc = getRightOperand().getExitDescriptor(); + codeflow.exitCompilationScope(); if (!CodeFlow.isPrimitive(rightdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightdesc); } switch (this.exitTypeDescriptor.charAt(0)) { case 'I': diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java index a20a6ea28a..8a3b420738 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java @@ -40,6 +40,8 @@ public class OpEQ extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); + rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); return BooleanTypedValue.forValue(equalityCheck(state, left, right)); } @@ -54,15 +56,14 @@ public class OpEQ extends Operator { } String leftdesc = left.getExitDescriptor(); String rightdesc = right.getExitDescriptor(); - if ((CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(leftdesc) || - CodeFlow.isPrimitiveOrUnboxableSupportedNumber(rightdesc))) { - if (!CodeFlow.areBoxingCompatible(leftdesc, rightdesc)) { - return false; - } + DescriptorComparison dc = DescriptorComparison.checkNumericCompatibility(leftdesc, rightdesc, leftActualDescriptor, rightActualDescriptor); + if (dc.areNumbers) { + return dc.areCompatible; } return true; } + @Override public void generateCode(MethodVisitor mv, CodeFlow codeflow) { String leftDesc = getLeftOperand().getExitDescriptor(); @@ -72,19 +73,21 @@ public class OpEQ extends Operator { boolean leftPrim = CodeFlow.isPrimitive(leftDesc); boolean rightPrim = CodeFlow.isPrimitive(rightDesc); - if ((CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(leftDesc) || - CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(rightDesc)) && - CodeFlow.areBoxingCompatible(leftDesc,rightDesc)) { - char targetType = CodeFlow.toPrimitiveTargetDesc(leftDesc); + DescriptorComparison dc = DescriptorComparison.checkNumericCompatibility(leftDesc, rightDesc, leftActualDescriptor, rightActualDescriptor); + + if (dc.areNumbers && dc.areCompatible) { + char targetType = dc.compatibleType; getLeftOperand().generateCode(mv, codeflow); if (!leftPrim) { - CodeFlow.insertUnboxInsns(mv, targetType, false); + CodeFlow.insertUnboxInsns(mv, targetType, leftDesc); } + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); + codeflow.exitCompilationScope(); if (!rightPrim) { - CodeFlow.insertUnboxInsns(mv, targetType, false); + CodeFlow.insertUnboxInsns(mv, targetType, rightDesc); } // assert: SpelCompiler.boxingCompatible(leftDesc, rightDesc) if (targetType=='D') { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java index 59f9302d6c..ef39c604f2 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGE.java @@ -44,6 +44,10 @@ public class OpGE extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + + leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); + rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); + if (left instanceof Number && right instanceof Number) { Number leftNumber = (Number) left; Number rightNumber = (Number) right; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java index fc55472fe4..7e022e6a49 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java @@ -45,6 +45,9 @@ public class OpGT extends Operator { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); + rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); + if (left instanceof Number && right instanceof Number) { Number leftNumber = (Number) left; Number rightNumber = (Number) right; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java index 1680e121e4..90e656e550 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLE.java @@ -46,6 +46,10 @@ public class OpLE extends Operator { throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + + leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); + rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); + if (left instanceof Number && right instanceof Number) { Number leftNumber = (Number) left; Number rightNumber = (Number) right; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java index 8fee088a6e..615eda5973 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java @@ -45,6 +45,9 @@ public class OpLT extends Operator { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); + rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); + if (left instanceof Number && right instanceof Number) { Number leftNumber = (Number) left; Number rightNumber = (Number) right; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java index 8d1fbdb51e..ccff337d76 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java @@ -169,13 +169,15 @@ public class OpMinus extends Operator { getLeftOperand().generateCode(mv, codeflow); String leftdesc = getLeftOperand().getExitDescriptor(); if (!CodeFlow.isPrimitive(leftdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftdesc); } if (this.children.length>1) { + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); String rightdesc = getRightOperand().getExitDescriptor(); + codeflow.exitCompilationScope(); if (!CodeFlow.isPrimitive(rightdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightdesc); } switch (this.exitTypeDescriptor.charAt(0)) { case 'I': diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java index d4722964a1..4d1fc60b63 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java @@ -138,13 +138,15 @@ public class OpMultiply extends Operator { getLeftOperand().generateCode(mv, codeflow); String leftdesc = getLeftOperand().getExitDescriptor(); if (!CodeFlow.isPrimitive(leftdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftdesc); } if (this.children.length>1) { + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); String rightdesc = getRightOperand().getExitDescriptor(); + codeflow.exitCompilationScope(); if (!CodeFlow.isPrimitive(rightdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightdesc); } switch (this.exitTypeDescriptor.charAt(0)) { case 'I': diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java index 61c17f60ac..f675773851 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java @@ -40,6 +40,8 @@ public class OpNE extends Operator { public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); + leftActualDescriptor = CodeFlow.toDescriptorFromObject(left); + rightActualDescriptor = CodeFlow.toDescriptorFromObject(right); return BooleanTypedValue.forValue(!equalityCheck(state, left, right)); } @@ -54,11 +56,9 @@ public class OpNE extends Operator { } String leftdesc = left.getExitDescriptor(); String rightdesc = right.getExitDescriptor(); - if ((CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(leftdesc) || - CodeFlow.isPrimitiveOrUnboxableSupportedNumber(rightdesc))) { - if (!CodeFlow.areBoxingCompatible(leftdesc, rightdesc)) { - return false; - } + DescriptorComparison dc = DescriptorComparison.checkNumericCompatibility(leftdesc, rightdesc, leftActualDescriptor, rightActualDescriptor); + if (dc.areNumbers) { + return dc.areCompatible; } return true; } @@ -72,19 +72,21 @@ public class OpNE extends Operator { boolean leftPrim = CodeFlow.isPrimitive(leftDesc); boolean rightPrim = CodeFlow.isPrimitive(rightDesc); - if ((CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(leftDesc) || - CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(rightDesc)) && - CodeFlow.areBoxingCompatible(leftDesc,rightDesc)) { - char targetType = CodeFlow.toPrimitiveTargetDesc(leftDesc); + DescriptorComparison dc = DescriptorComparison.checkNumericCompatibility(leftDesc, rightDesc, leftActualDescriptor, rightActualDescriptor); + + if (dc.areNumbers && dc.areCompatible) { + char targetType = dc.compatibleType; getLeftOperand().generateCode(mv, codeflow); if (!leftPrim) { - CodeFlow.insertUnboxInsns(mv, targetType, false); + CodeFlow.insertUnboxInsns(mv, targetType, leftDesc); } + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); + codeflow.exitCompilationScope(); if (!rightPrim) { - CodeFlow.insertUnboxInsns(mv, targetType, false); + CodeFlow.insertUnboxInsns(mv, targetType, rightDesc); } // assert: SpelCompiler.boxingCompatible(leftDesc, rightDesc) if (targetType == 'D') { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java index 843676aea7..b7a5f92106 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java @@ -196,13 +196,15 @@ public class OpPlus extends Operator { getLeftOperand().generateCode(mv, codeflow); String leftdesc = getLeftOperand().getExitDescriptor(); if (!CodeFlow.isPrimitive(leftdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), leftdesc); } if (this.children.length>1) { + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); String rightdesc = getRightOperand().getExitDescriptor(); + codeflow.exitCompilationScope(); if (!CodeFlow.isPrimitive(rightdesc)) { - CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), false); + CodeFlow.insertUnboxInsns(mv, this.exitTypeDescriptor.charAt(0), rightdesc); } switch (this.exitTypeDescriptor.charAt(0)) { case 'I': diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java index 0e7e32b211..fb1ef1aa7f 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java @@ -39,7 +39,12 @@ import org.springframework.util.ObjectUtils; public abstract class Operator extends SpelNodeImpl { private final String operatorName; - + + // The descriptors of the runtime operand values are used if the discovered declared + // descriptors are not providing enough information (for example a generic type + // whose accessors seem to only be returning 'Object' - the actual descriptors may + // indicate 'int') + protected String leftActualDescriptor, rightActualDescriptor; public Operator(String payload,int pos,SpelNodeImpl... operands) { super(pos, operands); @@ -84,10 +89,9 @@ public abstract class Operator extends SpelNodeImpl { // Supported operand types for equals (at the moment) String leftDesc = left.getExitDescriptor(); String rightDesc= right.getExitDescriptor(); - if (CodeFlow.isPrimitiveOrUnboxableSupportedNumber(leftDesc) && CodeFlow.isPrimitiveOrUnboxableSupportedNumber(rightDesc)) { - if (CodeFlow.areBoxingCompatible(leftDesc, rightDesc)) { - return true; - } + DescriptorComparison dc = DescriptorComparison.checkNumericCompatibility(leftDesc, rightDesc, leftActualDescriptor, rightActualDescriptor); + if (dc.areNumbers) { + return dc.areCompatible; } return false; } @@ -103,16 +107,19 @@ public abstract class Operator extends SpelNodeImpl { boolean unboxLeft = !CodeFlow.isPrimitive(leftDesc); boolean unboxRight = !CodeFlow.isPrimitive(rightDesc); - char targetType = CodeFlow.toPrimitiveTargetDesc(leftDesc); + DescriptorComparison dc = DescriptorComparison.checkNumericCompatibility(leftDesc, rightDesc, leftActualDescriptor, rightActualDescriptor); + char targetType = dc.compatibleType;//CodeFlow.toPrimitiveTargetDesc(leftDesc); getLeftOperand().generateCode(mv, codeflow); if (unboxLeft) { - CodeFlow.insertUnboxInsns(mv, targetType, false); + CodeFlow.insertUnboxInsns(mv, targetType, leftDesc); } + codeflow.enterCompilationScope(); getRightOperand().generateCode(mv, codeflow); + codeflow.exitCompilationScope(); if (unboxRight) { - CodeFlow.insertUnboxInsns(mv, targetType, false); + CodeFlow.insertUnboxInsns(mv, targetType, rightDesc); } // assert: SpelCompiler.boxingCompatible(leftDesc, rightDesc) Label elseTarget = new Label(); @@ -187,5 +194,69 @@ public abstract class Operator extends SpelNodeImpl { return false; } + + /** + * A descriptor comparison encapsulates the result of comparing descriptor for two operands and + * describes at what level they are compatible. + */ + protected static class DescriptorComparison { + static DescriptorComparison NOT_NUMBERS = new DescriptorComparison(false,false,' '); + static DescriptorComparison INCOMPATIBLE_NUMBERS = new DescriptorComparison(true,false,' '); + + final boolean areNumbers; // Were the two compared descriptor both for numbers? + final boolean areCompatible; // If they were numbers, were they compatible? + final char compatibleType; // When compatible, what is the descriptor of the common type + + private DescriptorComparison(boolean areNumbers, boolean areCompatible, char compatibleType) { + this.areNumbers = areNumbers; + this.areCompatible = areCompatible; + this.compatibleType = compatibleType; + } + + /** + * Returns an object that indicates whether the input descriptors are compatible. A declared descriptor + * is what could statically be determined (e.g. from looking at the return value of a property accessor + * method) whilst an actual descriptor is the type of an actual object that was returned, which may differ. + * For generic types with unbound type variables the declared descriptor discovered may be 'Object' but + * from the actual descriptor it is possible to observe that the objects are really numeric values (e.g. + * ints). + * + * @param leftDeclaredDescriptor the statically determinable left descriptor + * @param rightDeclaredDescriptor the statically determinable right descriptor + * @param leftActualDescriptor the dynamic/runtime left object descriptor + * @param rightActualDescriptor the dynamic/runtime right object descriptor + * @return a DescriptorComparison object indicating the type of compatibility, if any + */ + public static DescriptorComparison checkNumericCompatibility(String leftDeclaredDescriptor, String rightDeclaredDescriptor, String leftActualDescriptor, String rightActualDescriptor) { + String ld = leftDeclaredDescriptor; + String rd = rightDeclaredDescriptor; + + boolean leftNumeric = CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(ld) ; + boolean rightNumeric = CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(rd); + + // If the declared descriptors aren't providing the information, try the actual descriptors + if (!leftNumeric && !ld.equals(leftActualDescriptor)) { + ld = leftActualDescriptor; + leftNumeric = CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(ld); + } + if (!rightNumeric && !rd.equals(rightActualDescriptor)) { + rd = rightActualDescriptor; + rightNumeric = CodeFlow.isPrimitiveOrUnboxableSupportedNumberOrBoolean(rd); + } + + if (leftNumeric && rightNumeric) { + if (CodeFlow.areBoxingCompatible(ld, rd)) { + return new DescriptorComparison(true, true, CodeFlow.toPrimitiveTargetDesc(ld)); + } + else { + return DescriptorComparison.INCOMPATIBLE_NUMBERS; + } + } + else { + return DescriptorComparison.NOT_NUMBERS; + } + } + + } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 4e380e24bc..22489f29d7 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -2200,8 +2200,230 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { // } // System.out.println((System.currentTimeMillis()-stime)); } + + @Test + public void compilerWithGenerics_12040() { + expression = parser.parseExpression("payload!=2"); + assertTrue(expression.getValue(new GenericMessageTestHelper(4),Boolean.class)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper(2),Boolean.class)); + + expression = parser.parseExpression("2!=payload"); + assertTrue(expression.getValue(new GenericMessageTestHelper(4),Boolean.class)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper(2),Boolean.class)); + + expression = parser.parseExpression("payload!=6L"); + assertTrue(expression.getValue(new GenericMessageTestHelper(4L),Boolean.class)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper(6L),Boolean.class)); + + expression = parser.parseExpression("payload==2"); + assertFalse(expression.getValue(new GenericMessageTestHelper(4),Boolean.class)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper(2),Boolean.class)); + + expression = parser.parseExpression("2==payload"); + assertFalse(expression.getValue(new GenericMessageTestHelper(4),Boolean.class)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper(2),Boolean.class)); + + expression = parser.parseExpression("payload==6L"); + assertFalse(expression.getValue(new GenericMessageTestHelper(4L),Boolean.class)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper(6L),Boolean.class)); + + expression = parser.parseExpression("2==payload"); + assertFalse(expression.getValue(new GenericMessageTestHelper(4),Boolean.class)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper(2),Boolean.class)); + + expression = parser.parseExpression("payload/2"); + assertEquals(2,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(3,expression.getValue(new GenericMessageTestHelper(6))); + + expression = parser.parseExpression("100/payload"); + assertEquals(25,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(10,expression.getValue(new GenericMessageTestHelper(10))); + + expression = parser.parseExpression("payload+2"); + assertEquals(6,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(8,expression.getValue(new GenericMessageTestHelper(6))); + + expression = parser.parseExpression("100+payload"); + assertEquals(104,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(110,expression.getValue(new GenericMessageTestHelper(10))); + + expression = parser.parseExpression("payload-2"); + assertEquals(2,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(4,expression.getValue(new GenericMessageTestHelper(6))); + + expression = parser.parseExpression("100-payload"); + assertEquals(96,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(90,expression.getValue(new GenericMessageTestHelper(10))); + + expression = parser.parseExpression("payload*2"); + assertEquals(8,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(12,expression.getValue(new GenericMessageTestHelper(6))); + + expression = parser.parseExpression("100*payload"); + assertEquals(400,expression.getValue(new GenericMessageTestHelper(4))); + assertCanCompile(expression); + assertEquals(1000,expression.getValue(new GenericMessageTestHelper(10))); + + expression = parser.parseExpression("payload/2L"); + assertEquals(2L,expression.getValue(new GenericMessageTestHelper(4L))); + assertCanCompile(expression); + assertEquals(3L,expression.getValue(new GenericMessageTestHelper(6L))); + + expression = parser.parseExpression("100L/payload"); + assertEquals(25L,expression.getValue(new GenericMessageTestHelper(4L))); + assertCanCompile(expression); + assertEquals(10L,expression.getValue(new GenericMessageTestHelper(10L))); + + expression = parser.parseExpression("payload/2f"); + assertEquals(2f,expression.getValue(new GenericMessageTestHelper(4f))); + assertCanCompile(expression); + assertEquals(3f,expression.getValue(new GenericMessageTestHelper(6f))); + + expression = parser.parseExpression("100f/payload"); + assertEquals(25f,expression.getValue(new GenericMessageTestHelper(4f))); + assertCanCompile(expression); + assertEquals(10f,expression.getValue(new GenericMessageTestHelper(10f))); + + expression = parser.parseExpression("payload/2d"); + assertEquals(2d,expression.getValue(new GenericMessageTestHelper(4d))); + assertCanCompile(expression); + assertEquals(3d,expression.getValue(new GenericMessageTestHelper(6d))); + + expression = parser.parseExpression("100d/payload"); + assertEquals(25d,expression.getValue(new GenericMessageTestHelper(4d))); + assertCanCompile(expression); + assertEquals(10d,expression.getValue(new GenericMessageTestHelper(10d))); + } + + // The new helper class here uses an upper bound on the generic + @Test + public void compilerWithGenerics_12040_2() { + expression = parser.parseExpression("payload/2"); + assertEquals(2,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(3,expression.getValue(new GenericMessageTestHelper2(6))); + + expression = parser.parseExpression("9/payload"); + assertEquals(1,expression.getValue(new GenericMessageTestHelper2(9))); + assertCanCompile(expression); + assertEquals(3,expression.getValue(new GenericMessageTestHelper2(3))); + + expression = parser.parseExpression("payload+2"); + assertEquals(6,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(8,expression.getValue(new GenericMessageTestHelper2(6))); + + expression = parser.parseExpression("100+payload"); + assertEquals(104,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(110,expression.getValue(new GenericMessageTestHelper2(10))); + + expression = parser.parseExpression("payload-2"); + assertEquals(2,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(4,expression.getValue(new GenericMessageTestHelper2(6))); + + expression = parser.parseExpression("100-payload"); + assertEquals(96,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(90,expression.getValue(new GenericMessageTestHelper2(10))); + + expression = parser.parseExpression("payload*2"); + assertEquals(8,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(12,expression.getValue(new GenericMessageTestHelper2(6))); + + expression = parser.parseExpression("100*payload"); + assertEquals(400,expression.getValue(new GenericMessageTestHelper2(4))); + assertCanCompile(expression); + assertEquals(1000,expression.getValue(new GenericMessageTestHelper2(10))); + } + + // The other numeric operators + @Test + public void compilerWithGenerics_12040_3() { + expression = parser.parseExpression("payload >= 2"); + assertTrue(expression.getValue(new GenericMessageTestHelper2(4),Boolean.TYPE)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + + expression = parser.parseExpression("2 >= payload"); + assertFalse(expression.getValue(new GenericMessageTestHelper2(5),Boolean.TYPE)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + + expression = parser.parseExpression("payload > 2"); + assertTrue(expression.getValue(new GenericMessageTestHelper2(4),Boolean.TYPE)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + expression = parser.parseExpression("2 > payload"); + assertFalse(expression.getValue(new GenericMessageTestHelper2(5),Boolean.TYPE)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + + expression = parser.parseExpression("payload <=2"); + assertTrue(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper2(6),Boolean.TYPE)); + + expression = parser.parseExpression("2 <= payload"); + assertFalse(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper2(6),Boolean.TYPE)); + + expression = parser.parseExpression("payload < 2"); + assertTrue(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + assertCanCompile(expression); + assertFalse(expression.getValue(new GenericMessageTestHelper2(6),Boolean.TYPE)); + + expression = parser.parseExpression("2 < payload"); + assertFalse(expression.getValue(new GenericMessageTestHelper2(1),Boolean.TYPE)); + assertCanCompile(expression); + assertTrue(expression.getValue(new GenericMessageTestHelper2(6),Boolean.TYPE)); + } + + // --- + public static class GenericMessageTestHelper { + private T payload; + + GenericMessageTestHelper(T value) { + this.payload = value; + } + + public T getPayload() { + return payload; + } + } + + // This test helper has a bound on the type variable + public static class GenericMessageTestHelper2 { + private T payload; + + GenericMessageTestHelper2(T value) { + this.payload = value; + } + + public T getPayload() { + return payload; + } + } + static class MyAccessor implements CompilablePropertyAccessor { private Method method;