Modified formula parser to encode SUM taking a single argument as tAttrSum (from...

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

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 7a256bf2f7bf9dc6ba852f681a09a2f8a2e2906b..1fe6fa3d720cf1469f8d212bf8aa829dd9d69674 100644 (file)
--- 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 2940af686953a8acd719afde452d4c33039097a7..12da871e6d79b8e3fb36a464fa78a7b34b1d19d2 100644 (file)
--- 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 fe243bf0117bcd01dff3faa6d1769a7f0b042712..1b78cc44df370199219cd8d019b916392d659691 100644 (file)
--- 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 c05ef4475cca6961dffbe446802260a29ee93325..e67e6b302311d795fcfa1c844d7216fea505143a 100644 (file)
--- 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 84034004323704cff8a3502f3cfc2bf3c023203d..6a6e676a1fe67adc1ae123f3d87c517b7347a613 100644 (file)
--- 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;

diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java
index 48be912292a25cba17c74b24bdbf74f8a4c5e92b..fc93a34782899b8e36a10b37242cd7816355924f 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java
+++ b/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
-                       name.setReference("A1:A2,C3");
-               }
+               // 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() {