Fix for bug 45348 - required tweaks to RVA formula logic

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@675079 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index 54bfcfee85da6c4c87dae176bf830b09e6d286cb..2507fafbeaaec8dadb7e047fa838c48bc6ebe210 100644 (file)
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -37,6 +37,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45348 - Tweaks to RVA formula logic</action>
            <action dev="POI-DEVELOPERS" type="fix">45354 - Fixed recognition of named ranges within formulas</action>
            <action dev="POI-DEVELOPERS" type="fix">45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts</action>
            <action dev="POI-DEVELOPERS" type="fix">45336 - Fix HSSFColor.getTripletHash()</action>

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index bde398a9c4991d9c37a4bfbc2ca6edcdc2c790b8..013b3b3bd1a8f6f3beae65b7ab974dda863e4bda 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45348 - Tweaks to RVA formula logic</action>
            <action dev="POI-DEVELOPERS" type="fix">45354 - Fixed recognition of named ranges within formulas</action>
            <action dev="POI-DEVELOPERS" type="fix">45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts</action>
            <action dev="POI-DEVELOPERS" type="fix">45336 - Fix HSSFColor.getTripletHash()</action>

diff --git a/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java b/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java
index 5358324a38e1ad44a9b89ce409fc8da7f8a59201..07d2bd2fd623c343034a29462b5634a09fd870e6 100644 (file)
--- a/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java
+++ b/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java
@@ -71,11 +71,16 @@ final class OperandClassTransformer {
                                                + _formulaType + ") not supported yet");
                
                }
-               transformNode(rootNode, rootNodeOperandClass, false);
+               transformNode(rootNode, rootNodeOperandClass, false, false);
        }
 
