]> source.dussan.org Git - poi.git/commitdiff
bug58452: set cell formulas containing unregistered function names
authorJaven O'Neal <onealj@apache.org>
Sat, 31 Oct 2015 11:57:39 +0000 (11:57 +0000)
committerJaven O'Neal <onealj@apache.org>
Sat, 31 Oct 2015 11:57:39 +0000 (11:57 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1711605 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/formula/FormulaParser.java
src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java
src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java
src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java
src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
test-data/spreadsheet/testNames.xlsm [new file with mode: 0644]

index 18981ec725db27647aefc8446a2c59f5f763de3f..dddf2bbbbf818c83d9621377787b43cd197339eb 100644 (file)
@@ -50,6 +50,7 @@ import org.apache.poi.ss.formula.ptg.MissingArgPtg;
 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.UnaryPlusPtg;
 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.CellReference.NameType;
  * <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.
index f90c653a980d1c9d4dcd7af9f9e6a92b718efc5a..ade19a3846c48ff4b349409e84a2067cf1da09fc 100644 (file)
  */
 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 TestCase {
         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 09bf5a9b6a1f2d2522abb323eeb92dba553fa1ef..35576cac6dc38f3b46654e98a6c01d13abe260c0 100644 (file)
@@ -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 extends TestCase {
                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));
-
-               AbstractFunctionPtg tfunc = (AbstractFunctionPtg) ptg[1];
-               assertTrue(tfunc.isExternalFunction());
+               final String testFile = "testNames.xls";
+               HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(testFile);
+               try {
+                       HSSFEvaluationWorkbook book = HSSFEvaluationWorkbook.create(wb);
+
+                       //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 extends TestCase {
 
                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 87da6100db192138702a53cbfd3c3433c5889db4..d40bdca57a2409e2d5d837d59e639b7fa6c21d2e 100644 (file)
@@ -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 extends TestCase {
 
     public void testExternalFunctions() {
         Workbook wb = _testDataProvider.createWorkbook();
+        FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
 
         Sheet sh = wb.createSheet();
 
@@ -92,11 +95,16 @@ public class BaseTestExternalFunctions extends TestCase {
         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 extends TestCase {
         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());
index 9cdaf3d18de08b29d0fda6d9f2582954b7f9a90a..ebefb4358c344de989e7c7914cccd8baad59ca2c 100644 (file)
@@ -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() {
diff --git a/test-data/spreadsheet/testNames.xlsm b/test-data/spreadsheet/testNames.xlsm
new file mode 100644 (file)
index 0000000..f4e7db8
Binary files /dev/null and b/test-data/spreadsheet/testNames.xlsm differ