[patch] main FormulaParser changes

mainChanges-createName.patch (text/plain), 19.38 KB, created by Javen O'Neal on 2015-10-06 20:23:36 UTC

(hide)

Description: main FormulaParser changes

Filename:

MIME Type:

Creator: Javen O'Neal

Created: 2015-10-06 20:23:36 UTC

Size: 19.38 KB

patch

obsolete

>Index: src/java/org/apache/poi/ss/formula/FormulaParser.java >=================================================================== >--- src/java/org/apache/poi/ss/formula/FormulaParser.java (revision 1706948) >+++ src/java/org/apache/poi/ss/formula/FormulaParser.java (working copy) >@@ -50,6 +50,7 @@ > 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.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 @@ > * <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 @@ > */ > 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 @@ > > /** > * 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 @@ > * 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 @@ > // 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 @@ > > 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. >Index: src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java >=================================================================== >--- src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java (revision 1706948) >+++ src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java (working copy) >@@ -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 @@ > 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()); >+ } >+ } > > } >Index: src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java >=================================================================== >--- src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java (revision 1706948) >+++ src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java (working copy) >@@ -19,7 +19,10 @@ > > 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,21 +114,69 @@ > 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); >+ final String testFile = "testNames.xls"; >+ HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(testFile); >+ try { >+ HSSFEvaluationWorkbook book = HSSFEvaluationWorkbook.create(wb); > >- Ptg[] ptg = HSSFFormulaParser.parse("myFunc()", w); >- // myFunc() actually takes 1 parameter. Don't know if POI will ever be able to detect this problem >+ //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 arg >- NamePtg tname = (NamePtg) ptg[0]; >- assertEquals("myFunc", tname.toFormulaString(book)); >+ // the name gets encoded as the first operand on the stack >+ NamePtg tname = (NamePtg) ptg[0]; >+ assertEquals("myFunc", tname.toFormulaString(book)); > >- AbstractFunctionPtg tfunc = (AbstractFunctionPtg) ptg[1]; >- assertTrue(tfunc.isExternalFunction()); >+ // 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() { > confirmTokenClasses("HYPERLINK(\"http://www.jakarta.org\",\"Jakarta\")", >@@ -713,12 +764,19 @@ > > 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()); >Index: src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java >=================================================================== >--- src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java (revision 1706948) >+++ src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java (working copy) >@@ -298,6 +298,45 @@ > 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() { >Index: src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java >=================================================================== >--- src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java (revision 1706948) >+++ src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java (working copy) >@@ -19,6 +19,8 @@ > 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 void testExternalFunctions() { > Workbook wb = _testDataProvider.createWorkbook(); >+ FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator(); > > Sheet sh = wb.createSheet(); > >@@ -92,11 +95,15 @@ > 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); >+ } 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 +115,6 @@ > 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()); View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]