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.
This commit is contained in:
oleibman 2020-01-04 09:34:21 -08:00 committed by Mark Baker
parent 1b2c99b190
commit 1c6dd8923e
2 changed files with 80 additions and 7 deletions

View File

@ -441,7 +441,29 @@ class Worksheet extends BIFFwriter
case DataType::TYPE_FORMULA: case DataType::TYPE_FORMULA:
$calculatedValue = $this->preCalculateFormulas ? $calculatedValue = $this->preCalculateFormulas ?
$cell->getCalculatedValue() : null; $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; break;
case DataType::TYPE_BOOL: case DataType::TYPE_BOOL:
@ -764,14 +786,20 @@ class Worksheet extends BIFFwriter
return 0; 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). * Write a formula to the specified row and column (zero indexed).
* The textual representation of the formula is passed to the parser in * The textual representation of the formula is passed to the parser in
* Parser.php which returns a packed binary string. * Parser.php which returns a packed binary string.
* *
* Returns 0 : normal termination * Returns 0 : WRITE_FORMULA_NORMAL normal termination
* -1 : formula errors (bad formula) * -1 : WRITE_FORMULA_ERRORS formula errors (bad formula)
* -2 : row or column out of range * -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 $row Zero indexed row
* @param int $col Zero indexed column * @param int $col Zero indexed column
@ -829,7 +857,7 @@ class Worksheet extends BIFFwriter
// Error handling // Error handling
$this->writeString($row, $col, 'Unrecognised character for formula', 0); $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 // Parse the formula using the parser in Parser.php
@ -852,9 +880,9 @@ class Worksheet extends BIFFwriter
$this->writeStringRecord($stringValue); $this->writeStringRecord($stringValue);
} }
return 0; return self::WRITE_FORMULA_NORMAL;
} catch (PhpSpreadsheetException $e) { } catch (PhpSpreadsheetException $e) {
// do nothing return self::WRITE_FORMULA_EXCEPTION;
} }
} }

View File

@ -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);
}
}