[patch] main FormulaParser changes

mainChanges-createName.patch (text/plain), 14.60 KB, created by Javen O'Neal on 2015-10-06 03:17:52 UTC

(hide)

Description: main FormulaParser changes

Filename:

MIME Type:

Creator: Javen O'Neal

Created: 2015-10-06 03:17:52 UTC

Size: 14.60 KB

patch

obsolete

>Index: src/java/org/apache/poi/ss/formula/FormulaParser.java >=================================================================== >--- src/java/org/apache/poi/ss/formula/FormulaParser.java (revision 1704431) >+++ src/java/org/apache/poi/ss/formula/FormulaParser.java (working copy) >@@ -67,9 +67,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 +88,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 +112,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 +141,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 +932,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 +947,31 @@ > // 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, >+ "Name '" + name + "' is completely unknown in the current workbook."); >+ } >+ // name is probably the name of an unregistered User-Defined Function >+ addName(name); >+ hName = _book.getName(name, _sheetIndex); >+ nameToken = hName.createPtg(); >+ } > } > } > >@@ -965,6 +981,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 1704431) >+++ src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java (working copy) >@@ -20,6 +20,11 @@ > > 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.NamePtg; >+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 +67,81 @@ > catch (FormulaParseException expected) { > } > } >+ >+ // copied from org.apache.poi.hssf.model.TestFormulaParser >+ public void testMacroFunction() throws Exception { >+ // testNames.xlsx contains a VB function called 'myFunc' >+ XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("testNames.xlsm"); >+ 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 >+ NamePtg tname = (NamePtg) ptg[0]; >+ assertEquals("myFunc", tname.toFormulaString(workbook)); >+ >+ // 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); >+ } 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 1704431) >+++ src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java (working copy) >@@ -36,10 +36,12 @@ > import org.apache.poi.hssf.usermodel.HSSFSheet; > import org.apache.poi.hssf.usermodel.HSSFWorkbook; > import org.apache.poi.hssf.usermodel.TestHSSFName; >+import org.apache.poi.ss.formula.EvaluationName; > import org.apache.poi.ss.formula.FormulaParseException; > import org.apache.poi.ss.formula.FormulaParser; > import org.apache.poi.ss.formula.FormulaType; > import org.apache.poi.ss.formula.constant.ErrorConstant; >+import org.apache.poi.ss.formula.function.FunctionMetadataRegistry; > import org.apache.poi.ss.formula.ptg.*; > import org.apache.poi.ss.usermodel.BaseTestBugzillaIssues; > import org.apache.poi.ss.usermodel.Name; >@@ -116,15 +118,35 @@ > 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 >+ //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 >+ // 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()); > >- AbstractFunctionPtg tfunc = (AbstractFunctionPtg) ptg[1]; >+ // 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); > } > > public void testEmbeddedSlash() { >@@ -713,12 +735,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/formula/BaseTestExternalFunctions.java >=================================================================== >--- src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java (revision 1704431) >+++ 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]