[bug-66661] revert github-269 due to it breaking formulas with table references

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

5 files changed, 104 insertions, 22 deletions

diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java
index 07f4d6d314..755da69e12 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFCell.java
@@ -29,7 +29,6 @@ import org.apache.poi.ss.formula.FormulaRenderer;
 import org.apache.poi.ss.formula.FormulaType;
 import org.apache.poi.ss.formula.SharedFormula;
 import org.apache.poi.ss.formula.eval.ErrorEval;
-import org.apache.poi.ss.formula.ptg.ErrPtg;
 import org.apache.poi.ss.formula.ptg.Ptg;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellBase;
@@ -493,19 +492,7 @@ public final class XSSFCell extends CellBase {
         if (wb.getCellFormulaValidation()) {
             XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
             //validate through the FormulaParser
-            Ptg[] ptgs = FormulaParser.parse(formula, fpb, formulaType, wb.getSheetIndex(getSheet()), getRowIndex());
-            // Make its format consistent with Excel.
-            // eg: "SUM('Sheet1:Sheet2'!A1:B1)" will be trans to "SUM(Sheet1:Sheet2!A1:B1)"
-            boolean hasError = false;
-            for (Ptg ptg : ptgs) {
-                if (ptg instanceof ErrPtg) {
-                    hasError = true;
-                    break;
-                }
-            }
-            if (!hasError) {
-                formula = FormulaRenderer.toFormulaString(fpb, ptgs);
-            }
+            FormulaParser.parse(formula, fpb, formulaType, wb.getSheetIndex(getSheet()), getRowIndex());
         }
 
         CTCellFormula f;
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFCell.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFCell.java
index 2d2b930598..ec06fa81f2 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFCell.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFCell.java
@@ -560,14 +560,13 @@ public final class TestXSSFCell extends BaseTestXCell {
     @Test
     public final void testCopyCellFrom_CellCopyPolicy_formulaWithUnregisteredUDF() {
         setUp_testCopyCellFrom_CellCopyPolicy();
-        // this will format the formula string,
         srcCell.setCellFormula("MYFUNC2(123, $A5, Sheet1!$B7)");
 
         // Copy formula verbatim (no shifting). This is okay because copyCellFrom is Internal.
         // Users should use higher-level copying functions to row- or column-shift formulas.
         final CellCopyPolicy policy = new CellCopyPolicy.Builder().cellFormula(true).build();
         destCell.copyCellFrom(srcCell, policy);
-        assertEquals("MYFUNC2(123,$A5,Sheet1!$B7)", destCell.getCellFormula());
+        assertEquals("MYFUNC2(123, $A5, Sheet1!$B7)", destCell.getCellFormula());
     }
 
     @Test
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
index e8f2290935..887e23ae11 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
@@ -186,8 +186,8 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
                 // Add another formula referencing these workbooks
                 Cell cXSL_cell2 = rXSL.createCell(40);
                 cXSL_cell2.setCellFormula("[56737.xls]Uses!$C$1");
-                // it become [2] like the others
-                assertEquals("[2]Uses!$C$1", cXSL_cell2.getCellFormula());
+                // TODO Shouldn't it become [2] like the others?
+                assertEquals("[56737.xls]Uses!$C$1", cXSL_cell2.getCellFormula());
                 assertEquals("\"Hello!\"", evaluator.evaluate(cXSL_cell2).formatAsString());
 
 
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java
index 6f49ef11cd..fc4a9c6032 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java
@@ -400,10 +400,10 @@ public final class TestXSSFFormulaParser {
     }
 
     @Test
-    void testQuotedSheetNamesReference() {
+    void testQuotedSheetNamesReference() throws IOException {
         // quoted sheet names bug fix
-        Workbook[] wbs = new Workbook[]{new HSSFWorkbook(), new XSSFWorkbook()};
-        for (Workbook wb : wbs) {
+        // see TestHSSFFormulaEvaluator equivalent which behaves a little differently
+        try (XSSFWorkbook wb = new XSSFWorkbook()) {
             Sheet sheet1 = wb.createSheet("Sheet1");
             Sheet sheet2 = wb.createSheet("Sheet2");
             Sheet sheet3 = wb.createSheet("Sheet 3");
@@ -438,7 +438,7 @@ public final class TestXSSFFormulaParser {
             formula = "SUM('Sheet1:Sheet2'!A1:B1)";
             cell.setCellFormula(formula);
             cellFormula = cell.getCellFormula();
-            assertEquals("SUM(Sheet1:Sheet2!A1:B1)", cellFormula);
+            assertEquals("SUM('Sheet1:Sheet2'!A1:B1)", cellFormula);
 
             // quoted sheet names with space
             cell = tempRow.createCell(4);
diff --git a/poi/src/test/java/org/apache/poi/hssf/usermodel/TestHSSFFormulaParser.java b/poi/src/test/java/org/apache/poi/hssf/usermodel/TestHSSFFormulaParser.java
new file mode 100644
index 0000000000..add14594cb
--- /dev/null
+++ b/poi/src/test/java/org/apache/poi/hssf/usermodel/TestHSSFFormulaParser.java
@@ -0,0 +1,96 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.usermodel;
+
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class TestHSSFFormulaParser {
+
+    @Test
+    void testQuotedSheetNamesReference() throws IOException {
+        // quoted sheet names bug fix
+        // see TestXSSFFormulaEvaluator equivalent which behaves a little differently
+        try (Workbook wb = new HSSFWorkbook()) {
+            Sheet sheet1 = wb.createSheet("Sheet1");
+            Sheet sheet2 = wb.createSheet("Sheet2");
+            Sheet sheet3 = wb.createSheet("Sheet 3");
+            Sheet sheet4 = wb.createSheet("Sheet4>");
+
+            Row tempRow = sheet1.createRow(0);
+            tempRow.createCell(0).setCellValue(1);
+            tempRow.createCell(1).setCellValue(2);
+
+            tempRow = sheet2.createRow(0);
+            tempRow.createCell(0).setCellValue(3);
+            tempRow.createCell(1).setCellValue(4);
+
+            tempRow = sheet3.createRow(0);
+            tempRow.createCell(0).setCellValue(5);
+            tempRow.createCell(1).setCellValue(6);
+
+            tempRow = sheet4.createRow(0);
+            tempRow.createCell(0).setCellValue(5);
+            tempRow.createCell(1).setCellValue(6);
+
+            Cell cell = tempRow.createCell(2);
+
+            // unquoted sheet names
+            String formula = "SUM(Sheet1:Sheet2!A1:B1)";
+            cell.setCellFormula(formula);
+            String cellFormula = cell.getCellFormula();
+            assertEquals(formula, cellFormula);
+
+            // quoted sheet names with no space
+            cell = tempRow.createCell(3);
+            formula = "SUM('Sheet1:Sheet2'!A1:B1)";
+            cell.setCellFormula(formula);
+            cellFormula = cell.getCellFormula();
+            assertEquals("SUM(Sheet1:Sheet2!A1:B1)", cellFormula);
+
+            // quoted sheet names with space
+            cell = tempRow.createCell(4);
+            formula = "SUM('Sheet1:Sheet 3'!A1:B1)";
+            cell.setCellFormula(formula);
+            cellFormula = cell.getCellFormula();
+            assertEquals(formula, cellFormula);
+
+            // quoted sheet names with special character
+            cell = tempRow.createCell(5);
+            formula = "SUM('Sheet1:Sheet4>'!A1:B1)";
+            cell.setCellFormula(formula);
+            cellFormula = cell.getCellFormula();
+            assertEquals(formula, cellFormula);
+
+            // quoted sheet names with special character #2
+//            cell = tempRow.createCell(6);
+//            formula = "SUM('Sheet 3:Sheet4>'!A1:B1)";
+//            cell.setCellFormula(formula);
+//            cellFormula = cell.getCellFormula();
+//            assertEquals(formula, cellFormula);
+        }
+    }
+
+}