From 1c6dd8923efc08bc753d5421560b454b0ceff521 Mon Sep 17 00:00:00 2001
From: oleibman <eclipsechasers2@yahoo.com>
Date: Sat, 4 Jan 2020 09:34:21 -0800
Subject: [PATCH] Handle Error in Formula Processing Better for Xls (#1267)

* Handle Error in Formula Processing Better for Xls

When there is an error writing a formula to an Xls file,
which seems to happen when a defined name is part of a formula,
the cell is currently left blank. A better result would be to
write the calculated value of the formula.

* Making Changes Suggested in Review

Per comment from Mark Baker:
   1. Made return codes from writeFormula into constants.
   2. Skipped redundant call to getCellValue when possible.
   3. Added support for bool type, adding additional tests
      for bool and string.
Per comment from PowerKiki:
   1. Used standardized convention for assigning file name in test.
      Since before this change, save would throw Exception, I kept
      the unlink for the file in tearDown.
---
 src/PhpSpreadsheet/Writer/Xls/Worksheet.php   | 42 ++++++++++++++---
 .../Writer/Xls/FormulaErrTest.php             | 45 +++++++++++++++++++
 2 files changed, 80 insertions(+), 7 deletions(-)
 create mode 100644 tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php

diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
index d8822d80..baa45345 100644
--- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
+++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
@@ -441,7 +441,29 @@ class Worksheet extends BIFFwriter
                     case DataType::TYPE_FORMULA:
                         $calculatedValue = $this->preCalculateFormulas ?
                             $cell->getCalculatedValue() : null;
-                        $this->writeFormula($row, $column, $cVal, $xfIndex, $calculatedValue);
+                        if (self::WRITE_FORMULA_EXCEPTION == $this->writeFormula($row, $column, $cVal, $xfIndex, $calculatedValue)) {
+                            if ($calculatedValue === null) {
+                                $calculatedValue = $cell->getCalculatedValue();
+                            }
+                            $calctype = gettype($calculatedValue);
+                            switch ($calctype) {
+                                case 'integer':
+                                case 'double':
+                                    $this->writeNumber($row, $column, $calculatedValue, $xfIndex);
+
+                                    break;
+                                case 'string':
+                                    $this->writeString($row, $column, $calculatedValue, $xfIndex);
+
+                                    break;
+                                case 'boolean':
+                                    $this->writeBoolErr($row, $column, $calculatedValue, 0, $xfIndex);
+
+                                    break;
+                                default:
+                                    $this->writeString($row, $column, $cVal, $xfIndex);
+                            }
+                        }
 
                         break;
                     case DataType::TYPE_BOOL:
@@ -764,14 +786,20 @@ class Worksheet extends BIFFwriter
         return 0;
     }
 
+    const WRITE_FORMULA_NORMAL = 0;
+    const WRITE_FORMULA_ERRORS = -1;
+    const WRITE_FORMULA_RANGE = -2;
+    const WRITE_FORMULA_EXCEPTION = -3;
+
     /**
      * Write a formula to the specified row and column (zero indexed).
      * The textual representation of the formula is passed to the parser in
      * Parser.php which returns a packed binary string.
      *
-     * Returns  0 : normal termination
-     *         -1 : formula errors (bad formula)
-     *         -2 : row or column out of range
+     * Returns  0 : WRITE_FORMULA_NORMAL  normal termination
+     *         -1 : WRITE_FORMULA_ERRORS formula errors (bad formula)
+     *         -2 : WRITE_FORMULA_RANGE  row or column out of range
+     *         -3 : WRITE_FORMULA_EXCEPTION parse raised exception, probably due to definedname
      *
      * @param int $row Zero indexed row
      * @param int $col Zero indexed column
@@ -829,7 +857,7 @@ class Worksheet extends BIFFwriter
             // Error handling
             $this->writeString($row, $col, 'Unrecognised character for formula', 0);
 
-            return -1;
+            return self::WRITE_FORMULA_ERRORS;
         }
 
         // Parse the formula using the parser in Parser.php
@@ -852,9 +880,9 @@ class Worksheet extends BIFFwriter
                 $this->writeStringRecord($stringValue);
             }
 
-            return 0;
+            return self::WRITE_FORMULA_NORMAL;
         } catch (PhpSpreadsheetException $e) {
-            // do nothing
+            return self::WRITE_FORMULA_EXCEPTION;
         }
     }
 
diff --git a/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php b/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php
new file mode 100644
index 00000000..2c615b72
--- /dev/null
+++ b/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Xls\Workbook;
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\NamedRange;
+use PhpOffice\PhpSpreadsheet\Shared\File;
+use PHPUnit\Framework\TestCase;
+
+class FormulaErrTest extends TestCase
+{
+    public function tearDown()
+    {
+        $filename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test');
+        if (file_exists($filename)) {
+            unlink($filename);
+        }
+    }
+
+    public function testFormulaError()
+    {
+        $obj = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
+        $sheet0 = $obj->setActiveSheetIndex(0);
+        $sheet0->setCellValue('A1', 2);
+        $obj->addNamedRange(new NamedRange('DEFNAM', $sheet0, 'A1'));
+        $sheet0->setCellValue('B1', '=2*DEFNAM');
+        $sheet0->setCellValue('C1', '=DEFNAM=2');
+        $sheet0->setCellValue('D1', '=CONCAT("X",DEFNAM)');
+        $writer = IOFactory::createWriter($obj, 'Xls');
+        $filename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test');
+        $writer->save($filename);
+        $reader = IOFactory::createReader('Xls');
+        $robj = $reader->load($filename);
+        $sheet0 = $robj->setActiveSheetIndex(0);
+        $a1 = $sheet0->getCell('A1')->getCalculatedValue();
+        self::assertEquals(2, $a1);
+        $b1 = $sheet0->getCell('B1')->getCalculatedValue();
+        self::assertEquals(4, $b1);
+        $c1 = $sheet0->getCell('C1')->getCalculatedValue();
+        $tru = true;
+        self::assertEquals($tru, $c1);
+        $d1 = $sheet0->getCell('D1')->getCalculatedValue();
+        self::assertEquals('X2', $d1);
+    }
+}