]> source.dussan.org Git - poi.git/commitdiff
Fixed 44675 - Parameter operand classes (function metadata) required to encode SUM...
authorJosh Micich <josh@apache.org>
Sat, 3 May 2008 03:59:32 +0000 (03:59 +0000)
committerJosh Micich <josh@apache.org>
Sat, 3 May 2008 03:59:32 +0000 (03:59 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@652994 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml
src/documentation/content/xdocs/status.xml
src/java/org/apache/poi/hssf/model/FormulaParser.java
src/java/org/apache/poi/hssf/record/formula/FuncPtg.java
src/java/org/apache/poi/hssf/record/formula/FuncVarPtg.java
src/java/org/apache/poi/hssf/record/formula/function/FunctionDataBuilder.java
src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadata.java
src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataReader.java
src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java
src/testcases/org/apache/poi/hssf/record/formula/AllFormulaTests.java
src/testcases/org/apache/poi/hssf/record/formula/TestFuncVarPtg.java [new file with mode: 0644]

index e0fdf3966d85b051baee2b697331204c9aff8843..ce08eff1a9fb27a79d87330e71fef371982c6dc6 100644 (file)
@@ -37,6 +37,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters</action>
            <action dev="POI-DEVELOPERS" type="fix">44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*)</action>
            <action dev="POI-DEVELOPERS" type="fix">44914 - Fix/suppress warning message "WARN. Unread n bytes of record 0xNN"</action>
            <action dev="POI-DEVELOPERS" type="fix">44892 - made HSSFWorkbook.getSheet(String) case insensitive</action>
index f1a5cd1d8e3384bd0fb9c0e455d8c4c52f7be449..7e4277acb083d0cbdf04464ec5ce587407e2e6c4 100644 (file)
@@ -34,6 +34,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">44675 - Parameter operand classes (function metadata) required to encode SUM() etc properly. Added parse validation for number of parameters</action>
            <action dev="POI-DEVELOPERS" type="fix">44921 - allow Ptg.writeBytes() to be called on relative ref Ptgs (RefN* and AreaN*)</action>
            <action dev="POI-DEVELOPERS" type="fix">44914 - Fix/suppress warning message "WARN. Unread n bytes of record 0xNN"</action>
            <action dev="POI-DEVELOPERS" type="fix">44892 - made HSSFWorkbook.getSheet(String) case insensitive</action>
index 05b4d80f4a2b57f397a074419f8c5427a6959aea..f16ab451294805967c0f8d4c3691f69c00b8b3b2 100644 (file)
@@ -380,12 +380,13 @@ public final class FormulaParser {
         } else {
             isVarArgs = !fm.hasFixedArgsLength();
             funcIx = fm.getIndex();
+            validateNumArgs(numArgs, fm);
         }
         AbstractFunctionPtg retval;
         if(isVarArgs) {
             retval = new FuncVarPtg(name, (byte)numArgs);
         } else {
-            retval = new FuncPtg(funcIx, (byte)numArgs);
+            retval = new FuncPtg(funcIx);
         }
         if (!name.equals(AbstractFunctionPtg.FUNCTION_NAME_IF)) {
             // early return for everything else besides IF()
@@ -447,6 +448,29 @@ public final class FormulaParser {
         return retval;
     }
 
+    private void validateNumArgs(int numArgs, FunctionMetadata fm) {
+        if(numArgs < fm.getMinParams()) {
+            String msg = "Too few arguments to function '" + fm.getName() + "'. ";
+            if(fm.hasFixedArgsLength()) {
+                msg += "Expected " + fm.getMinParams();
+            } else {
+                msg += "At least " + fm.getMinParams() + " were expected";
+            }
+            msg += " but got " + numArgs + ".";
+            throw new FormulaParseException(msg);
+         }
+        if(numArgs > fm.getMaxParams()) {
+            String msg = "Too many arguments to function '" + fm.getName() + "'. ";
+            if(fm.hasFixedArgsLength()) {
+                msg += "Expected " + fm.getMaxParams();
+            } else {
+                msg += "At most " + fm.getMaxParams() + " were expected";
+            }
+            msg += " but got " + numArgs + ".";
+            throw new FormulaParseException(msg);
+       }
+    }
+
     private static boolean isArgumentDelimiter(char ch) {
         return ch ==  ',' || ch == ')';
     }
index 69edf88aae0ca05c2e9d0e7ccea3c7172467ef40..7901fb675d30aaf91eaf3c349b576882e9f3bcb4 100644 (file)
@@ -57,10 +57,12 @@ public final class FuncPtg extends AbstractFunctionPtg {
         }
         numParams = fm.getMinParams();
     }
