Skip to content

Commit cd9f165

Browse files
authored
WIP Handle REF Error as Part of Range (#3467)
Fix #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.
1 parent 1f32fa4 commit cd9f165

File tree

3 files changed

+68
-6
lines changed

3 files changed

+68
-6
lines changed

src/PhpSpreadsheet/Calculation/Calculation.php

+23-6
Original file line numberDiff line numberDiff line change
@@ -4364,8 +4364,12 @@ private function internalParseFormula($formula, ?Cell $cell = null)
43644364
$rangeStartCellRef = $output[count($output) - 2]['value'] ?? '';
43654365
}
43664366
preg_match('/^' . self::CALCULATION_REGEXP_CELLREF . '$/miu', $rangeStartCellRef, $rangeStartMatches);
4367-
if ($rangeStartMatches[2] > '') {
4368-
$val = $rangeStartMatches[2] . '!' . $val;
4367+
if (array_key_exists(2, $rangeStartMatches)) {
4368+
if ($rangeStartMatches[2] > '') {
4369+
$val = $rangeStartMatches[2] . '!' . $val;
4370+
}
4371+
} else {
4372+
$val = Information\ExcelError::REF();
43694373
}
43704374
} else {
43714375
$rangeStartCellRef = $output[count($output) - 1]['value'] ?? '';
@@ -4434,6 +4438,8 @@ private function internalParseFormula($formula, ?Cell $cell = null)
44344438
}
44354439
$val = $address;
44364440
}
4441+
} elseif ($val === Information\ExcelError::REF()) {
4442+
$stackItemReference = $val;
44374443
} else {
44384444
$startRowColRef = $output[count($output) - 1]['value'] ?? '';
44394445
[$rangeWS1, $startRowColRef] = Worksheet::extractSheetTitle($startRowColRef, true);
@@ -4793,7 +4799,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)
47934799
}
47944800
}
47954801
}
4796-
if (strpos($operand1Data['reference'], '!') !== false) {
4802+
if (strpos($operand1Data['reference'] ?? '', '!') !== false) {
47974803
[$sheet1, $operand1Data['reference']] = Worksheet::extractSheetTitle($operand1Data['reference'], true);
47984804
} else {
47994805
$sheet1 = ($pCellWorksheet !== null) ? $pCellWorksheet->getTitle() : '';
@@ -4830,10 +4836,21 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)
48304836

48314837
$oData = array_merge(explode(':', $operand1Data['reference']), explode(':', $operand2Data['reference']));
48324838
$oCol = $oRow = [];
4839+
$breakNeeded = false;
48334840
foreach ($oData as $oDatum) {
4834-
$oCR = Coordinate::coordinateFromString($oDatum);
4835-
$oCol[] = Coordinate::columnIndexFromString($oCR[0]) - 1;
4836-
$oRow[] = $oCR[1];
4841+
try {
4842+
$oCR = Coordinate::coordinateFromString($oDatum);
4843+
$oCol[] = Coordinate::columnIndexFromString($oCR[0]) - 1;
4844+
$oRow[] = $oCR[1];
4845+
} catch (\Exception $e) {
4846+
$stack->push('Error', Information\ExcelError::REF(), null);
4847+
$breakNeeded = true;
4848+
4849+
break;
4850+
}
4851+
}
4852+
if ($breakNeeded) {
4853+
break;
48374854
}
48384855
$cellRef = Coordinate::stringFromColumnIndex(min($oCol) + 1) . min($oRow) . ':' . Coordinate::stringFromColumnIndex(max($oCol) + 1) . max($oRow);
48394856
if ($pCellParent !== null && $this->spreadsheet !== null) {
+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests;
4+
5+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
6+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
7+
use PHPUnit\Framework\TestCase;
8+
9+
class RefRangeTest extends TestCase
10+
{
11+
/**
12+
* @param int|string $expectedResult
13+
*
14+
* @dataProvider providerRefRange
15+
*/
16+
public function testRefRange($expectedResult, string $rangeString): void
17+
{
18+
$spreadsheet = new Spreadsheet();
19+
$sheet = $spreadsheet->getActiveSheet();
20+
$sheet->getCell('A1')->setValue("=SUM($rangeString)");
21+
self::assertSame($expectedResult, $sheet->getCell('A1')->getCalculatedValue());
22+
$spreadsheet->disconnectWorksheets();
23+
}
24+
25+
public function providerRefRange(): array
26+
{
27+
return [
28+
'normal range' => [0, 'B1:B2'],
29+
'ref as end of range' => ['#REF!', 'B1:#REF!'],
30+
'ref as start of range' => ['#REF!', '#REF!:B2'],
31+
'ref as both parts of range' => ['#REF!', '#REF!:#REF!'],
32+
'using indirect for ref' => ['#REF!', 'B1:INDIRECT("XYZ")'],
33+
];
34+
}
35+
36+
public function testRefRangeRead(): void
37+
{
38+
$reader = new Xlsx();
39+
$spreadsheet = $reader->load('tests/data/Reader/XLSX/issue.3453.xlsx');
40+
$sheet = $spreadsheet->getActiveSheet();
41+
self::assertSame(0, $sheet->getCell('H1')->getCalculatedValue());
42+
self::assertSame('#REF!', $sheet->getCell('H2')->getCalculatedValue());
43+
$spreadsheet->disconnectWorksheets();
44+
}
45+
}
9.37 KB
Binary file not shown.

0 commit comments

Comments
 (0)