<!-- 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>
<!-- 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>
} 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()
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 == ')';
}
}
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) {
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();
}
}
_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);
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;
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(" [");
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>
*
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.
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 + "'");
}
/**
HSSFWorkbook book = new HSSFWorkbook();
Ptg[] ptgs = {
- new FuncPtg(10, 0),
+ new FuncPtg(10),
};
assertEquals("NA()", FormulaParser.toFormulaString(book, ptgs));
}
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());
+ }
+ }
}
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);
--- /dev/null
+/* ====================================================================
+ 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");
+ }
+ }
+}