Bug 63845: Adjust handling of formula-cells to fix regression introduced in 4.1.0

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

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -531,6 +531,12 @@ public class HSSFCell extends CellBase {
      */
     @Override
     protected void setCellFormulaImpl(String formula) {
+        // formula cells always have a value. If the cell is blank (either initially or after removing an
+        // array formula), set value to 0
+        if (getValueType() == CellType.BLANK) {
+            setCellValue(0);
+        }
+
         assert formula != null;
 
         int row=_record.getRow();
@@ -566,7 +572,7 @@ public class HSSFCell extends CellBase {
             case ERROR:
                 return CellValue.getError(getErrorCellValue());
             default:
-                throw new IllegalStateException();
+                throw new IllegalStateException("Unexpected cell-type " + valueType);
         }
     }
 
@@ -585,7 +591,7 @@ public class HSSFCell extends CellBase {
                 setCellErrorValue(FormulaError.forInt(value.getErrorValue()));
                 break;
             default:
-                throw new IllegalStateException();
+                throw new IllegalStateException("Unexpected cell-type " + value.getCellType() + " for cell-value: " + value);
         }
     }
 

src/java/org/apache/poi/ss/usermodel/CellBase.java

@@ -123,12 +123,6 @@ public abstract class CellBase implements Cell {
             return;
         }
 
-        // formula cells always have a value. If the cell is blank (either initially or after removing an
-        // array formula), set value to 0
-        if (getValueType() == CellType.BLANK) {
-            setCellValue(0);
-        }
-
         setCellFormulaImpl(formula);
     }
 
