SetCalculatedValue Avoid Casting String to Numeric (#3685)

Fix #3658. Readers will call `setCalculatedValue` to save the original calculated value for a cell that contains a formula; this affects the result of `getOldCalculatedValue`. The set routine automatically casts numeric values to float, which is not appropriate when the calculated value is explicitly set to a string type. An optional parameter is added to the set routine to indicate whether a cast is appropriate (default is true so userland uses of this method will remain unaffected). The readers for Xlsx, Xls, Xml, Ods, and Slk are all changed to set the new parameter appropriately. Gnumeric saves the formula, but does not appear to save the result, so no change is needed there. Html and Csv save the result but not the formula (for preCalculateFormulas true - they save the formula but not the result for false), so no change is needed for them.

It is not clear what the use case is for oldCalculatedValue, but it does date back to PhpExcel. A possible use case is demonstrated in the new tests - Excel function INFO is not implemented, so getCalculatedValue will generally return null on such a cell, but will return oldCalculatedValue when it sees that the function is not implemented but oldCalculatedValue has been set (typically by loading an existing spreadsheet), so the result that is on the spreadsheet will be available to the program which loaded it.

Reading a spreadsheet which has been created with preCalculateFormulas false will, or at least should, result in a null oldCalculatedValue. This was true for Xls, Xml, and Slk. Xlsx was actually storing 0 in this situation; that wasn't precisely a problem, but it wasn't necessary and seems misleading. Xlsx Writer is changed to no longer set a value in this situation. This is technically a break, but the existing code was wrong; one unit test needed a very minor modification as a result of this change.

src/PhpSpreadsheet/Cell/Cell.php

@@ -418,10 +418,10 @@ public function getCalculatedValue(bool $resetLog = true)
      *
      * @param mixed $originalValue Value
      */
-    public function setCalculatedValue($originalValue): self
+    public function setCalculatedValue($originalValue, bool $tryNumeric = true): self
     {
         if ($originalValue !== null) {
-            $this->calculatedValue = (is_numeric($originalValue)) ? (float) $originalValue : $originalValue;
+            $this->calculatedValue = ($tryNumeric && is_numeric($originalValue)) ? (0 + $originalValue) : $originalValue;
         }
 
         return $this->updateInCollection();

src/PhpSpreadsheet/Reader/Ods.php

@@ -607,7 +607,7 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)
                                                 }
 
                                                 if ($hasCalculatedValue) {
-                                                    $cell->setCalculatedValue($dataValue);
+                                                    $cell->setCalculatedValue($dataValue, $type === DataType::TYPE_NUMERIC);
                                                 }
 
                                                 // Set other properties

src/PhpSpreadsheet/Reader/Slk.php

