Bug 58648: Fix handling whitespaces in formulas, unfortunately blank can be the inter...

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1738033 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/poi/ss/formula/FormulaParser.java b/src/java/org/apache/poi/ss/formula/FormulaParser.java
index f2430a90be45f74b623c054f58ad3fa5093d5666..8e5d8e999a0732ec47163a480ba66ba4ee32c085 100644 (file)
--- a/src/java/org/apache/poi/ss/formula/FormulaParser.java
+++ b/src/java/org/apache/poi/ss/formula/FormulaParser.java
@@ -1530,11 +1530,19 @@ public final class FormulaParser {
                while (true) {
                        SkipWhite();
                        if (_inIntersection) {
+                               int savePointer = _pointer;
+
                                // Don't getChar() as the space has already been eaten and recorded by SkipWhite().
-                               hasIntersections = true;
-                               ParseNode other = comparisonExpression();
-                               result = new ParseNode(IntersectionPtg.instance, result, other);
-                               continue;
+                               try {
+                                       ParseNode other = comparisonExpression();
+                                       result = new ParseNode(IntersectionPtg.instance, result, other);
+                                       hasIntersections = true;
+                                       continue;
+                               } catch (FormulaParseException e) {
+                                       // if parsing for intersection fails we assume that we actually had an arbitrary
+                                       // whitespace and thus should simply skip this whitespace
+                                       resetPointer(savePointer);
+                               }
                        }
                        if (hasIntersections) {
                                return augmentWithMemPtg(result);

diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java
index c7c466cf6506094dd8765c8a8667d4559b64cef2..83dabacb9fd283446fa7b6cda35dffb16eb2eb0c 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.ss.formula.FormulaParseException;
@@ -31,25 +32,14 @@ import org.apache.poi.ss.formula.FormulaParsingWorkbook;
 import org.apache.poi.ss.formula.FormulaRenderingWorkbook;
 import org.apache.poi.ss.formula.FormulaType;
 import org.apache.poi.ss.formula.WorkbookDependentFormula;
-import org.apache.poi.ss.formula.ptg.Area3DPtg;
-import org.apache.poi.ss.formula.ptg.Area3DPxg;
-import org.apache.poi.ss.formula.ptg.AreaPtg;
-import org.apache.poi.ss.formula.ptg.AttrPtg;
-import org.apache.poi.ss.formula.ptg.FuncPtg;
-import org.apache.poi.ss.formula.ptg.FuncVarPtg;
-import org.apache.poi.ss.formula.ptg.IntPtg;
-import org.apache.poi.ss.formula.ptg.NamePtg;
-import org.apache.poi.ss.formula.ptg.NameXPxg;
-import org.apache.poi.ss.formula.ptg.Ptg;
-import org.apache.poi.ss.formula.ptg.Ref3DPtg;
-import org.apache.poi.ss.formula.ptg.Ref3DPxg;
-import org.apache.poi.ss.formula.ptg.RefPtg;
-import org.apache.poi.ss.usermodel.Cell;
-import org.apache.poi.ss.usermodel.Sheet;
-import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.formula.ptg.*;
+import org.apache.poi.ss.usermodel.*;
+import org.apache.poi.ss.util.CellReference;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 import org.junit.Test;
 
+import java.util.Arrays;
+
 public final class TestXSSFFormulaParser {
        private static Ptg[] parse(FormulaParsingWorkbook fpb, String fmla) {
                return FormulaParser.parse(fmla, fpb, FormulaType.CELL, -1);
@@ -63,25 +53,25 @@ public final class TestXSSFFormulaParser {
 
         ptgs = parse(fpb, "ABC10");
         assertEquals(1, ptgs.length);
-        assertTrue("", ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
 
         ptgs = parse(fpb, "A500000");
         assertEquals(1, ptgs.length);
-        assertTrue("", ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
 
         ptgs = parse(fpb, "ABC500000");
         assertEquals(1, ptgs.length);
-        assertTrue("", ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
 
         //highest allowed rows and column (XFD and 0x100000)
         ptgs = parse(fpb, "XFD1048576");
         assertEquals(1, ptgs.length);
-        assertTrue("", ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
 
 
         //column greater than XFD
         try {
-            ptgs = parse(fpb, "XFE10");
+            /*ptgs =*/ parse(fpb, "XFE10");
             fail("expected exception");
         } catch (FormulaParseException e){
             assertEquals("Specified named range 'XFE10' does not exist in the current workbook.", e.getMessage());
@@ -89,7 +79,7 @@ public final class TestXSSFFormulaParser {
 
         //row greater than 0x100000
         try {
-            ptgs = parse(fpb, "XFD1048577");
+            /*ptgs =*/ parse(fpb, "XFD1048577");
             fail("expected exception");
         } catch (FormulaParseException e){
             assertEquals("Specified named range 'XFD1048577' does not exist in the current workbook.", e.getMessage());
@@ -142,8 +132,8 @@ public final class TestXSSFFormulaParser {
 
         ptgs = parse(fpb, "LOG10(100)");
         assertEquals(2, ptgs.length);
-        assertTrue("", ptgs[0] instanceof IntPtg);
-        assertTrue("", ptgs[1] instanceof FuncPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof IntPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof FuncPtg);
     }
     
     @Test
@@ -296,16 +286,14 @@ public final class TestXSSFFormulaParser {
             
             
             // Create a formula parser
-            FormulaParsingWorkbook fpb = null;
+            final FormulaParsingWorkbook fpb;
             if (wb instanceof HSSFWorkbook)
                 fpb = HSSFEvaluationWorkbook.create((HSSFWorkbook)wb);
             else
                 fpb = XSSFEvaluationWorkbook.create((XSSFWorkbook)wb);
-            
-            
+
             // Check things parse as expected:
-            
-            
+
             // SUM to one cell over 3 workbooks, relative reference
             ptgs = parse(fpb, "SUM(Sheet1:Sheet3!A1)");
             assertEquals(2, ptgs.length);
@@ -317,8 +305,7 @@ public final class TestXSSFFormulaParser {
             assertEquals("Sheet1:Sheet3!A1", toFormulaString(ptgs[0], fpb));
             assertEquals(AttrPtg.class, ptgs[1].getClass());
             assertEquals("SUM",         toFormulaString(ptgs[1], fpb));
-            
-            
+
             // MAX to one cell over 3 workbooks, absolute row reference
             ptgs = parse(fpb, "MAX(Sheet1:Sheet3!A$1)");
             assertEquals(2, ptgs.length);
@@ -330,8 +317,7 @@ public final class TestXSSFFormulaParser {
             assertEquals("Sheet1:Sheet3!A$1", toFormulaString(ptgs[0], fpb));
             assertEquals(FuncVarPtg.class, ptgs[1].getClass());
             assertEquals("MAX",            toFormulaString(ptgs[1], fpb));
-            
-            
+
             // MIN to one cell over 3 workbooks, absolute reference
             ptgs = parse(fpb, "MIN(Sheet1:Sheet3!$A$1)");
             assertEquals(2, ptgs.length);
@@ -343,8 +329,7 @@ public final class TestXSSFFormulaParser {
             assertEquals("Sheet1:Sheet3!$A$1", toFormulaString(ptgs[0], fpb));
             assertEquals(FuncVarPtg.class, ptgs[1].getClass());
             assertEquals("MIN",            toFormulaString(ptgs[1], fpb));
-            
-            
+
             // SUM to a range of cells over 3 workbooks
             ptgs = parse(fpb, "SUM(Sheet1:Sheet3!A1:B2)");
             assertEquals(2, ptgs.length);
@@ -356,8 +341,7 @@ public final class TestXSSFFormulaParser {
             assertEquals("Sheet1:Sheet3!A1:B2", toFormulaString(ptgs[0], fpb));
             assertEquals(AttrPtg.class, ptgs[1].getClass());
             assertEquals("SUM",         toFormulaString(ptgs[1], fpb));
-            
-            
+
             // MIN to a range of cells over 3 workbooks, absolute reference
             ptgs = parse(fpb, "MIN(Sheet1:Sheet3!$A$1:$B$2)");
             assertEquals(2, ptgs.length);
@@ -369,8 +353,7 @@ public final class TestXSSFFormulaParser {
             assertEquals("Sheet1:Sheet3!$A$1:$B$2", toFormulaString(ptgs[0], fpb));
             assertEquals(FuncVarPtg.class, ptgs[1].getClass());
             assertEquals("MIN",            toFormulaString(ptgs[1], fpb));
-            
-            
+
             // Check we can round-trip - try to set a new one to a new single cell
             Cell newF = s1.getRow(0).createCell(10, Cell.CELL_TYPE_FORMULA);
             newF.setCellFormula("SUM(Sheet2:Sheet3!A1)");
@@ -382,10 +365,169 @@ public final class TestXSSFFormulaParser {
             assertEquals("MIN(Sheet1:Sheet2!A1:B2)", newF.getCellFormula());
         }
     }
+
     private static String toFormulaString(Ptg ptg, FormulaParsingWorkbook wb) {
         if (ptg instanceof WorkbookDependentFormula) {
             return ((WorkbookDependentFormula)ptg).toFormulaString((FormulaRenderingWorkbook)wb);
         }
         return ptg.toFormulaString();
     }
+
+    @Test
+    public void test58648Single() {
+        XSSFWorkbook wb = new XSSFWorkbook();
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
+        Ptg[] ptgs;
+
+        ptgs = parse(fpb, "(ABC10 )");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                2, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+    }
+
+    @Test
+    public void test58648Basic() {
+        XSSFWorkbook wb = new XSSFWorkbook();
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
+        Ptg[] ptgs;
+
+        // verify whitespaces in different places
+        ptgs = parse(fpb, "(ABC10)");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                2, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+
+        ptgs = parse(fpb, "( ABC10)");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                2, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+
+        ptgs = parse(fpb, "(ABC10 )");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                2, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+
+        ptgs = parse(fpb, "((ABC10))");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                3, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof ParenthesisPtg);
+
+        ptgs = parse(fpb, "((ABC10) )");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                3, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof ParenthesisPtg);
+
+        ptgs = parse(fpb, "( (ABC10))");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                3, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof ParenthesisPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof ParenthesisPtg);
+    }
+
+    @Test
+    public void test58648FormulaParsing() {
+        Workbook wb = XSSFTestDataSamples.openSampleWorkbook("58648.xlsx");
+
+        FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
+
+        for (int i = 0; i < wb.getNumberOfSheets(); i++) {
+            Sheet xsheet = wb.getSheetAt(i);
+
+            for (Row row : xsheet) {
+                for (Cell cell : row) {
+                    if (cell.getCellType() == HSSFCell.CELL_TYPE_FORMULA) {
+                        try {
+                            evaluator.evaluateFormulaCell(cell);
+                        } catch (Exception e) {
+                            CellReference cellRef = new CellReference(cell.getRowIndex(), cell.getColumnIndex());
+                            throw new RuntimeException("error at: " + cellRef.toString(), e);
+                        }
+                    }
+                }
+            }
+        }
+
+        Sheet sheet = wb.getSheet("my-sheet");
+        Cell cell = sheet.getRow(1).getCell(4);
+
+        assertEquals(5d, cell.getNumericCellValue(), 0d);
+    }
+
+    @Test
+    public void testWhitespaceInFormula() {
+        XSSFWorkbook wb = new XSSFWorkbook();
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
+        Ptg[] ptgs;
+
+        // verify whitespaces in different places
+        ptgs = parse(fpb, "INTERCEPT(A2:A5, B2:B5)");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                3, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof FuncPtg);
+
+        ptgs = parse(fpb, " INTERCEPT ( \t \r A2 : \nA5 , B2 : B5 ) \t");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                3, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof FuncPtg);
+
+        ptgs = parse(fpb, "(VLOOKUP(\"item1\", A2:B3, 2, FALSE) - VLOOKUP(\"item2\", A2:B3, 2, FALSE) )");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                12, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof StringPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof IntPtg);
+
+        ptgs = parse(fpb, "A1:B1 B1:B2");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                4, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof MemAreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[3] instanceof IntersectionPtg);
+
+        ptgs = parse(fpb, "A1:B1    B1:B2");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                4, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof MemAreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[3] instanceof IntersectionPtg);
+    }
+
+    @Test
+    public void testWhitespaceInComplexFormula() {
+        XSSFWorkbook wb = new XSSFWorkbook();
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
+        Ptg[] ptgs;
+
+        // verify whitespaces in different places
+        ptgs = parse(fpb, "SUM(A1:INDEX(1:1048576,MAX(IFERROR(MATCH(99^99,B:B,1),0),IFERROR(MATCH(\"zzzz\",B:B,1),0)),MAX(IFERROR(MATCH(99^99,1:1,1),0),IFERROR(MATCH(\"zzzz\",1:1,1),0))))");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                40, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof MemFuncPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[3] instanceof NameXPxg);
+
+        ptgs = parse(fpb, "SUM ( A1 : INDEX( 1 : 1048576 , MAX( IFERROR ( MATCH ( 99 ^ 99 , B : B , 1 ) , 0 ) , IFERROR ( MATCH ( \"zzzz\" , B:B , 1 ) , 0 ) ) , MAX ( IFERROR ( MATCH ( 99 ^ 99 , 1 : 1 , 1 ) , 0 ) , IFERROR ( MATCH ( \"zzzz\" , 1 : 1 , 1 )   , 0 )   )   )   )");
+        assertEquals("Had: " + Arrays.toString(ptgs),
+                40, ptgs.length);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[0] instanceof MemFuncPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[1] instanceof RefPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[2] instanceof AreaPtg);
+        assertTrue("Had " + Arrays.toString(ptgs), ptgs[3] instanceof NameXPxg);
+    }
 }

diff --git a/test-data/spreadsheet/58648.xlsx b/test-data/spreadsheet/58648.xlsx
new file mode 100644 (file)
index 0000000..9f2f7bf
Binary files /dev/null and b/test-data/spreadsheet/58648.xlsx differ