-    public FuncPtg(int functionIndex, int numberOfParameters) {
+    public FuncPtg(int functionIndex) {
         field_2_fnc_index = (short) functionIndex;
-        numParams = numberOfParameters;
-        paramClass = new byte[] { Ptg.CLASS_VALUE, }; // TODO
+        FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(functionIndex);
+        numParams = fm.getMinParams(); // same as max since these are not var-arg funcs
+        returnClass = fm.getReturnClassCode();
+        paramClass = fm.getParameterClassCodes();
     }
 
     public void writeBytes(byte[] array, int offset) {
index fb652713933d09b40fe64b0be60859cdf385c288..431dc5717b820969f231bf436ee9a5aab4139b97 100644 (file)
@@ -54,8 +54,8 @@ public final class FuncVarPtg extends AbstractFunctionPtg{
             returnClass = Ptg.CLASS_VALUE;
             paramClass = new byte[] {Ptg.CLASS_VALUE};
         } else {
-            returnClass = Ptg.CLASS_VALUE;
-            paramClass = new byte[] {Ptg.CLASS_VALUE};
+            returnClass = fm.getReturnClassCode();
+            paramClass = fm.getParameterClassCodes();
         }
     }
 
index 2009ebae1ff4793feaa7139d53ece759cb07511c..789be63240e4e4068200592f9a6af2248f58eaef 100644 (file)
@@ -42,8 +42,10 @@ final class FunctionDataBuilder {
                _mutatingFunctionIndexes = new HashSet();
        }
 
-       public void add(int functionIndex, String functionName, int minParams, int maxParams, boolean hasFootnote) {
-               FunctionMetadata fm = new FunctionMetadata(functionIndex, functionName, minParams, maxParams);
+       public void add(int functionIndex, String functionName, int minParams, int maxParams,
+                       byte returnClassCode, byte[] parameterClassCodes, boolean hasFootnote) {
+               FunctionMetadata fm = new FunctionMetadata(functionIndex, functionName, minParams, maxParams,
+                               returnClassCode, parameterClassCodes);
                
                Integer indexKey = new Integer(functionIndex);
                
index 94f1659b5128bf11d7a561e5527fbc41d6b3de3b..fc5f845437a76bcb767e2eb4e6addc0c556dac7a 100644 (file)
@@ -27,12 +27,17 @@ public final class FunctionMetadata {
        private final String _name;
        private final int _minParams;
        private final int _maxParams;
+       private final byte _returnClassCode;
+       private final byte[] _parameterClassCodes;
 
-       /* package */ FunctionMetadata(int index, String name, int minParams, int maxParams) {
+       /* package */ FunctionMetadata(int index, String name, int minParams, int maxParams,
+                       byte returnClassCode, byte[] parameterClassCodes) {
                _index = index;
                _name = name;
                _minParams = minParams;
                _maxParams = maxParams;
+               _returnClassCode = returnClassCode;
+               _parameterClassCodes = parameterClassCodes;
        }
        public int getIndex() {
                return _index;
@@ -49,6 +54,12 @@ public final class FunctionMetadata {
        public boolean hasFixedArgsLength() {
                return _minParams == _maxParams;
        }
+       public byte getReturnClassCode() {
+               return _returnClassCode;
+       }
+       public byte[] getParameterClassCodes() {
+               return (byte[]) _parameterClassCodes.clone();
+       }
        public String toString() {
                StringBuffer sb = new StringBuffer(64);
                sb.append(getClass().getName()).append(" [");
index bd72a9223374ff4b71e80f6310bcc74b96864f24..2cdc540e6f22fb922422e4a56b63ce7ac5a4a965 100644 (file)
@@ -26,6 +26,8 @@ import java.util.HashSet;
 import java.util.Set;
 import java.util.regex.Pattern;
 
+import org.apache.poi.hssf.record.formula.Ptg;
+
 /**
  * Converts the text meta-data file into a <tt>FunctionMetadataRegistry</tt>
  * 
@@ -36,6 +38,12 @@ final class FunctionMetadataReader {
        private static final String METADATA_FILE_NAME = "functionMetadata.txt";
 
        private static final Pattern TAB_DELIM_PATTERN = Pattern.compile("\t");
+       private static final Pattern SPACE_DELIM_PATTERN = Pattern.compile(" ");
+       private static final byte[] EMPTY_BYTE_ARRAY = { };
+
+       // special characters from the ooo document
+       private static final int CHAR_ELLIPSIS_8230 = 8230;
+       private static final int CHAR_NDASH_8211 = 8211;
        
        private static final String[] DIGIT_ENDING_FUNCTION_NAMES = {
                // Digits at the end of a function might be due to a left-over footnote marker.
@@ -86,14 +94,66 @@ final class FunctionMetadataReader {
                String functionName = parts[1];
                int minParams = parseInt(parts[2]);
                int maxParams = parseInt(parts[3]);
-               // 4 returnClass
-               // 5 parameterClasses
+               byte returnClassCode = parseReturnTypeCode(parts[4]);
+               byte[] parameterClassCodes = parseOperandTypeCodes(parts[5]);
                // 6 isVolatile
                boolean hasNote = parts[7].length() > 0;
 
                validateFunctionName(functionName);
-               // TODO - make POI use returnClass, parameterClasses, isVolatile
-               fdb.add(functionIndex, functionName, minParams, maxParams, hasNote);
+               // TODO - make POI use isVolatile
+               fdb.add(functionIndex, functionName, minParams, maxParams, 
+                               returnClassCode, parameterClassCodes, hasNote);
+       }
+       
+
+       private static byte parseReturnTypeCode(String code) {
+               if(code.length() == 0) {
+                       return Ptg.CLASS_REF; // happens for GETPIVOTDATA
+               }
+               return parseOperandTypeCode(code);
+       }
+
+       private static byte[] parseOperandTypeCodes(String codes) {
+               if(codes.length() < 1) {
+                       return EMPTY_BYTE_ARRAY; // happens for GETPIVOTDATA
+               }
+               if(isDash(codes)) {
+                       // '-' means empty:
+                       return EMPTY_BYTE_ARRAY;
+               }
+               String[] array = SPACE_DELIM_PATTERN.split(codes);
+               int nItems = array.length;
+               if(array[nItems-1].charAt(0) == CHAR_ELLIPSIS_8230) {
+                       nItems --;
+               }
+               byte[] result = new byte[nItems];
+               for (int i = 0; i < nItems; i++) {
+                       result[i] = parseOperandTypeCode(array[i]);
+               }
+               return result;
+       }
+
+       private static boolean isDash(String codes) {
+               if(codes.length() == 1) {
+                       switch (codes.charAt(0)) {
+                               case '-':
+                               case CHAR_NDASH_8211: // this is what the ooo doc has
+                                       return true;
+                       }
+               }
+               return false;
+       }
+
+       private static byte parseOperandTypeCode(String code) {
+               if(code.length() != 1) {
+                       throw new RuntimeException("Bad operand type code format '" + code  + "' expected single char");
+               }
+               switch(code.charAt(0)) {
+                       case 'V': return Ptg.CLASS_VALUE;
+                       case 'R': return Ptg.CLASS_REF;
+                       case 'A': return Ptg.CLASS_ARRAY;
+               }
+               throw new IllegalArgumentException("Unexpected operand type code '" + code + "'");
        }
 
        /**
index 31b4dd180abbe4e9b07c6e9f33913a36275d5770..9ce862b9b69b9de66bda790bc6d3dd2ea32a37bb 100644 (file)
@@ -592,7 +592,7 @@ public final class TestFormulaParser extends TestCase {
        HSSFWorkbook book = new HSSFWorkbook();
 
         Ptg[] ptgs = {
-                new FuncPtg(10, 0),
+                new FuncPtg(10),
         };
         assertEquals("NA()", FormulaParser.toFormulaString(book, ptgs));
     }
@@ -900,4 +900,21 @@ public final class TestFormulaParser extends TestCase {
         assertEquals(2, ptgs.length);
         assertEquals(FuncPtg.class, ptgs[1].getClass());
     }
+    
+    public void testWrongNumberOfFunctionArgs() {
+        confirmArgCountMsg("sin()", "Too few arguments to function 'SIN'. Expected 1 but got 0.");
+        confirmArgCountMsg("countif(1, 2, 3, 4)", "Too many arguments to function 'COUNTIF'. Expected 2 but got 4.");
+        confirmArgCountMsg("index(1, 2, 3, 4, 5, 6)", "Too many arguments to function 'INDEX'. At most 4 were expected but got 6.");
+        confirmArgCountMsg("vlookup(1, 2)", "Too few arguments to function 'VLOOKUP'. At least 3 were expected but got 2.");
+    }
+
+    private static void confirmArgCountMsg(String formula, String expectedMessage) {
+        HSSFWorkbook book = new HSSFWorkbook();
+        try {
+            FormulaParser.parse(formula, book);
+            throw new AssertionFailedError("Didn't get parse exception as expected");
+        } catch (FormulaParseException e) {
+            assertEquals(expectedMessage, e.getMessage());
+        }
+    }
 }
index 3a704502230367f8bc7a7fcaebd254415cf076cf..80f6616fa6b982dd4dff6144574298814ea658e6 100644 (file)
@@ -43,6 +43,7 @@ public final class AllFormulaTests {
                result.addTestSuite(TestErrPtg.class);
                result.addTestSuite(TestExternalFunctionFormulas.class);
                result.addTestSuite(TestFuncPtg.class);
+               result.addTestSuite(TestFuncVarPtg.class);
                result.addTestSuite(TestIntersectionPtg.class);
                result.addTestSuite(TestPercentPtg.class);
                result.addTestSuite(TestRangePtg.class);
diff --git a/src/testcases/org/apache/poi/hssf/record/formula/TestFuncVarPtg.java b/src/testcases/org/apache/poi/hssf/record/formula/TestFuncVarPtg.java
new file mode 100644 (file)
index 0000000..58ae55f
--- /dev/null
@@ -0,0 +1,53 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.record.formula;
+
+import org.apache.poi.hssf.model.FormulaParser;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+/**
+ * @author Josh Micich
+ */
+public final class TestFuncVarPtg extends TestCase {
+
+       /**
+        * The first fix for bugzilla 44675 broke the encoding of SUM formulas (and probably others).
+        * The operand classes of the parameters to SUM() should be coerced to 'reference' not 'value'.
+        * In the case of SUM, Excel evaluates the formula to '#VALUE!' if a parameter operand class is
+        * wrong.  In other cases Excel seems to tolerate bad operand classes.</p>
+        * This functionality is related to the setParameterRVA() methods of <tt>FormulaParser</tt>
+        */
+       public void testOperandClass() {
+               HSSFWorkbook book = new HSSFWorkbook();
+               Ptg[] ptgs = FormulaParser.parse("sum(A1:A2)", book);
+               assertEquals(2, ptgs.length);
+               assertEquals(AreaPtg.class, ptgs[0].getClass());
+               
+               switch(ptgs[0].getPtgClass()) {
+                       case Ptg.CLASS_REF:
+                               // correct behaviour
+                               break;
+                       case Ptg.CLASS_VALUE:
+                               throw new AssertionFailedError("Identified bug 44675b");
+                       default:
+                               throw new RuntimeException("Unexpected operand class");
+               }
+       }
+}