@@ -270,6 +270,7 @@ private function processCRecord(array $rowData, Spreadsheet &$spreadsheet, strin
     {
         //    Read cell value data
         $hasCalculatedValue = false;
+        $tryNumeric = false;
         $cellDataFormula = $cellData = '';
         foreach ($rowData as $rowDatum) {
             switch ($rowDatum[0]) {
@@ -285,6 +286,7 @@ private function processCRecord(array $rowData, Spreadsheet &$spreadsheet, strin
                     break;
                 case 'K':
                     $cellData = substr($rowDatum, 1);
+                    $tryNumeric = is_numeric($cellData);
 
                     break;
                 case 'E':
@@ -306,16 +308,16 @@ private function processCRecord(array $rowData, Spreadsheet &$spreadsheet, strin
         $cellData = Calculation::unwrapResult($cellData);
 
         // Set cell value
-        $this->processCFinal($spreadsheet, $hasCalculatedValue, $cellDataFormula, $cellData, "$columnLetter$row");
+        $this->processCFinal($spreadsheet, $hasCalculatedValue, $cellDataFormula, $cellData, "$columnLetter$row", $tryNumeric);
     }
 
-    private function processCFinal(Spreadsheet &$spreadsheet, bool $hasCalculatedValue, string $cellDataFormula, string $cellData, string $coordinate): void
+    private function processCFinal(Spreadsheet &$spreadsheet, bool $hasCalculatedValue, string $cellDataFormula, string $cellData, string $coordinate, bool $tryNumeric): void
     {
         // Set cell value
         $spreadsheet->getActiveSheet()->getCell($coordinate)->setValue(($hasCalculatedValue) ? $cellDataFormula : $cellData);
         if ($hasCalculatedValue) {
             $cellData = Calculation::unwrapResult($cellData);
-            $spreadsheet->getActiveSheet()->getCell($coordinate)->setCalculatedValue($cellData);
+            $spreadsheet->getActiveSheet()->getCell($coordinate)->setCalculatedValue($cellData, $tryNumeric);
         }
     }
 

src/PhpSpreadsheet/Reader/Xls.php

@@ -4042,7 +4042,7 @@ private function readFormula(): void
             }
 
             // store the cached calculated value
-            $cell->setCalculatedValue($value);
+            $cell->setCalculatedValue($value, $dataType === DataType::TYPE_NUMERIC);
         }
     }
 

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -843,6 +843,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                             $r = Coordinate::stringFromColumnIndex($rowIndex) . $cIndex;
                                         }
                                         $cellDataType = (string) $cAttr['t'];
+                                        $originalCellDataTypeNumeric = $cellDataType === '';
                                         $value = null;
                                         $calculatedValue = null;
 
@@ -949,7 +950,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                 $cell->setValue($value);
                                             }
                                             if ($calculatedValue !== null) {
-                                                $cell->setCalculatedValue($calculatedValue);
+                                                $cell->setCalculatedValue($calculatedValue, $originalCellDataTypeNumeric);
                                             }
 
                                             // Style information?

src/PhpSpreadsheet/Reader/Xml.php

@@ -468,6 +468,7 @@ public function loadIntoExisting(string $filename, Spreadsheet $spreadsheet, boo
                                 }
                             }
 
