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

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>

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>

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) {
+		if (isDirectChildOfValueOperator) {
-        	switch (desiredOperandClass) {
+			// As per OOO documentation Sec 3.2.4 "Token Class Transformation", "Step 1"
-        		case Ptg.CLASS_VALUE:
+			// All direct operands of value operators that are initially 'R' type will 
-        		case Ptg.CLASS_ARRAY:
+			// be converted to 'V' type.
-        			token.setClass(Ptg.CLASS_ARRAY); 
+			if (token.getPtgClass() == Ptg.CLASS_REF) {
-        			break;
+				token.setClass(Ptg.CLASS_VALUE); 
-        		case Ptg.CLASS_REF:
+			}
-        			token.setClass(Ptg.CLASS_REF); 
+		}
-        			break;
+		token.setClass(transformClass(token.getPtgClass(), desiredOperandClass, callerForceArrayFlag));
-        		default:
+	}
-        			throw new IllegalStateException("Unexpected operand class ("
+
-        					+ desiredOperandClass + ")");
+	private byte transformClass(byte currentOperandClass, byte desiredOperandClass,
-        	}
+			boolean callerForceArrayFlag) {
-        } else {
+		switch (desiredOperandClass) {
-        	token.setClass(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);
 		}
 	}
 }

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()));
 		}