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

5 files changed, 58 insertions, 56 deletions

diff --git a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java
index 7a256bf2f7..1fe6fa3d72 100644
--- a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java
+++ b/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)
-    {
-        field_1_options = options;
+    public static AttrPtg getSumSingle() {
+        return new AttrPtg(sum.set(0), 0, null, -1);
     }
 
-    public byte getOptions()
-    {
-        return field_1_options;
-    }
 
-    public boolean isSemiVolatile()
-    {
-        return semiVolatile.isSet(getOptions());
+    public boolean isSemiVolatile() {
+        return semiVolatile.isSet(field_1_options);
     }
 
-    public boolean isOptimizedIf()
-    {
-        return optiIf.isSet(getOptions());
+    public boolean isOptimizedIf() {
+        return optiIf.isSet(field_1_options);
     }
 
-    public boolean isOptimizedChoose()
-    {
-        return optiChoose.isSet(getOptions());
+    public boolean isOptimizedChoose() {
+        return optiChoose.isSet(field_1_options);
     }
 
     // lets hope no one uses this anymore
-    public boolean isGoto()
-    {
-        return optGoto.isSet(getOptions());
-    }
-
-    public boolean isSum()
-    {
-        return sum.isSet(getOptions());
+    public boolean isGoto() {
+        return optGoto.isSet(field_1_options);
     }
 
-    public void setSum(boolean bsum) {
-        field_1_options=sum.setByteBoolean(field_1_options,bsum);
+    public boolean isSum() {
+        return sum.isSet(field_1_options);
     }
 
     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()
-    {
-        return baxcel.isSet(getOptions());
+    private boolean isBaxcel() {
+        return baxcel.isSet(field_1_options);
     }
 
     // biff3&4 only  shouldn't happen anymore
-    public boolean isSpace()
-    {
-        return space.isSet(getOptions());
+    public boolean isSpace() {
+        return space.isSet(field_1_options);
     }
 
-    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;
         }
@@ -239,22 +219,20 @@ public final class AttrPtg extends ControlPtg {
         if(space.isSet(field_1_options)) {
             return operands[ 0 ];
         } else if (optiIf.isSet(field_1_options)) {
-            return toFormulaString() + "(" + operands[ 0 ]             +")";
+            return toFormulaString() + "(" + operands[0] + ")";
         } else if (optGoto.isSet(field_1_options)) {
             return toFormulaString() + operands[0];   //goto isn't a real formula element should not show up
         } else {
-            return toFormulaString() + "(" + operands[ 0 ] + ")";
+            return toFormulaString() + "(" + operands[0] + ")";
         }
     }
 
 
-    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);
     }
diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java
index 2940af6869..12da871e6d 100644
--- a/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java
+++ b/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;
diff --git a/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java b/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java
index fe243bf011..1b78cc44df 100644
--- a/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java
+++ b/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);
 	}
 
 
diff --git a/src/java/org/apache/poi/ss/formula/FormulaParser.java b/src/java/org/apache/poi/ss/formula/FormulaParser.java
index c05ef4475c..e67e6b3023 100644
--- a/src/java/org/apache/poi/ss/formula/FormulaParser.java
+++ b/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) {
-            // TODO - make POI behave more like Excel when summing a single argument:
-            // return new ParseNode(AttrPtg.getSumSingle(), args);
+        if (funcIx == FunctionMetadataRegistry.FUNCTION_INDEX_SUM && args.length == 1) {
+            // Excel encodes the sum of a single argument as tAttrSum
+            // 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);
 
diff --git a/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java b/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java
index 8403400432..6a6e676a1f 100644
--- a/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java
+++ b/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;