Skip to content

Commit

Permalink
WIP Handle REF Error as Part of Range
Browse files Browse the repository at this point in the history
Fix PHPOffice#3453. User sets a valid formula (e.g. `=SUM(Sheet2!B1:Sheet2!B3)`), and then does something to invalidate the formula (e.g. delete Sheet2). Excel changes the formula to `SUM(#REF!:#REF!)` when the spreadsheet is saved; apparently someone thought this was a good idea. But PhpSpreadsheet (a) used to throw an Exception when it evaluated the formula, and (b) now gives a result of `0` when evaluating the formula. Neither is ideal. It would be better to propagate the `#REF!` error.

It is likely that more tests are needed, which is why I will keep this in draft status for a bit.
  • Loading branch information
oleibman committed Mar 18, 2023
1 parent 4b7aa20 commit 2e19cf5
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 6 deletions.
29 changes: 23 additions & 6 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4364,8 +4364,12 @@ private function internalParseFormula($formula, ?Cell $cell = null)
$rangeStartCellRef = $output[count($output) - 2]['value'] ?? '';
}
preg_match('/^' . self::CALCULATION_REGEXP_CELLREF . '$/miu', $rangeStartCellRef, $rangeStartMatches);
if ($rangeStartMatches[2] > '') {
$val = $rangeStartMatches[2] . '!' . $val;
if (array_key_exists(2, $rangeStartMatches)) {
if ($rangeStartMatches[2] > '') {
$val = $rangeStartMatches[2] . '!' . $val;
}
} else {
$val = Information\ExcelError::REF();
}
} else {
$rangeStartCellRef = $output[count($output) - 1]['value'] ?? '';
Expand Down Expand Up @@ -4434,6 +4438,8 @@ private function internalParseFormula($formula, ?Cell $cell = null)
}
$val = $address;
}
} elseif ($val === Information\ExcelError::REF()) {
$stackItemReference = $val;
} else {
$startRowColRef = $output[count($output) - 1]['value'] ?? '';
[$rangeWS1, $startRowColRef] = Worksheet::extractSheetTitle($startRowColRef, true);
Expand Down Expand Up @@ -4793,7 +4799,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)
}
}
}
if (strpos($operand1Data['reference'], '!') !== false) {
if (strpos($operand1Data['reference'] ?? '', '!') !== false) {
[$sheet1, $operand1Data['reference']] = Worksheet::extractSheetTitle($operand1Data['reference'], true);
} else {
$sheet1 = ($pCellWorksheet !== null) ? $pCellWorksheet->getTitle() : '';
Expand Down Expand Up @@ -4830,10 +4836,21 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)

$oData = array_merge(explode(':', $operand1Data['reference']), explode(':', $operand2Data['reference']));
$oCol = $oRow = [];
$breakNeeded = false;
foreach ($oData as $oDatum) {
$oCR = Coordinate::coordinateFromString($oDatum);
$oCol[] = Coordinate::columnIndexFromString($oCR[0]) - 1;
$oRow[] = $oCR[1];
try {
$oCR = Coordinate::coordinateFromString($oDatum);
$oCol[] = Coordinate::columnIndexFromString($oCR[0]) - 1;
$oRow[] = $oCR[1];
} catch (\Exception $e) {
$stack->push('Error', Information\ExcelError::REF(), null);
$breakNeeded = true;

break;
}
}
if ($breakNeeded) {
break;
}
$cellRef = Coordinate::stringFromColumnIndex(min($oCol) + 1) . min($oRow) . ':' . Coordinate::stringFromColumnIndex(max($oCol) + 1) . max($oRow);
if ($pCellParent !== null && $this->spreadsheet !== null) {
Expand Down
45 changes: 45 additions & 0 deletions tests/PhpSpreadsheetTests/RefRangeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests;

use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class RefRangeTest extends TestCase
{
/**
* @param int|string $expectedResult
*
* @dataProvider providerRefRange
*/
public function testRefRange($expectedResult, string $rangeString): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->getCell('A1')->setValue("=SUM($rangeString)");
self::assertSame($expectedResult, $sheet->getCell('A1')->getCalculatedValue());
$spreadsheet->disconnectWorksheets();
}

public function providerRefRange(): array
{
return [
'normal range' => [0, 'B1:B2'],
'ref as end of range' => ['#REF!', 'B1:#REF!'],
'ref as start of range' => ['#REF!', '#REF!:B2'],
'ref as both parts of range' => ['#REF!', '#REF!:#REF!'],
'using indirect for ref' => ['#REF!', 'B1:INDIRECT("XYZ")'],
];
}

public function testRefRangeRead(): void
{
$reader = new Xlsx();
$spreadsheet = $reader->load('tests/data/Reader/XLSX/issue.3453.xlsx');
$sheet = $spreadsheet->getActiveSheet();
self::assertSame(0, $sheet->getCell('H1')->getCalculatedValue());
self::assertSame('#REF!', $sheet->getCell('H2')->getCalculatedValue());
$spreadsheet->disconnectWorksheets();
}
}
Binary file added tests/data/Reader/XLSX/issue.3453.xlsx
Binary file not shown.

0 comments on commit 2e19cf5

Please sign in to comment.