+                            $originalType = $type;
                             if ($hasCalculatedValue) {
                                 $type = DataType::TYPE_FORMULA;
                                 $columnNumber = Coordinate::columnIndexFromString($columnID);
@@ -476,7 +477,7 @@ public function loadIntoExisting(string $filename, Spreadsheet $spreadsheet, boo
 
                             $spreadsheet->getActiveSheet()->getCell($columnID . $rowID)->setValueExplicit((($hasCalculatedValue) ? $cellDataFormula : $cellValue), $type);
                             if ($hasCalculatedValue) {
-                                $spreadsheet->getActiveSheet()->getCell($columnID . $rowID)->setCalculatedValue($cellValue);
+                                $spreadsheet->getActiveSheet()->getCell($columnID . $rowID)->setCalculatedValue($cellValue, $originalType === DataType::TYPE_NUMERIC);
                             }
                             $rowHasData = true;
                         }

src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php

@@ -1311,9 +1311,11 @@ private function writeCellFormula(XMLWriter $objWriter, string $cellValue, Cell
             $objWriter->writeElement('f', FunctionPrefix::addFunctionPrefixStripEquals($cellValue));
             self::writeElementIf(
                 $objWriter,
-                $this->getParentWriter()->getOffice2003Compatibility() === false,
+                $this->getParentWriter()->getOffice2003Compatibility() === false
+                && $this->getParentWriter()->getPreCalculateFormulas()
+                && $calculatedValue !== null,
                 'v',
-                ($this->getParentWriter()->getPreCalculateFormulas() && !is_array($calculatedValue) && substr($calculatedValue ?? '', 0, 1) !== '#')
+                (!is_array($calculatedValue) && substr($calculatedValue ?? '', 0, 1) !== '#')
                     ? StringHelper::formatNumber($calculatedValue) : '0'
             );
         }

tests/PhpSpreadsheetTests/Functional/TypeAttributePreservationTest.php

@@ -2,7 +2,13 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Functional;
 
+use PhpOffice\PhpSpreadsheet\Reader\Ods as ReaderOds;
+use PhpOffice\PhpSpreadsheet\Reader\Slk as ReaderSlk;
+use PhpOffice\PhpSpreadsheet\Reader\Xls as ReaderXls;
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as ReaderXlsx;
+use PhpOffice\PhpSpreadsheet\Reader\Xml as ReaderXml;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Writer\Xlsx as WriterXlsx;
 
 class TypeAttributePreservationTest extends AbstractFunctional
 {
@@ -46,5 +52,124 @@ public function testFormulae($format, array $values): void
         }
 
         self::assertSame($expected, $actual);
+        $spreadsheet->disconnectWorksheets();
+        $reloadedSpreadsheet->disconnectWorksheets();
+    }
+
+    public static function customizeWriter(WriterXlsx $writer): void
+    {
+        $writer->setPreCalculateFormulas(false);
+    }
+
+    /**
+     * Ensure saved spreadsheets maintain the correct data type.
+     *
+     * @dataProvider providerFormulae
+     *
+     * @param string $format
+     */
+    public function testFormulaeNoPrecalc($format, array $values): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->fromArray($values);
+
+        /** @var callable */
+        $writerCustomizer = [self::class, 'customizeWriter'];
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format, null, $writerCustomizer);
+        $reloadedSheet = $reloadedSpreadsheet->getActiveSheet();
+
+        $expected = $sheet->getCell('A1')->getCalculatedValue();
+
+        if ($sheet->getCell('A1')->getDataType() === 'f') {
+            $actual = $reloadedSheet->getCell('A1')->getOldCalculatedValue();
+            self::assertNull($actual);
+        } else {
+            $actual = $reloadedSheet->getCell('A1')->getValue();
+            self::assertSame($expected, $actual);
+        }
+
+        $spreadsheet->disconnectWorksheets();
+        $reloadedSpreadsheet->disconnectWorksheets();
+    }
+
+    public function testUnimplementedFunctionXlsx(): void
+    {
+        $file = 'tests/data/Reader/XLSX/issue.3658.xlsx';
+        $reader = new ReaderXlsx();
+        $spreadsheet = $reader->load($file);
+        $sheet = $spreadsheet->getActiveSheet();
+        $unimplemented = '=INFO("SYSTEM")';
+        self::assertSame(124, $sheet->getCell('A2')->getOldCalculatedValue());
+        self::assertSame('00123', $sheet->getCell('A3')->getOldCalculatedValue());
+        self::assertSame($unimplemented, $sheet->getCell('A4')->getValue());
+        self::assertSame('pcdos', $sheet->getCell('A4')->getCalculatedValue(), 'use oldCalculatedValue from read for unimplemented function');
+        $sheet->getCell('D10')->setValue($unimplemented);
+        self::assertNull($sheet->getCell('D10')->getCalculatedValue(), 'return null for unimplemented function when old value unassigned');
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testUnimplementedFunctionXls(): void
+    {
+        $file = 'tests/data/Reader/XLS/issue.3658.xls';
+        $reader = new ReaderXls();
+        $spreadsheet = $reader->load($file);
+        $sheet = $spreadsheet->getActiveSheet();
+        $unimplemented = '=INFO("SYSTEM")';
+        self::assertSame(124, $sheet->getCell('A2')->getOldCalculatedValue());
+        self::assertSame('00123', $sheet->getCell('A3')->getOldCalculatedValue());
+        self::assertSame($unimplemented, $sheet->getCell('A4')->getValue());
+        self::assertSame('pcdos', $sheet->getCell('A4')->getCalculatedValue(), 'use oldCalculatedValue from read for unimplemented function');
+        $sheet->getCell('D10')->setValue($unimplemented);
+        self::assertNull($sheet->getCell('D10')->getCalculatedValue(), 'return null for unimplemented function when old value unassigned');
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testUnimplementedFunctionXml(): void
+    {
+        $file = 'tests/data/Reader/Xml/issue.3658.xml';
+        $reader = new ReaderXml();
+        $spreadsheet = $reader->load($file);
+        $sheet = $spreadsheet->getActiveSheet();
+        $unimplemented = '=INFO("SYSTEM")';
+        self::assertSame(124, $sheet->getCell('A2')->getOldCalculatedValue());
+        self::assertSame('00123', $sheet->getCell('A3')->getOldCalculatedValue());
+        self::assertSame($unimplemented, $sheet->getCell('A4')->getValue());
+        self::assertSame('pcdos', $sheet->getCell('A4')->getCalculatedValue(), 'use oldCalculatedValue from read for unimplemented function');
+        $sheet->getCell('D10')->setValue($unimplemented);
+        self::assertNull($sheet->getCell('D10')->getCalculatedValue(), 'return null for unimplemented function when old value unassigned');
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testUnimplementedFunctionOds(): void
+    {
+        $file = 'tests/data/Reader/Ods/issue.3658.ods';
+        $reader = new ReaderOds();
+        $spreadsheet = $reader->load($file);
+        $sheet = $spreadsheet->getActiveSheet();
+        $unimplemented = '=INFO("SYSTEM")';
+        self::assertSame(124, $sheet->getCell('A2')->getOldCalculatedValue());
+        self::assertSame('00123', $sheet->getCell('A3')->getOldCalculatedValue());
+        self::assertSame($unimplemented, $sheet->getCell('A4')->getValue());
+        self::assertSame('WNT', $sheet->getCell('A4')->getCalculatedValue(), 'use oldCalculatedValue from read for unimplemented function');
+        $sheet->getCell('D10')->setValue($unimplemented);
+        self::assertNull($sheet->getCell('D10')->getCalculatedValue(), 'return null for unimplemented function when old value unassigned');
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testUnimplementedFunctionSlk(): void
+    {
+        $file = 'tests/data/Reader/Slk/issue.3658.slk';
+        $reader = new ReaderSlk();
+        $spreadsheet = $reader->load($file);
+        $sheet = $spreadsheet->getActiveSheet();
+        $unimplemented = '=INFO("SYSTEM")';
+        self::assertSame(124, $sheet->getCell('A2')->getOldCalculatedValue());
+        self::assertSame('00123', $sheet->getCell('A3')->getOldCalculatedValue());
+        self::assertSame($unimplemented, $sheet->getCell('A4')->getValue());
+        self::assertSame('pcdos', $sheet->getCell('A4')->getCalculatedValue(), 'use oldCalculatedValue from read for unimplemented function');
+        $sheet->getCell('D10')->setValue($unimplemented);
+        self::assertNull($sheet->getCell('D10')->getCalculatedValue(), 'return null for unimplemented function when old value unassigned');
+        $spreadsheet->disconnectWorksheets();
     }
 }

tests/PhpSpreadsheetTests/Writer/PreCalcTest.php

@@ -106,7 +106,7 @@ private function verifyXlsx(?bool $preCalc, string $type): void
             $data = self::readFile($file);
             // confirm that file contains B2 pre-calculated or not as appropriate
             if ($preCalc === false) {
-                self::assertStringContainsString('<c r="B2" t="str"><f>3+A3</f><v>0</v></c>', $data);
+                self::assertStringContainsString('<c r="B2" t="str"><f>3+A3</f></c>', $data);
             } else {
                 self::assertStringContainsString('<c r="B2"><f>3+A3</f><v>14</v></c>', $data);
             }

tests/data/Functional/TypeAttributePreservation/Formula.php

@@ -37,4 +37,10 @@
     [
         [null],
     ],
+    'issue3568' => [
+        ['="00"&B1', '123'],
+    ],
+    'unimplemented function' => [
+        ['=INFO("SYSTEM")'],
+    ],
 ];

tests/data/Reader/Slk/issue.3658.slk

@@ -0,0 +1,85 @@
+ID;PWXL;N;E
+P;PGeneral
+P;P0
+P;P0.00
+P;P#,##0
+P;P#,##0.00
+P;P#,##0_);;\(#,##0\)
+P;P#,##0_);;[Red]\(#,##0\)
+P;P#,##0.00_);;\(#,##0.00\)
+P;P#,##0.00_);;[Red]\(#,##0.00\)
+P;P"$"#,##0_);;\("$"#,##0\)
+P;P"$"#,##0_);;[Red]\("$"#,##0\)
+P;P"$"#,##0.00_);;\("$"#,##0.00\)
+P;P"$"#,##0.00_);;[Red]\("$"#,##0.00\)
+P;P0%
+P;P0.00%
+P;P0.00E+00
+P;P##0.0E+0
+P;P#\ ?/?
+P;P#\ ??/??
+P;Pyyyy/mm/dd
+P;Pdd/mmm/yy
+P;Pdd/mmm
+P;Pmmm/yy
+P;Ph:mm\ AM/PM
+P;Ph:mm:ss\ AM/PM
+P;Ph:mm
+P;Ph:mm:ss
+P;Pyyyy/mm/dd\ h:mm
+P;Pmm:ss
+P;Pmm:ss.0
+P;P@
+P;P[h]:mm:ss
+P;P_("$"* #,##0_);;_("$"* \(#,##0\);;_("$"* "-"_);;_(@_)
+P;P_(* #,##0_);;_(* \(#,##0\);;_(* "-"_);;_(@_)
+P;P_("$"* #,##0.00_);;_("$"* \(#,##0.00\);;_("$"* "-"??_);;_(@_)
+P;P_(* #,##0.00_);;_(* \(#,##0.00\);;_(* "-"??_);;_(@_)
+P;FCalibri;M220;L9
+P;FCalibri;M220;L9
+P;FCalibri;M220;L9
+P;FCalibri;M220;L9
+P;ECalibri;M220;L9
+P;ECalibri Light;M360;L55
+P;ECalibri;M300;SB;L55
+P;ECalibri;M260;SB;L55
+P;ECalibri;M220;SB;L55
+P;ECalibri;M220;L18
+P;ECalibri;M220;L21
+P;ECalibri;M220;L61
+P;ECalibri;M220;L63
+P;ECalibri;M220;SB;L64
+P;ECalibri;M220;SB;L53
+P;ECalibri;M220;L53
+P;ECalibri;M220;SB;L10
+P;ECalibri;M220;L11
+P;ECalibri;M220;SI;L24
+P;ECalibri;M220;SB;L9
+P;ECalibri;M220;L10
+P;ECalibri;M220;L9
+P;ECalibri Light;M360;L55
+P;ECalibri;M300;SB;L55
+P;ECalibri;M260;SB;L55
+P;ECalibri;M220;SB;L55
+P;ECalibri;M220;L18
+P;ECalibri;M220;L21
+P;ECalibri;M220;L61
+P;ECalibri;M220;L63
+P;ECalibri;M220;SB;L64
+P;ECalibri;M220;SB;L53
+P;ECalibri;M220;L53
+P;ECalibri;M220;SB;L10
+P;ECalibri;M220;L11
+P;ECalibri;M220;SI;L24
+P;ECalibri;M220;SB;L9
+P;ECalibri;M220;L10
+P;ESegoe UI;M200;L9
+P;ECalibri;M220;L9
+F;P0;DG0G8;M290
+B;Y4;X1;D0 0 3 0
+O;L;D;V0;G100 0.001
+C;Y1;X1;K123
+C;Y2;K124;ER[-1]C+1
+C;Y3;K"00123";E"00"&R[-2]C
+C;Y4;K"pcdos";EINFO("SYSTEM")
+E