@@ -255,7 +249,7 @@ public abstract class CellBase implements Cell {
      * @param value the new date to set
      */
     protected abstract void setCellValueImpl(LocalDateTime value);
-    
+
     /**
      * {@inheritDoc}
      */

src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java

@@ -51,7 +51,7 @@ public class SXSSFCell extends CellBase {
     private Value _value;
     private CellStyle _style;
     private Property _firstProperty;
-    
+
     public SXSSFCell(SXSSFRow row, CellType cellType)
     {
         _row=row;
@@ -133,7 +133,7 @@ public class SXSSFCell extends CellBase {
 
         return _value.getType();
     }
-    
+
     /**
      * Only valid for formula cells
      * @return one of ({@link CellType#NUMERIC}, {@link CellType#STRING},
@@ -194,7 +194,7 @@ public class SXSSFCell extends CellBase {
         boolean date1904 = getSheet().getWorkbook().isDate1904();
         setCellValue(DateUtil.getExcelDate(value, date1904));
     }
-    
+
     /**
      * {@inheritDoc}
      */
@@ -249,6 +249,7 @@ public class SXSSFCell extends CellBase {
             ((FormulaValue)_value).setValue(formula);
         } else {
             switch (getCellType()) {
+                case BLANK:
                 case NUMERIC:
                     _value = new NumericFormulaValue(formula, getNumericCellValue());
                     break;
@@ -266,12 +267,8 @@ public class SXSSFCell extends CellBase {
                 case ERROR:
                     _value = new ErrorFormulaValue(formula, getErrorCellValue());
                     break;
-                case _NONE:
-                    // fall-through
-                case FORMULA:
-                    // fall-through
-                case BLANK:
-                    throw new AssertionError();
+                default:
+                    throw new IllegalStateException("Cannot set a formula for a cell of type " + getCellType());
             }
         }
     }
@@ -334,7 +331,7 @@ public class SXSSFCell extends CellBase {
     public double getNumericCellValue()
     {
         CellType cellType = getCellType();
-        switch(cellType) 
+        switch(cellType)
         {
             case BLANK:
                 return 0.0;
@@ -366,7 +363,7 @@ public class SXSSFCell extends CellBase {
     public Date getDateCellValue()
     {
         CellType cellType = getCellType();
-        if (cellType == CellType.BLANK) 
+        if (cellType == CellType.BLANK)
         {
             return null;
         }
@@ -434,7 +431,7 @@ public class SXSSFCell extends CellBase {
     public String getStringCellValue()
     {
         CellType cellType = getCellType();
-        switch(cellType) 
+        switch(cellType)
         {
             case BLANK:
                 return "";
@@ -510,7 +507,7 @@ public class SXSSFCell extends CellBase {
     public boolean getBooleanCellValue()
     {
         CellType cellType = getCellType();
-        switch(cellType) 
+        switch(cellType)
         {
             case BLANK:
                 return false;
@@ -545,7 +542,7 @@ public class SXSSFCell extends CellBase {
     public byte getErrorCellValue()
     {
         CellType cellType = getCellType();
-        switch(cellType) 
+        switch(cellType)
         {
             case BLANK:
                 return 0;
@@ -568,10 +565,10 @@ public class SXSSFCell extends CellBase {
     /**
      * <p>Set the style for the cell.  The style should be an CellStyle created/retreived from
      * the Workbook.</p>
-     * 
+     *
      * <p>To change the style of a cell without affecting other cells that use the same style,
      * use {@link org.apache.poi.ss.util.CellUtil#setCellStyleProperties(Cell, Map)}</p>
-     * 
+     *
      * @param style  reference contained in the workbook.
      * If the value is null then the style information is removed causing the cell to used the default workbook style.
      * @see org.apache.poi.ss.usermodel.Workbook#createCellStyle
@@ -964,7 +961,7 @@ public class SXSSFCell extends CellBase {
                 return false;
             default: throw new RuntimeException("Unexpected cell type (" + cellType + ")");
         }
-        
+
     }
     private String convertCellValueToString() {
         CellType cellType = getCellType();

src/ooxml/testcases/org/apache/poi/ss/usermodel/BaseTestXCell.java

@@ -20,8 +20,6 @@ package org.apache.poi.ss.usermodel;
 import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
-import java.util.Calendar;
-import java.util.Date;
 
 import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.ss.ITestDataProvider;
@@ -30,9 +28,9 @@ import org.apache.poi.xssf.usermodel.XSSFCell;
 import org.junit.Test;
 
 /**
- * Class for combined testing of XML-specific functionality of 
+ * Class for combined testing of XML-specific functionality of
  * {@link XSSFCell} and {@link SXSSFCell}.
- * 
+ *
  *  Any test that is applicable for {@link HSSFCell} as well should go into
  *  the common base class {@link BaseTestCell}.
  */
@@ -47,14 +45,14 @@ public abstract class BaseTestXCell extends BaseTestCell {
         Sheet sh = wb1.createSheet();
         Row row = sh.createRow(0);
         Cell cell = row.createCell(0);
-        String sval = "\u0000\u0002\u0012<>\t\n\u00a0 &\"POI\'\u2122";
+        String sval = "\u0000\u0002\u0012<>\t\n\u00a0 &\"POI'\u2122";
         cell.setCellValue(sval);
 
         Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1);
         wb1.close();
 
         // invalid characters are replaced with question marks
-        assertEquals("???<>\t\n\u00a0 &\"POI\'\u2122", wb2.getSheetAt(0).getRow(0).getCell(0).getStringCellValue());
+        assertEquals("???<>\t\n\u00a0 &\"POI'\u2122", wb2.getSheetAt(0).getRow(0).getCell(0).getStringCellValue());
         wb2.close();
     }
 }

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java

@@ -3494,4 +3494,52 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
             LOG.log(POILogger.INFO, Duration.between(start, Instant.now()));
         }
     }
+
+    @Test
+    public void testBug63845() throws IOException {
+        try (Workbook wb = new XSSFWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            Row row = sheet.createRow(0);
+
+            Cell cell = row.createCell(0, CellType.FORMULA);
+            cell.setCellFormula("SUM(B1:E1)");
+
+            assertNull("Element 'v' should not be set for formulas unless the value was calculated",
+                    ((XSSFCell) cell).getCTCell().getV());
+
+            try (Workbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb)) {
+                Cell cellBack = wbBack.getSheetAt(0).getRow(0).getCell(0);
+                assertNull("Element 'v' should not be set for formulas unless the value was calculated",
+                        ((XSSFCell) cellBack).getCTCell().getV());
+
+                wbBack.getCreationHelper().createFormulaEvaluator().evaluateInCell(cellBack);
+
+                assertEquals("Element 'v' should be set now as the formula was calculated manually",
+                        "0.0", ((XSSFCell) cellBack).getCTCell().getV());
+            }
+        }
+    }
+
+    @Test
+    public void testBug63845_2() throws IOException {
+        try (Workbook wb = new XSSFWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            Row row = sheet.createRow(0);
+            row.createCell(0).setCellValue(2);
+            row.createCell(1).setCellValue(5);
+            row.createCell(2).setCellFormula("A1+B1");
+
+            try (Workbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb)) {
+                Cell cellBack = wbBack.getSheetAt(0).getRow(0).getCell(2);
+
+                assertNull("Element 'v' should not be set for formulas unless the value was calculated",
+                        ((XSSFCell) cellBack).getCTCell().getV());
+
+                wbBack.getCreationHelper().createFormulaEvaluator().evaluateInCell(cellBack);
+
+                assertEquals("Element 'v' should be set now as the formula was calculated manually",
+                        "7.0", ((XSSFCell) cellBack).getCTCell().getV());
+            }
+        }
+    }
 }

src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java

@@ -1352,7 +1352,8 @@ public abstract class BaseTestCell {
             cell.setCellFormula("\"foo\"");
             assertEquals(CellType.FORMULA, cell.getCellType());
             assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType());
-            assertTrue(cell.getBooleanCellValue());
+            assertTrue("Expected a boolean cell-value, but had 'false'",
+                    cell.getBooleanCellValue());
         }
     }
 
@@ -1369,7 +1370,8 @@ public abstract class BaseTestCell {
             cell.setCellFormula("\"bar\"");
             assertEquals(CellType.FORMULA, cell.getCellType());
             assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType());
-            assertTrue(cell.getBooleanCellValue());
+            assertTrue("Expected a boolean cell-value, but had 'false'",
+                    cell.getBooleanCellValue());
         }
     }
 
@@ -1387,8 +1389,10 @@ public abstract class BaseTestCell {
 
             cell.getSheet().setArrayFormula("\"bar\"", CellRangeAddress.valueOf("A1"));
             assertEquals(CellType.FORMULA, cell.getCellType());
-            assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType());
-            assertTrue(cell.getBooleanCellValue());
+            assertEquals("Expected a boolean cell-value, but had " + cell.getCachedFormulaResultType(),
+                    CellType.BOOLEAN, cell.getCachedFormulaResultType());
+            assertTrue("Expected a boolean cell-value, but had 'false'",
+                    cell.getBooleanCellValue());
         }
     }