+       /**
+        * @param callerForceArrayFlag <code>true</code> if one of the current node's parents is a 
+        * function Ptg which has been changed from default 'V' to 'A' type (due to requirements on
+        * the function return value).
+        */
        private void transformNode(ParseNode node, byte desiredOperandClass,
-                       boolean callerForceArrayFlag) {
+                       boolean callerForceArrayFlag, boolean isDirectChildOfValueOperator) {
                Ptg token = node.getToken();
                ParseNode[] children = node.getChildren();
                if (token instanceof ValueOperatorPtg || token instanceof ControlPtg) {
@@ -84,7 +89,7 @@ final class OperandClassTransformer {
                        // but any child nodes are processed according to desiredOperandClass and callerForceArrayFlag
                        for (int i = 0; i < children.length; i++) {
                                ParseNode child = children[i];
-                               transformNode(child, desiredOperandClass, callerForceArrayFlag);
+                               transformNode(child, desiredOperandClass, callerForceArrayFlag, true);
                        }
                        return;
                }
@@ -101,22 +106,34 @@ final class OperandClassTransformer {
                        // nothing to do
                        return;
                }
-        if (callerForceArrayFlag) {
-               switch (desiredOperandClass) {
-                       case Ptg.CLASS_VALUE:
-                       case Ptg.CLASS_ARRAY:
-                               token.setClass(Ptg.CLASS_ARRAY); 
-                               break;
-                       case Ptg.CLASS_REF:
-                               token.setClass(Ptg.CLASS_REF); 
-                               break;
-                       default:
-                               throw new IllegalStateException("Unexpected operand class ("
-                                               + desiredOperandClass + ")");
-               }
-        } else {
-               token.setClass(desiredOperandClass);
-        }
+               if (isDirectChildOfValueOperator) {
+                       // As per OOO documentation Sec 3.2.4 "Token Class Transformation", "Step 1"
+                       // All direct operands of value operators that are initially 'R' type will 
+                       // be converted to 'V' type.
+                       if (token.getPtgClass() == Ptg.CLASS_REF) {
+                               token.setClass(Ptg.CLASS_VALUE); 
+                       }
+               }
+               token.setClass(transformClass(token.getPtgClass(), desiredOperandClass, callerForceArrayFlag));
+       }
+
+       private byte transformClass(byte currentOperandClass, byte desiredOperandClass,
+                       boolean callerForceArrayFlag) {
+               switch (desiredOperandClass) {
+                       case Ptg.CLASS_VALUE:
+                               if (!callerForceArrayFlag) {
+                                       return Ptg.CLASS_VALUE;
+                               }
+                               // else fall through
+                       case Ptg.CLASS_ARRAY:
+                               return Ptg.CLASS_ARRAY; 
+                       case Ptg.CLASS_REF:
+                               if (!callerForceArrayFlag) {
+                                       return currentOperandClass;
+                               }
+                               return Ptg.CLASS_REF; 
+               }
+               throw new IllegalStateException("Unexpected operand class (" + desiredOperandClass + ")");
        }
 
        private void transformFunctionNode(AbstractFunctionPtg afp, ParseNode[] children,
@@ -200,7 +217,7 @@ final class OperandClassTransformer {
                for (int i = 0; i < children.length; i++) {
                        ParseNode child = children[i];
                        byte paramOperandClass = afp.getParameterClass(i);
-                       transformNode(child, paramOperandClass, localForceArrayFlag);
+                       transformNode(child, paramOperandClass, localForceArrayFlag, false);
                }
        }
 }

diff --git a/src/testcases/org/apache/poi/hssf/data/testRVA.xls b/src/testcases/org/apache/poi/hssf/data/testRVA.xls
index f23821117e96c2c5e9649a8c5227748bb519ae5a..17aa9fd7108c2f6cf471f36780a549725365e6c4 100644 (file)
Binary files a/src/testcases/org/apache/poi/hssf/data/testRVA.xls and b/src/testcases/org/apache/poi/hssf/data/testRVA.xls differ

diff --git a/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java b/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java
index 90a5d8b13fd69fad082263fab7b0594d4fce730c..47cfb574c1dd29ee093f89db321696420d081cc6 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java
+++ b/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java
@@ -57,6 +57,31 @@ public final class TestOperandClassTransformer extends TestCase {
                confirmFuncClass(ptgs, 2, "INDEX", Ptg.CLASS_VALUE);
        }
 
+       /**
+        * Even though count expects args of type R, because A1 is a direct operand of a
+        * value operator it must get type V
+        */
+       public void testDirectOperandOfValueOperator() {
+               String formula = "COUNT(A1*1)";
+               Ptg[] ptgs = FormulaParser.parse(formula, null);
+               if (ptgs[0].getPtgClass() == Ptg.CLASS_REF) {
+                       throw new AssertionFailedError("Identified bug 45348");
+               }
+
+               confirmTokenClass(ptgs, 0, Ptg.CLASS_VALUE);
+               confirmTokenClass(ptgs, 3, Ptg.CLASS_VALUE);
+       }
+       
+       /**
+        * A cell ref passed to a function expecting type V should be converted to type V
+        */
+       public void testRtoV() {
+
+               String formula = "lookup(A1, A3:A52, B3:B52)";
+               Ptg[] ptgs = FormulaParser.parse(formula, null);
+               confirmTokenClass(ptgs, 0, Ptg.CLASS_VALUE);
+       }
+       
        public void testComplexIRR_bug45041() {
                String formula = "(1+IRR(SUMIF(A:A,ROW(INDIRECT(MIN(A:A)&\":\"&MAX(A:A))),B:B),0))^365-1";
                Ptg[] ptgs = FormulaParser.parse(formula, null);
@@ -89,8 +114,11 @@ public final class TestOperandClassTransformer extends TestCase {
 
        private void confirmTokenClass(Ptg[] ptgs, int i, byte operandClass) {
                Ptg ptg = ptgs[i];
+               if (ptg.isBaseToken()) {
+                       throw new AssertionFailedError("ptg[" + i + "] is a base token");
+               }
                if (operandClass != ptg.getPtgClass()) {
-                       throw new AssertionFailedError("Wrong operand class for function ptg ("
+                       throw new AssertionFailedError("Wrong operand class for ptg ("
                                        + ptg.toString() + "). Expected " + getOperandClassName(operandClass)
                                        + " but got " + getOperandClassName(ptg.getPtgClass()));
                }