Modified formula parser to encode SUM taking a single argument as tAttrSum (from comment 7 of bugzilla 46643)

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

src/java/org/apache/poi/hssf/record/formula/AttrPtg.java

@@ -79,8 +79,7 @@ public final class AttrPtg extends ControlPtg {
         _chooseFuncOffset = -1;
     }
 
-    public AttrPtg(LittleEndianInput in) 
+    public AttrPtg(LittleEndianInput in) {
-    {
         field_1_options = in.readByte();
         field_2_data    = in.readShort();
         if (isOptimizedChoose()) {
@@ -113,44 +112,30 @@ public final class AttrPtg extends ControlPtg {
         return new AttrPtg(space.set(0), data, null, -1);
     }
 
-    public void setOptions(byte options)
+    public static AttrPtg getSumSingle() {
-    {
+        return new AttrPtg(sum.set(0), 0, null, -1);
-        field_1_options = options;
     }
 
-    public byte getOptions()
+
-    {
+    public boolean isSemiVolatile() {
-        return field_1_options;
+        return semiVolatile.isSet(field_1_options);
     }
 
-    public boolean isSemiVolatile()
+    public boolean isOptimizedIf() {
-    {
+        return optiIf.isSet(field_1_options);
-        return semiVolatile.isSet(getOptions());
     }
 
-    public boolean isOptimizedIf()
+    public boolean isOptimizedChoose() {
-    {
+        return optiChoose.isSet(field_1_options);
-        return optiIf.isSet(getOptions());
-    }
-
-    public boolean isOptimizedChoose()
-    {
-        return optiChoose.isSet(getOptions());
     }
 
     // lets hope no one uses this anymore
-    public boolean isGoto()
+    public boolean isGoto() {
-    {
+        return optGoto.isSet(field_1_options);
-        return optGoto.isSet(getOptions());
     }
 
-    public boolean isSum()
+    public boolean isSum() {
-    {
+        return sum.isSet(field_1_options);
-        return sum.isSet(getOptions());
-    }
-
-    public void setSum(boolean bsum) {
-        field_1_options=sum.setByteBoolean(field_1_options,bsum);
     }
 
     public void setOptimizedIf(boolean bif) {
@@ -166,24 +151,20 @@ public final class AttrPtg extends ControlPtg {
     }
 
     // lets hope no one uses this anymore
-    public boolean isBaxcel()
+    private boolean isBaxcel() {
-    {
+        return baxcel.isSet(field_1_options);
-        return baxcel.isSet(getOptions());
     }
 
     // biff3&4 only  shouldn't happen anymore
-    public boolean isSpace()
+    public boolean isSpace() {
-    {
+        return space.isSet(field_1_options);
-        return space.isSet(getOptions());
     }
 
-    public void setData(short data)
+    public void setData(short data) {
-    {
         field_2_data = data;
     }
 
-    public short getData()
+    public short getData() {
-    {
         return field_2_data;
     }
 
@@ -227,8 +208,7 @@ public final class AttrPtg extends ControlPtg {
         }
     }
 
-    public int getSize()
+    public int getSize() {
-    {
         if (_jumpTable != null) {
             return SIZE + (_jumpTable.length + 1) * LittleEndian.SHORT_SIZE;
         }
@@ -248,13 +228,11 @@ public final class AttrPtg extends ControlPtg {
     }
 
 
-    public int getNumberOfOperands()
+    public int getNumberOfOperands() {
-    {
         return 1;
     }
 
-    public int getType()
+    public int getType() {
-    {
         return -1;
     }
 
@@ -288,7 +266,7 @@ public final class AttrPtg extends ControlPtg {
         if (_jumpTable == null) {
             jt = null;
         } else {
-            jt = (int[]) _jumpTable.clone();
+            jt = _jumpTable.clone();
         }
         return new AttrPtg(field_1_options, field_2_data, jt, _chooseFuncOffset);
     }

src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java

@@ -20,6 +20,7 @@ package org.apache.poi.hssf.record.formula.eval;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.poi.hssf.record.formula.function.FunctionMetadataRegistry;
 import org.apache.poi.hssf.record.formula.functions.*;
 
 /**
@@ -31,12 +32,14 @@ public abstract class FunctionEval implements OperationEval {
      * Some function IDs that require special treatment
      */
     private static final class FunctionID {
+        /** 4 */
+        public static final int SUM = FunctionMetadataRegistry.FUNCTION_INDEX_SUM;
         /** 78 */
         public static final int OFFSET = 78;
         /** 148 */
         public static final int INDIRECT = 148;
         /** 255 */
-        public static final int EXTERNAL_FUNC = 255;
+        public static final int EXTERNAL_FUNC = FunctionMetadataRegistry.FUNCTION_INDEX_EXTERNAL;
     }
     // convenient access to namespace
     private static final FunctionID ID = null;
@@ -75,7 +78,7 @@ public abstract class FunctionEval implements OperationEval {
         retval[1] = new If(); // IF
         retval[2] = new IsNa(); // ISNA
         retval[3] = new IsError(); // ISERROR
-        retval[4] = AggregateFunction.SUM;
+        retval[ID.SUM] = AggregateFunction.SUM;
         retval[5] = AggregateFunction.AVERAGE;
         retval[6] = AggregateFunction.MIN;
         retval[7] = AggregateFunction.MAX;

src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java

@@ -30,11 +30,12 @@ public final class FunctionMetadataRegistry {
 	 */ 
 	public static final String FUNCTION_NAME_IF = "IF";
 
+	public static final short FUNCTION_INDEX_SUM = 4;
 	public static final short FUNCTION_INDEX_EXTERNAL = 255;
 	private static FunctionMetadataRegistry _instance;
 
 	private final FunctionMetadata[] _functionDataByIndex;
-	private final Map _functionDataByName;
+	private final Map<String, FunctionMetadata> _functionDataByName;
 
 	private static FunctionMetadataRegistry getInstance() {
 		if (_instance == null) {
@@ -43,12 +44,12 @@ public final class FunctionMetadataRegistry {
 		return _instance;
 	}
 
-	/* package */ FunctionMetadataRegistry(FunctionMetadata[] functionDataByIndex, Map functionDataByName) {
+	/* package */ FunctionMetadataRegistry(FunctionMetadata[] functionDataByIndex, Map<String, FunctionMetadata> functionDataByName) {
 		_functionDataByIndex = functionDataByIndex;
 		_functionDataByName = functionDataByName;
 	}
 
-	/* package */ Set getAllFunctionNames() {
+	/* package */ Set<String> getAllFunctionNames() {
 		return _functionDataByName.keySet();
 	}
 
@@ -75,7 +76,7 @@ public final class FunctionMetadataRegistry {
 	}
 
 	private FunctionMetadata getFunctionByNameInternal(String name) {
-		return (FunctionMetadata) _functionDataByName.get(name);
+		return _functionDataByName.get(name);
 	}
 
 

src/java/org/apache/poi/ss/formula/FormulaParser.java

@@ -27,6 +27,7 @@ import org.apache.poi.hssf.record.formula.AddPtg;
 import org.apache.poi.hssf.record.formula.Area3DPtg;
 import org.apache.poi.hssf.record.formula.AreaPtg;
 import org.apache.poi.hssf.record.formula.ArrayPtg;
+import org.apache.poi.hssf.record.formula.AttrPtg;
 import org.apache.poi.hssf.record.formula.BoolPtg;
 import org.apache.poi.hssf.record.formula.ConcatPtg;
 import org.apache.poi.hssf.record.formula.DividePtg;
@@ -592,9 +593,11 @@ public final class FormulaParser {
         }
         boolean isVarArgs = !fm.hasFixedArgsLength();
         int funcIx = fm.getIndex();
-        if (false && funcIx == 4 && args.length == 1) {
+        if (funcIx == FunctionMetadataRegistry.FUNCTION_INDEX_SUM && args.length == 1) {
-            // TODO - make POI behave more like Excel when summing a single argument:
+            // Excel encodes the sum of a single argument as tAttrSum
-            // return new ParseNode(AttrPtg.getSumSingle(), args);
+            // POI does the same for consistency, but this is not critical
+            return new ParseNode(AttrPtg.getSumSingle(), args);
+            // The code below would encode tFuncVar(SUM) which seems to do no harm 
         }
         validateNumArgs(args.length, fm);
 

src/java/org/apache/poi/ss/formula/OperandClassTransformer.java

@@ -18,7 +18,9 @@
 package org.apache.poi.ss.formula;
 
 import org.apache.poi.hssf.record.formula.AbstractFunctionPtg;
+import org.apache.poi.hssf.record.formula.AttrPtg;
 import org.apache.poi.hssf.record.formula.ControlPtg;
+import org.apache.poi.hssf.record.formula.FuncVarPtg;
 import org.apache.poi.hssf.record.formula.MemFuncPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.RangePtg;
@@ -101,6 +103,13 @@ final class OperandClassTransformer {
 			return;
 		}
 		
+		if (isSingleArgSum(token)) {
+			// Need to process the argument of SUM with transformFunctionNode below
+			// so make a dummy FuncVarPtg for that call.
+			token = new FuncVarPtg("SUM", (byte)1);
+			// Note - the tAttrSum token (node.getToken()) is a base 
+			// token so does not need to have its operand class set
+		}
 		if (token instanceof ValueOperatorPtg || token instanceof ControlPtg
 				|| token instanceof MemFuncPtg
 				|| token instanceof UnionPtg) {
@@ -135,6 +144,14 @@ final class OperandClassTransformer {
 		token.setClass(transformClass(token.getPtgClass(), desiredOperandClass, callerForceArrayFlag));
 	}
 
+	private static boolean isSingleArgSum(Ptg token) {
+		if (token instanceof AttrPtg) {
+			AttrPtg attrPtg = (AttrPtg) token;
+			return attrPtg.isSum(); 
+		}
+		return false;
+	}
+
 	private static boolean isSimpleValueFunction(Ptg token) {
 		if (token instanceof AbstractFunctionPtg) {
 			AbstractFunctionPtg aptg = (AbstractFunctionPtg) token;

src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java

@@ -20,7 +20,7 @@ package org.apache.poi.hssf.model;
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
-import org.apache.poi.hssf.record.formula.FuncVarPtg;
+import org.apache.poi.hssf.record.formula.AttrPtg;
 import org.apache.poi.hssf.record.formula.NamePtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.usermodel.HSSFCell;
@@ -58,9 +58,8 @@ public final class TestFormulaParserEval extends TestCase {
 		confirmParseFormula(workbook);
 		
 		// And make it non-contiguous
-		if (false) { // TODO (Nov 2008) - make the formula parser support area unions
+		// using area unions
 		name.setReference("A1:A2,C3");
-		}
 		
 		confirmParseFormula(workbook);
 	}
@@ -72,7 +71,7 @@ public final class TestFormulaParserEval extends TestCase {
 		Ptg[] ptgs = HSSFFormulaParser.parse("SUM(testName)", workbook);
 		assertTrue("two tokens expected, got "+ptgs.length,ptgs.length == 2);
 		assertEquals(NamePtg.class, ptgs[0].getClass());
-		assertEquals(FuncVarPtg.class, ptgs[1].getClass());
+		assertEquals(AttrPtg.class, ptgs[1].getClass());
 	}
 
 	public void testEvaluateFormulaWithRowBeyond32768_Bug44539() {