Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct Re-computation of Relative Addresses in Defined Names #3673

Merged
merged 3 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -5745,7 +5745,8 @@ private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksh

$this->debugLog->writeDebugLog('Defined Name is a %s with a value of %s', $definedNameType, $definedNameValue);

$recursiveCalculationCell = ($definedNameWorksheet !== null && $definedNameWorksheet !== $cellWorksheet)
$originalCoordinate = $cell->getCoordinate();
$recursiveCalculationCell = ($definedNameType !== 'Formula' && $definedNameWorksheet !== null && $definedNameWorksheet !== $cellWorksheet)
? $definedNameWorksheet->getCell('A1')
: $cell;
$recursiveCalculationCellAddress = $recursiveCalculationCell->getCoordinate();
Expand All @@ -5763,6 +5764,7 @@ private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksh
$recursiveCalculator->getDebugLog()->setWriteDebugLog($this->getDebugLog()->getWriteDebugLog());
$recursiveCalculator->getDebugLog()->setEchoDebugLog($this->getDebugLog()->getEchoDebugLog());
$result = $recursiveCalculator->_calculateFormulaValue($definedNameValue, $recursiveCalculationCellAddress, $recursiveCalculationCell, true);
$cellWorksheet->getCell($originalCoordinate);

if ($this->getDebugLog()->getWriteDebugLog()) {
$this->debugLog->mergeDebugLog(array_slice($recursiveCalculator->getDebugLog()->getLog(), 3));
Expand Down
7 changes: 5 additions & 2 deletions src/PhpSpreadsheet/CellReferenceHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function refreshRequired(string $beforeCellAddress, int $numberOfColumns,
$this->numberOfRows !== $numberOfRows;
}

public function updateCellReference(string $cellReference = 'A1', bool $includeAbsoluteReferences = false): string
public function updateCellReference(string $cellReference = 'A1', bool $includeAbsoluteReferences = false, bool $onlyAbsoluteReferences = false): string
{
if (Coordinate::coordinateIsRange($cellReference)) {
throw new Exception('Only single cell references may be passed to this method.');
Expand All @@ -70,7 +70,10 @@ public function updateCellReference(string $cellReference = 'A1', bool $includeA
$absoluteColumn = $newColumn[0] === '$' ? '$' : '';
$absoluteRow = $newRow[0] === '$' ? '$' : '';
// Verify which parts should be updated
if ($includeAbsoluteReferences === false) {
if ($onlyAbsoluteReferences === true) {
$updateColumn = (($absoluteColumn === '$') && $newColumnIndex >= $this->beforeColumn);
$updateRow = (($absoluteRow === '$') && $newRowIndex >= $this->beforeRow);
} elseif ($includeAbsoluteReferences === false) {
$updateColumn = (($absoluteColumn !== '$') && $newColumnIndex >= $this->beforeColumn);
$updateRow = (($absoluteRow !== '$') && $newRowIndex >= $this->beforeRow);
} else {
Expand Down
40 changes: 22 additions & 18 deletions src/PhpSpreadsheet/ReferenceHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PhpOffice\PhpSpreadsheet;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Cell\AddressRange;
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Style\Conditional;
Expand Down Expand Up @@ -572,7 +573,8 @@ public function updateFormulaReferences(
$numberOfColumns = 0,
$numberOfRows = 0,
$worksheetName = '',
bool $includeAbsoluteReferences = false
bool $includeAbsoluteReferences = false,
bool $onlyAbsoluteReferences = false
) {
if (
$this->cellReferenceHelper === null ||
Expand All @@ -596,8 +598,8 @@ public function updateFormulaReferences(
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = substr($this->updateCellReference('$A' . $match[3], $includeAbsoluteReferences), 2);
$modified4 = substr($this->updateCellReference('$A' . $match[4], $includeAbsoluteReferences), 2);
$modified3 = substr($this->updateCellReference('$A' . $match[3], $includeAbsoluteReferences, $onlyAbsoluteReferences), 2);
$modified4 = substr($this->updateCellReference('$A' . $match[4], $includeAbsoluteReferences, $onlyAbsoluteReferences), 2);

if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -621,8 +623,8 @@ public function updateFormulaReferences(
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = substr($this->updateCellReference($match[3] . '$1', $includeAbsoluteReferences), 0, -2);
$modified4 = substr($this->updateCellReference($match[4] . '$1', $includeAbsoluteReferences), 0, -2);
$modified3 = substr($this->updateCellReference($match[3] . '$1', $includeAbsoluteReferences, $onlyAbsoluteReferences), 0, -2);
$modified4 = substr($this->updateCellReference($match[4] . '$1', $includeAbsoluteReferences, $onlyAbsoluteReferences), 0, -2);

if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -646,8 +648,8 @@ public function updateFormulaReferences(
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
$modified4 = $this->updateCellReference($match[4], $includeAbsoluteReferences);
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences, $onlyAbsoluteReferences);
$modified4 = $this->updateCellReference($match[4], $includeAbsoluteReferences, $onlyAbsoluteReferences);

if ($match[3] . $match[4] !== $modified3 . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -674,7 +676,7 @@ public function updateFormulaReferences(
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3];

$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences, $onlyAbsoluteReferences);
if ($match[3] !== $modified3) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
$toString = ($match[2] > '') ? $match[2] . '!' : '';
Expand Down Expand Up @@ -757,11 +759,13 @@ private function updateCellReferencesAllWorksheets(string $formula, int $numberO
$row = $rows[$splitCount][0];

if (!empty($column) && $column[0] !== '$') {
$column = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($column) + $numberOfColumns);
$column = ((Coordinate::columnIndexFromString($column) + $numberOfColumns) % AddressRange::MAX_COLUMN_INT) ?: AddressRange::MAX_COLUMN_INT;
$column = Coordinate::stringFromColumnIndex($column);
$rowOffset -= ($columnLength - strlen($column));
$formula = substr($formula, 0, $columnOffset) . $column . substr($formula, $columnOffset + $columnLength);
}
if (!empty($row) && $row[0] !== '$') {
$row = (int) $row + $numberOfRows;
$row = (((int) $row + $numberOfRows) % AddressRange::MAX_ROW) ?: AddressRange::MAX_ROW;
$formula = substr($formula, 0, $rowOffset) . $row . substr($formula, $rowOffset + $rowLength);
}
}
Expand Down Expand Up @@ -854,7 +858,7 @@ private function updateRowRangesAllWorksheets(string $formula, int $numberOfRows
*
* @return string Updated cell range
*/
private function updateCellReference($cellReference = 'A1', bool $includeAbsoluteReferences = false)
private function updateCellReference($cellReference = 'A1', bool $includeAbsoluteReferences = false, bool $onlyAbsoluteReferences = false)
{
// Is it in another worksheet? Will not have to update anything.
if (strpos($cellReference, '!') !== false) {
Expand All @@ -863,11 +867,11 @@ private function updateCellReference($cellReference = 'A1', bool $includeAbsolut
// Is it a range or a single cell?
if (!Coordinate::coordinateIsRange($cellReference)) {
// Single cell
return $this->cellReferenceHelper->updateCellReference($cellReference, $includeAbsoluteReferences);
return $this->cellReferenceHelper->updateCellReference($cellReference, $includeAbsoluteReferences, $onlyAbsoluteReferences);
}

// Range
return $this->updateCellRange($cellReference, $includeAbsoluteReferences);
return $this->updateCellRange($cellReference, $includeAbsoluteReferences, $onlyAbsoluteReferences);
}

/**
Expand Down Expand Up @@ -922,7 +926,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet
* them with a #REF!
*/
if ($asFormula === true) {
$formula = $this->updateFormulaReferences($cellAddress, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle(), true);
$formula = $this->updateFormulaReferences($cellAddress, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle(), true, true);
$definedName->setValue($formula);
} else {
$definedName->setValue($this->updateCellReference(ltrim($cellAddress, '='), true));
Expand Down Expand Up @@ -953,7 +957,7 @@ private function updateNamedFormula(DefinedName $definedName, Worksheet $workshe
*
* @return string Updated cell range
*/
private function updateCellRange(string $cellRange = 'A1:A1', bool $includeAbsoluteReferences = false): string
private function updateCellRange(string $cellRange = 'A1:A1', bool $includeAbsoluteReferences = false, bool $onlyAbsoluteReferences = false): string
{
if (!Coordinate::coordinateIsRange($cellRange)) {
throw new Exception('Only cell ranges may be passed to this method.');
Expand All @@ -967,14 +971,14 @@ private function updateCellRange(string $cellRange = 'A1:A1', bool $includeAbsol
for ($j = 0; $j < $jc; ++$j) {
if (ctype_alpha($range[$i][$j])) {
$range[$i][$j] = Coordinate::coordinateFromString(
$this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1', $includeAbsoluteReferences)
$this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1', $includeAbsoluteReferences, $onlyAbsoluteReferences)
)[0];
} elseif (ctype_digit($range[$i][$j])) {
$range[$i][$j] = Coordinate::coordinateFromString(
$this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j], $includeAbsoluteReferences)
$this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j], $includeAbsoluteReferences, $onlyAbsoluteReferences)
)[1];
} else {
$range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j], $includeAbsoluteReferences);
$range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j], $includeAbsoluteReferences, $onlyAbsoluteReferences);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\LookupRef;

use PhpOffice\PhpSpreadsheet\NamedFormula;
use PhpOffice\PhpSpreadsheet\NamedRange;
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as ReaderXlsx;

Expand Down Expand Up @@ -176,4 +177,33 @@ public static function providerRelative(): array
'uninitialized cell' => [null, 'RC[+2]'], // Excel result is 0
];
}

/** @var bool */
private static $definedFormulaWorking = false;

public function testAboveCell(): void
{
$spreadsheet = $this->getSpreadsheet();
$spreadsheet->addNamedFormula(
new NamedFormula('SumAbove', $spreadsheet->getActiveSheet(), '=SUM(INDIRECT(ADDRESS(1,COLUMN())):INDIRECT(ADDRESS(ROW()-1,COLUMN())))')
);
$sheet = $this->getSheet();
$sheet->getCell('A1')->setValue(100);
$sheet->getCell('A2')->setValue(200);
$sheet->getCell('A3')->setValue(300);
$sheet->getCell('A4')->setValue(400);
$sheet->getCell('A5')->setValue(500);
$sheet->getCell('A6')->setValue('=SUM(A$1:INDIRECT(ADDRESS(ROW()-1,COLUMN())))');
self::AssertSame(1500, $sheet->getCell('A6')->getCalculatedValue());
if (self::$definedFormulaWorking) {
$sheet->getCell('B1')->setValue(10);
$sheet->getCell('B2')->setValue(20);
$sheet->getCell('B3')->setValue(30);
$sheet->getCell('B4')->setValue(40);
$sheet->getCell('B5')->setValue('=SumAbove');
self::AssertSame(100, $sheet->getCell('B5')->getCalculatedValue());
} else {
self::markTestIncomplete('PhpSpreadsheet does not handle this correctly');
}
}
}
113 changes: 113 additions & 0 deletions tests/PhpSpreadsheetTests/ReferenceHelper3Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\NamedRange;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class ReferenceHelper3Test extends TestCase
{
public function testIssue3661(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setTitle('Data');

$spreadsheet->addNamedRange(new NamedRange('FIRST', $sheet, '=$A1'));
$spreadsheet->addNamedRange(new NamedRange('SECOND', $sheet, '=$B1'));
$spreadsheet->addNamedRange(new NamedRange('THIRD', $sheet, '=$C1'));

$sheet->fromArray([
[1, 2, 3, '=FIRST', '=SECOND', '=THIRD', '=10*$A1'],
[4, 5, 6, '=FIRST', '=SECOND', '=THIRD'],
[7, 8, 9, '=FIRST', '=SECOND', '=THIRD'],
]);

$sheet->insertNewRowBefore(1, 4);
$sheet->insertNewColumnBefore('A', 1);
self::assertSame(1, $sheet->getCell('E5')->getCalculatedValue());
self::assertSame(5, $sheet->getCell('F6')->getCalculatedValue());
self::assertSame(9, $sheet->getCell('G7')->getCalculatedValue());
self::assertSame('=10*$B5', $sheet->getCell('H5')->getValue());
self::assertSame(10, $sheet->getCell('H5')->getCalculatedValue());
$firstColumn = $spreadsheet->getNamedRange('FIRST');
/** @var NamedRange $firstColumn */
self::assertSame('=$B1', $firstColumn->getRange());
$spreadsheet->disconnectWorksheets();
}

public function testCompletelyRelative(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setTitle('Data');

$spreadsheet->addNamedRange(new NamedRange('CellAbove', $sheet, '=A1048576'));
$spreadsheet->addNamedRange(new NamedRange('CellBelow', $sheet, '=A2'));
$spreadsheet->addNamedRange(new NamedRange('CellToLeft', $sheet, '=XFD1'));
$spreadsheet->addNamedRange(new NamedRange('CellToRight', $sheet, '=B1'));

$sheet->fromArray([
[null, 'Above', null, null, 'Above', null, null, 'Above', null, null, 'Above', null],
['Left', '=CellAbove', 'Right', 'Left', '=CellBelow', 'Right', 'Left', '=CellToLeft', 'Right', 'Left', '=CellToRight', 'Right'],
[null, 'Below', null, null, 'Below', null, null, 'Below', null, null, 'Below', null],
], null, 'A1', true);
self::assertSame('Above', $sheet->getCell('B2')->getCalculatedValue());
self::assertSame('Below', $sheet->getCell('E2')->getCalculatedValue());
self::assertSame('Left', $sheet->getCell('H2')->getCalculatedValue());
self::assertSame('Right', $sheet->getCell('K2')->getCalculatedValue());

Calculation::getInstance($spreadsheet)->flushInstance();
self::assertNull($sheet->getCell('L7')->getCalculatedValue(), 'value in L7 after flush is null');
// Reset it once more
Calculation::getInstance($spreadsheet)->flushInstance();
// shift 5 rows down and 1 column to the right
$sheet->insertNewRowBefore(1, 5);
$sheet->insertNewColumnBefore('A', 1);

self::assertSame('Above', $sheet->getCell('C7')->getCalculatedValue()); // Above
self::assertSame('Below', $sheet->getCell('F7')->getCalculatedValue());
self::assertSame('Left', $sheet->getCell('I7')->getCalculatedValue());
self::assertSame('Right', $sheet->getCell('L7')->getCalculatedValue());

$sheet2 = $spreadsheet->createSheet();
$sheet2->setCellValue('L6', 'NotThisCell');
$sheet2->setCellValue('L7', '=CellAbove');
self::assertSame('Above', $sheet2->getCell('L7')->getCalculatedValue(), 'relative value uses cell on worksheet where name is defined');
$spreadsheet->disconnectWorksheets();
}

/** @var bool */
private static $sumFormulaWorking = false;

public function testSumAboveCell(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$spreadsheet->addNamedRange(new NamedRange('AboveCell', $sheet, 'A1048576'));
$sheet->setCellValue('C2', 123);
$sheet->setCellValue('C3', '=AboveCell');
$sheet->fromArray([
['Column 1', 'Column 2'],
[2, 1],
[4, 3],
[6, 5],
[8, 7],
[10, 9],
[12, 11],
[14, 13],
[16, 15],
['=SUM(A2:AboveCell)', '=SUM(B2:AboveCell)'],
], null, 'A1', true);
self::assertSame(123, $sheet->getCell('C3')->getCalculatedValue());
if (self::$sumFormulaWorking) {
self::assertSame(72, $sheet->getCell('A10')->getCalculatedValue());
} else {
$spreadsheet->disconnectWorksheets();
self::markTestIncomplete('PhpSpreadsheet does not handle this correctly');
}
$spreadsheet->disconnectWorksheets();
}
}
Loading