You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by on...@apache.org on 2015/10/31 12:57:40 UTC
svn commit: r1711605 - in /poi/trunk: src/java/org/apache/poi/ss/formula/
src/ooxml/testcases/org/apache/poi/ss/formula/
src/testcases/org/apache/poi/hssf/model/
src/testcases/org/apache/poi/ss/formula/
src/testcases/org/apache/poi/ss/usermodel/ test-d...
Author: onealj
Date: Sat Oct 31 11:57:39 2015
New Revision: 1711605
URL: http://svn.apache.org/viewvc?rev=1711605&view=rev
Log:
bug58452: set cell formulas containing unregistered function names
Added:
poi/trunk/test-data/spreadsheet/testNames.xlsm (with props)
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/FormulaParser.java
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java
poi/trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java
poi/trunk/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/FormulaParser.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/FormulaParser.java?rev=1711605&r1=1711604&r2=1711605&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/FormulaParser.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/FormulaParser.java Sat Oct 31 11:57:39 2015
@@ -50,6 +50,7 @@ import org.apache.poi.ss.formula.ptg.Mis
import org.apache.poi.ss.formula.ptg.MultiplyPtg;
import org.apache.poi.ss.formula.ptg.NamePtg;
import org.apache.poi.ss.formula.ptg.NameXPtg;
+import org.apache.poi.ss.formula.ptg.NameXPxg;
import org.apache.poi.ss.formula.ptg.NotEqualPtg;
import org.apache.poi.ss.formula.ptg.NumberPtg;
import org.apache.poi.ss.formula.ptg.OperandPtg;
@@ -67,9 +68,12 @@ import org.apache.poi.ss.formula.ptg.Una
import org.apache.poi.ss.formula.ptg.UnionPtg;
import org.apache.poi.ss.formula.ptg.ValueOperatorPtg;
import org.apache.poi.ss.usermodel.ErrorConstants;
+import org.apache.poi.ss.usermodel.Name;
import org.apache.poi.ss.util.AreaReference;
import org.apache.poi.ss.util.CellReference;
import org.apache.poi.ss.util.CellReference.NameType;
+import org.apache.poi.util.POILogFactory;
+import org.apache.poi.util.POILogger;
/**
* This class parses a formula string into a List of tokens in RPN order.
@@ -85,6 +89,7 @@ import org.apache.poi.ss.util.CellRefere
* <p/>
*/
public final class FormulaParser {
+ private final static POILogger log = POILogFactory.getLogger(FormulaParser.class);
private final String _formulaString;
private final int _formulaLength;
/** points at the next character to be read (after the {@link #look} char) */
@@ -108,10 +113,10 @@ public final class FormulaParser {
*/
private boolean _inIntersection = false;
- private FormulaParsingWorkbook _book;
- private SpreadsheetVersion _ssVersion;
+ private final FormulaParsingWorkbook _book;
+ private final SpreadsheetVersion _ssVersion;
- private int _sheetIndex;
+ private final int _sheetIndex;
/**
@@ -137,6 +142,7 @@ public final class FormulaParser {
/**
* Parse a formula into a array of tokens
+ * Side effect: creates name (Workbook.createName) if formula contains unrecognized names (names are likely UDFs)
*
* @param formula the formula to parse
* @param workbook the parent workbook
@@ -927,6 +933,8 @@ public final class FormulaParser {
* Note - Excel function names are 'case aware but not case sensitive'. This method may end
* up creating a defined name record in the workbook if the specified name is not an internal
* Excel function, and has not been encountered before.
+ *
+ * Side effect: creates workbook name if name is not recognized (name is probably a UDF)
*
* @param name case preserved function name (as it was entered/appeared in the formula).
*/
@@ -940,22 +948,42 @@ public final class FormulaParser {
// Only test cases omit the book (expecting it not to be needed)
throw new IllegalStateException("Need book to evaluate name '" + name + "'");
}
+ // Check to see if name is a named range in the workbook
EvaluationName hName = _book.getName(name, _sheetIndex);
- if (hName == null) {
- nameToken = _book.getNameXPtg(name, null);
- if (nameToken == null) {
- throw new FormulaParseException("Name '" + name
- + "' is completely unknown in the current workbook");
- }
- } else {
+ if (hName != null) {
if (!hName.isFunctionName()) {
throw new FormulaParseException("Attempt to use name '" + name
+ "' as a function, but defined name in workbook does not refer to a function");
}
-
+
// calls to user-defined functions within the workbook
// get a Name token which points to a defined name record
nameToken = hName.createPtg();
+ } else {
+ // Check if name is an external names table
+ nameToken = _book.getNameXPtg(name, null);
+ if (nameToken == null) {
+ // name is not an internal or external name
+ if (log.check(POILogger.WARN)) {
+ log.log(POILogger.WARN,
+ "FormulaParser.function: Name '" + name + "' is completely unknown in the current workbook.");
+ }
+ // name is probably the name of an unregistered User-Defined Function
+ switch (_book.getSpreadsheetVersion()) {
+ case EXCEL97:
+ // HSSFWorkbooks require a name to be added to Workbook defined names table
+ addName(name);
+ hName = _book.getName(name, _sheetIndex);
+ nameToken = hName.createPtg();
+ break;
+ case EXCEL2007:
+ // XSSFWorkbooks store formula names as strings.
+ nameToken = new NameXPxg(name);
+ break;
+ default:
+ throw new IllegalStateException("Unexpected spreadsheet version: " + _book.getSpreadsheetVersion().name());
+ }
+ }
}
}
@@ -965,6 +993,17 @@ public final class FormulaParser {
return getFunction(name, nameToken, args);
}
+
+ /**
+ * Adds a name (named range or user defined function) to underlying workbook's names table
+ * @param functionName
+ */
+ private final void addName(String functionName) {
+ final Name name = _book.createName();
+ name.setFunction(true);
+ name.setNameName(functionName);
+ name.setSheetIndex(_sheetIndex);
+ }
/**
* Generates the variable function ptg for the formula.
Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java?rev=1711605&r1=1711604&r2=1711605&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java Sat Oct 31 11:57:39 2015
@@ -18,8 +18,17 @@
*/
package org.apache.poi.ss.formula;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.util.Locale;
+
import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.ss.formula.ptg.AbstractFunctionPtg;
+import org.apache.poi.ss.formula.ptg.NameXPxg;
+import org.apache.poi.ss.formula.ptg.Ptg;
+import org.apache.poi.ss.formula.ptg.StringPtg;
+import org.apache.poi.xssf.XSSFTestDataSamples;
import org.apache.poi.xssf.usermodel.XSSFEvaluationWorkbook;
import org.apache.poi.xssf.usermodel.XSSFWorkbook;
@@ -62,5 +71,92 @@ public class TestFormulaParser extends T
catch (FormulaParseException expected) {
}
}
+
+ // copied from org.apache.poi.hssf.model.TestFormulaParser
+ public void testMacroFunction() throws Exception {
+ // testNames.xlsm contains a VB function called 'myFunc'
+ final String testFile = "testNames.xlsm";
+ XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook(testFile);
+ try {
+ XSSFEvaluationWorkbook workbook = XSSFEvaluationWorkbook.create(wb);
+
+ //Expected ptg stack: [NamePtg(myFunc), StringPtg(arg), (additional operands would go here...), FunctionPtg(myFunc)]
+ Ptg[] ptg = FormulaParser.parse("myFunc(\"arg\")", workbook, FormulaType.CELL, -1);
+ assertEquals(3, ptg.length);
+
+ // the name gets encoded as the first operand on the stack
+ NameXPxg tname = (NameXPxg) ptg[0];
+ assertEquals("myFunc", tname.toFormulaString());
+
+ // the function's arguments are pushed onto the stack from left-to-right as OperandPtgs
+ StringPtg arg = (StringPtg) ptg[1];
+ assertEquals("arg", arg.getValue());
+
+ // The external FunctionPtg is the last Ptg added to the stack
+ // During formula evaluation, this Ptg pops off the the appropriate number of
+ // arguments (getNumberOfOperands()) and pushes the result on the stack
+ AbstractFunctionPtg tfunc = (AbstractFunctionPtg) ptg[2];
+ assertTrue(tfunc.isExternalFunction());
+
+ // confirm formula parsing is case-insensitive
+ FormulaParser.parse("mYfUnC(\"arg\")", workbook, FormulaType.CELL, -1);
+
+ // confirm formula parsing doesn't care about argument count or type
+ // this should only throw an error when evaluating the formula.
+ FormulaParser.parse("myFunc()", workbook, FormulaType.CELL, -1);
+ FormulaParser.parse("myFunc(\"arg\", 0, TRUE)", workbook, FormulaType.CELL, -1);
+
+ // A completely unknown formula name (not saved in workbook) should still be parseable and renderable
+ // but will throw an NotImplementedFunctionException or return a #NAME? error value if evaluated.
+ FormulaParser.parse("yourFunc(\"arg\")", workbook, FormulaType.CELL, -1);
+
+ // Make sure workbook can be written and read
+ XSSFTestDataSamples.writeOutAndReadBack(wb).close();
+
+ // Manually check to make sure file isn't corrupted
+ final File fileIn = XSSFTestDataSamples.getSampleFile(testFile);
+ final File reSavedFile = new File(fileIn.getParentFile(), fileIn.getName().replace(".xlsm", "-saved.xlsm"));
+ final FileOutputStream fos = new FileOutputStream(reSavedFile);
+ wb.write(fos);
+ fos.close();
+ } finally {
+ wb.close();
+ }
+ }
+
+ public void testParserErrors() throws Exception {
+ XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("testNames.xlsm");
+ try {
+ XSSFEvaluationWorkbook workbook = XSSFEvaluationWorkbook.create(wb);
+
+ parseExpectedException("(");
+ parseExpectedException(")");
+ parseExpectedException("+");
+ parseExpectedException("42+");
+ parseExpectedException("IF()");
+ parseExpectedException("IF("); //no closing paren
+ parseExpectedException("myFunc(", workbook); //no closing paren
+ } finally {
+ wb.close();
+ }
+ }
+
+ private static void parseExpectedException(String formula) {
+ parseExpectedException(formula, null);
+ }
+
+ /** confirm formula has invalid syntax and parsing the formula results in FormulaParseException
+ * @param formula
+ * @param wb
+ */
+ private static void parseExpectedException(String formula, FormulaParsingWorkbook wb) {
+ try {
+ FormulaParser.parse(formula, wb, FormulaType.CELL, -1);
+ fail("Expected FormulaParseException: " + formula);
+ } catch (final FormulaParseException e) {
+ // expected during successful test
+ assertNotNull(e.getMessage());
+ }
+ }
}
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java?rev=1711605&r1=1711604&r2=1711605&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java Sat Oct 31 11:57:39 2015
@@ -19,7 +19,10 @@ package org.apache.poi.hssf.model;
import static org.junit.Assert.assertArrayEquals;
+import java.io.File;
+import java.io.FileOutputStream;
import java.io.IOException;
+import java.util.Locale;
import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
@@ -111,20 +114,68 @@ public final class TestFormulaParser ext
assertEquals("TOTAL[", ((StringPtg)ptgs[0]).getValue());
}
- public void testMacroFunction() {
+ public void testMacroFunction() throws IOException {
// testNames.xls contains a VB function called 'myFunc'
- HSSFWorkbook w = HSSFTestDataSamples.openSampleWorkbook("testNames.xls");
- HSSFEvaluationWorkbook book = HSSFEvaluationWorkbook.create(w);
-
- Ptg[] ptg = HSSFFormulaParser.parse("myFunc()", w);
- // myFunc() actually takes 1 parameter. Don't know if POI will ever be able to detect this problem
-
- // the name gets encoded as the first arg
- NamePtg tname = (NamePtg) ptg[0];
- assertEquals("myFunc", tname.toFormulaString(book));
+ final String testFile = "testNames.xls";
+ HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(testFile);
+ try {
+ HSSFEvaluationWorkbook book = HSSFEvaluationWorkbook.create(wb);
- AbstractFunctionPtg tfunc = (AbstractFunctionPtg) ptg[1];
- assertTrue(tfunc.isExternalFunction());
+ //Expected ptg stack: [NamePtg(myFunc), StringPtg(arg), (additional operands go here...), FunctionPtg(myFunc)]
+ Ptg[] ptg = FormulaParser.parse("myFunc(\"arg\")", book, FormulaType.CELL, -1);
+ assertEquals(3, ptg.length);
+
+ // the name gets encoded as the first operand on the stack
+ NamePtg tname = (NamePtg) ptg[0];
+ assertEquals("myFunc", tname.toFormulaString(book));
+
+ // the function's arguments are pushed onto the stack from left-to-right as OperandPtgs
+ StringPtg arg = (StringPtg) ptg[1];
+ assertEquals("arg", arg.getValue());
+
+ // The external FunctionPtg is the last Ptg added to the stack
+ // During formula evaluation, this Ptg pops off the the appropriate number of
+ // arguments (getNumberOfOperands()) and pushes the result on the stack
+ AbstractFunctionPtg tfunc = (AbstractFunctionPtg) ptg[2]; //FuncVarPtg
+ assertTrue(tfunc.isExternalFunction());
+
+ // confirm formula parsing is case-insensitive
+ FormulaParser.parse("mYfUnC(\"arg\")", book, FormulaType.CELL, -1);
+
+ // confirm formula parsing doesn't care about argument count or type
+ // this should only throw an error when evaluating the formula.
+ FormulaParser.parse("myFunc()", book, FormulaType.CELL, -1);
+ FormulaParser.parse("myFunc(\"arg\", 0, TRUE)", book, FormulaType.CELL, -1);
+
+ // A completely unknown formula name (not saved in workbook) should still be parseable and renderable
+ // but will throw an NotImplementedFunctionException or return a #NAME? error value if evaluated.
+ FormulaParser.parse("yourFunc(\"arg\")", book, FormulaType.CELL, -1);
+
+ // Verify that myFunc and yourFunc were successfully added to Workbook names
+ HSSFWorkbook wb2 = HSSFTestDataSamples.writeOutAndReadBack(wb);
+ try {
+ // HSSFWorkbook/EXCEL97-specific side-effects user-defined function names must be added to Workbook's defined names in order to be saved.
+ assertNotNull(wb2.getName("myFunc"));
+ assertEqualsIgnoreCase("myFunc", wb2.getName("myFunc").getNameName());
+ assertNotNull(wb2.getName("yourFunc"));
+ assertEqualsIgnoreCase("yourFunc", wb2.getName("yourFunc").getNameName());
+
+ // Manually check to make sure file isn't corrupted
+ final File fileIn = HSSFTestDataSamples.getSampleFile(testFile);
+ final File reSavedFile = new File(fileIn.getParentFile(), fileIn.getName().replace(".xls", "-saved.xls"));
+ FileOutputStream fos = new FileOutputStream(reSavedFile);
+ wb2.write(fos);
+ fos.close();
+ } finally {
+ wb2.close();
+ }
+ } finally {
+ wb.close();
+ }
+ }
+
+ private final static void assertEqualsIgnoreCase(String expected, String actual) {
+ assertEquals(expected.toLowerCase(Locale.ROOT), actual.toLowerCase(Locale.ROOT));
}
public void testEmbeddedSlash() {
@@ -713,12 +764,19 @@ public final class TestFormulaParser ext
parseExpectedException("IF(TRUE)");
parseExpectedException("countif(A1:B5, C1, D1)");
+
+ parseExpectedException("(");
+ parseExpectedException(")");
+ parseExpectedException("+");
+ parseExpectedException("42+");
+
+ parseExpectedException("IF(");
}
private static void parseExpectedException(String formula) {
try {
parseFormula(formula);
- throw new AssertionFailedError("expected parse exception");
+ throw new AssertionFailedError("Expected FormulaParseException: " + formula);
} catch (FormulaParseException e) {
// expected during successful test
assertNotNull(e.getMessage());
Modified: poi/trunk/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java?rev=1711605&r1=1711604&r2=1711605&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java Sat Oct 31 11:57:39 2015
@@ -19,6 +19,8 @@ package org.apache.poi.ss.formula;
import junit.framework.TestCase;
import org.apache.poi.ss.ITestDataProvider;
import org.apache.poi.ss.formula.eval.ErrorEval;
+import org.apache.poi.ss.formula.eval.NotImplementedException;
+import org.apache.poi.ss.formula.eval.NotImplementedFunctionException;
import org.apache.poi.ss.formula.eval.StringEval;
import org.apache.poi.ss.formula.eval.ValueEval;
import org.apache.poi.ss.formula.functions.FreeRefFunction;
@@ -84,6 +86,7 @@ public class BaseTestExternalFunctions e
public void testExternalFunctions() {
Workbook wb = _testDataProvider.createWorkbook();
+ FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
Sheet sh = wb.createSheet();
@@ -92,11 +95,16 @@ public class BaseTestExternalFunctions e
assertEquals("ISODD(1)+ISEVEN(2)", cell1.getCellFormula());
Cell cell2 = sh.createRow(1).createCell(0);
+ cell2.setCellFormula("MYFUNC(\"B1\")"); //unregistered functions are parseable and renderable, but may not be evaluateable
try {
- cell2.setCellFormula("MYFUNC(\"B1\")");
- fail("Should fail because MYFUNC is an unknown function");
- } catch (FormulaParseException e){
- ; //expected
+ evaluator.evaluate(cell2);
+ fail("Expected NotImplementedFunctionException/NotImplementedException");
+ } catch (final NotImplementedException e) {
+ if (!(e.getCause() instanceof NotImplementedFunctionException))
+ throw e;
+ // expected
+ // Alternatively, a future implementation of evaluate could return #NAME? error to align behavior with Excel
+ // assertEquals(ErrorEval.NAME_INVALID, ErrorEval.valueOf(evaluator.evaluate(cell2).getErrorValue()));
}
wb.addToolPack(customToolpack);
@@ -108,7 +116,6 @@ public class BaseTestExternalFunctions e
cell3.setCellFormula("MYFUNC2(\"C1\")&\"-\"&A2"); //where A2 is defined above
assertEquals("MYFUNC2(\"C1\")&\"-\"&A2", cell3.getCellFormula());
- FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
assertEquals(2.0, evaluator.evaluate(cell1).getNumberValue());
assertEquals("B1abc", evaluator.evaluate(cell2).getStringValue());
assertEquals("C1abc2-B1abc", evaluator.evaluate(cell3).getStringValue());
Modified: poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java?rev=1711605&r1=1711604&r2=1711605&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java Sat Oct 31 11:57:39 2015
@@ -298,6 +298,45 @@ public abstract class BaseTestCell {
private Cell createACell() {
return _testDataProvider.createWorkbook().createSheet("Sheet1").createRow(0).createCell(0);
}
+
+ /**
+ * bug 58452: Copy cell formulas containing unregistered function names
+ * Make sure that formulas with unknown/unregistered UDFs can be written to and read back from a file.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testFormulaWithUnknownUDF() throws IOException {
+ final Workbook wb1 = _testDataProvider.createWorkbook();
+ final FormulaEvaluator evaluator1 = wb1.getCreationHelper().createFormulaEvaluator();
+ try {
+ final Cell cell1 = wb1.createSheet().createRow(0).createCell(0);
+ final String formula = "myFunc(\"arg\")";
+ cell1.setCellFormula(formula);
+ confirmFormulaWithUnknownUDF(formula, cell1, evaluator1);
+
+ final Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1);
+ final FormulaEvaluator evaluator2 = wb2.getCreationHelper().createFormulaEvaluator();
+ try {
+ final Cell cell2 = wb2.getSheetAt(0).getRow(0).getCell(0);
+ confirmFormulaWithUnknownUDF(formula, cell2, evaluator2);
+ } finally {
+ wb2.close();
+ }
+ } finally {
+ wb1.close();
+ }
+ }
+
+ private static void confirmFormulaWithUnknownUDF(String expectedFormula, Cell cell, FormulaEvaluator evaluator) {
+ assertEquals(expectedFormula, cell.getCellFormula());
+ try {
+ evaluator.evaluate(cell);
+ fail("Expected NotImplementedFunctionException/NotImplementedException");
+ } catch (final org.apache.poi.ss.formula.eval.NotImplementedException e) {
+ // expected
+ }
+ }
@Test
public void testChangeTypeStringToBool() {
Added: poi/trunk/test-data/spreadsheet/testNames.xlsm
URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/testNames.xlsm?rev=1711605&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/testNames.xlsm
------------------------------------------------------------------------------
svn:mime-type = application/vnd.ms-excel.sheet.macroEnabled.12
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org