Skip to content

Commit fda4855

Browse files
oleibmanoleibman
committed
Anchor Cell Without Spill
Some examples submitted by @infojunkie have demonstrated that an anchor cell without a spill operator was not being handled consistently with Excel. This means that the calculated value of such a cell needs to be a scalar when using it as part of a formula without the spill operator, but as an array when using it with the spill operator or when just getting the cell's value. This is tricky. My solution seems awfully kludgey to me, but it does seem to work. I have another change that I will want to make in a day or so. When that change is pushed, I will take this back out of draft status, and will now aim for an install date of about August 6. This particular commit removes the changing of array return type during calculations. It's been this way for a very long time, but I don't understand why it should have been needed in the first place. It causes problems (you need to be sure to restore the original value even when, for example, you throw an exception during calculation). I am relieved that it caused a problem only for one test member. The TEXTSPLIT function really makes sense only when you are returning arrays as arrays. Its test now needs to set that value explicitly since the Calculation engine is no longer changing it under the covers. No other tests broke as a result of this change. One other test "broke" as a result of this commit. One of the tests for INDIRECT had been expecting a null value, and the test was commented with "Excel result is 0". The PhpSpreadsheet result is now 0 as well, so this would seem to be a bugfix rather than a breaking change.
1 parent 5b18dcf commit fda4855

File tree

4 files changed

+123
-25
lines changed

4 files changed

+123
-25
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ class Calculation
124124

125125
private bool $suppressFormulaErrors = false;
126126

127+
private bool $processingAnchorArray = false;
128+
127129
/**
128130
* Error message for any error that was raised/thrown by the calculation engine.
129131
*/
@@ -3450,15 +3452,12 @@ public function calculateCellValue(?Cell $cell = null, bool $resetLog = true): m
34503452
return null;
34513453
}
34523454

3453-
$returnArrayAsType = self::$returnArrayAsType;
34543455
if ($resetLog) {
34553456
// Initialise the logging settings if requested
34563457
$this->formulaError = null;
34573458
$this->debugLog->clearLog();
34583459
$this->cyclicReferenceStack->clear();
34593460
$this->cyclicFormulaCounter = 1;
3460-
3461-
self::$returnArrayAsType = self::RETURN_ARRAY_AS_ARRAY;
34623461
}
34633462

34643463
// Execute the calculation for the cell formula
@@ -3503,36 +3502,17 @@ public function calculateCellValue(?Cell $cell = null, bool $resetLog = true): m
35033502
$testSheet->getCell($cellAddress['cell']);
35043503
}
35053504
}
3506-
self::$returnArrayAsType = $returnArrayAsType;
35073505

35083506
throw new Exception($e->getMessage(), $e->getCode(), $e);
35093507
}
35103508

35113509
if ((is_array($result)) && (self::$returnArrayAsType != self::RETURN_ARRAY_AS_ARRAY)) {
3512-
self::$returnArrayAsType = $returnArrayAsType;
35133510
$testResult = Functions::flattenArray($result);
35143511
if (self::$returnArrayAsType == self::RETURN_ARRAY_AS_ERROR) {
35153512
return ExcelError::VALUE();
35163513
}
3517-
// If there's only a single cell in the array, then we allow it
3518-
if (count($testResult) != 1) {
3519-
// If keys are numeric, then it's a matrix result rather than a cell range result, so we permit it
3520-
$r = array_keys($result);
3521-
$r = array_shift($r);
3522-
if (!is_numeric($r)) {
3523-
return ExcelError::VALUE();
3524-
}
3525-
if (is_array($result[$r])) {
3526-
$c = array_keys($result[$r]);
3527-
$c = array_shift($c);
3528-
if (!is_numeric($c)) {
3529-
return ExcelError::VALUE();
3530-
}
3531-
}
3532-
}
35333514
$result = array_shift($testResult);
35343515
}
3535-
self::$returnArrayAsType = $returnArrayAsType;
35363516

35373517
if ($result === null && $cell->getWorksheet()->getSheetView()->getShowZeros()) {
35383518
return 0;
@@ -4576,7 +4556,13 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
45764556
// help us to know when pruning ['branchTestId' => true/false]
45774557
$branchStore = [];
45784558
// Loop through each token in turn
4559+
$tokenIdx = -1;
45794560
foreach ($tokens as $tokenData) {
4561+
++$tokenIdx;
4562+
$this->processingAnchorArray = false;
4563+
if ($tokenData['type'] === 'Cell Reference' && isset($tokens[$tokenIdx + 1]) && $tokens[$tokenIdx + 1]['type'] === 'Operand Count for Function ANCHORARRAY()') {
4564+
$this->processingAnchorArray = true;
4565+
}
45804566
$token = $tokenData['value'];
45814567
// Branch pruning: skip useless resolutions
45824568
$storeKey = $tokenData['storeKey'] ?? null;
@@ -4983,6 +4969,13 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
49834969
}
49844970
}
49854971

4972+
if (self::$returnArrayAsType === self::RETURN_ARRAY_AS_ARRAY && !$this->processingAnchorArray && is_array($cellValue)) {
4973+
while (is_array($cellValue)) {
4974+
$cellValue = array_shift($cellValue);
4975+
}
4976+
$this->debugLog->writeDebugLog('Scalar Result for cell %s is %s', $cellRef, $this->showTypeDetails($cellValue));
4977+
}
4978+
$this->processingAnchorArray = false;
49864979
$stack->push('Cell Value', $cellValue, $cellRef);
49874980
if (isset($storeKey)) {
49884981
$branchStore[$storeKey] = $cellValue;
@@ -5439,7 +5432,13 @@ public function extractCellRange(string &$range = 'A1', ?Worksheet $worksheet =
54395432
// Single cell in range
54405433
sscanf($aReferences[0], '%[A-Z]%d', $currentCol, $currentRow);
54415434
if ($worksheet !== null && $worksheet->cellExists($aReferences[0])) {
5442-
$returnValue[$currentRow][$currentCol] = $worksheet->getCell($aReferences[0])->getCalculatedValue($resetLog);
5435+
$temp = $worksheet->getCell($aReferences[0])->getCalculatedValue($resetLog);
5436+
if (self::$returnArrayAsType === self::RETURN_ARRAY_AS_ARRAY) {
5437+
while (is_array($temp)) {
5438+
$temp = array_shift($temp);
5439+
}
5440+
}
5441+
$returnValue[$currentRow][$currentCol] = $temp;
54435442
} else {
54445443
$returnValue[$currentRow][$currentCol] = null;
54455444
}
@@ -5449,7 +5448,13 @@ public function extractCellRange(string &$range = 'A1', ?Worksheet $worksheet =
54495448
// Extract range
54505449
sscanf($reference, '%[A-Z]%d', $currentCol, $currentRow);
54515450
if ($worksheet !== null && $worksheet->cellExists($reference)) {
5452-
$returnValue[$currentRow][$currentCol] = $worksheet->getCell($reference)->getCalculatedValue($resetLog);
5451+
$temp = $worksheet->getCell($reference)->getCalculatedValue($resetLog);
5452+
if (self::$returnArrayAsType === self::RETURN_ARRAY_AS_ARRAY) {
5453+
while (is_array($temp)) {
5454+
$temp = array_shift($temp);
5455+
}
5456+
}
5457+
$returnValue[$currentRow][$currentCol] = $temp;
54535458
} else {
54545459
$returnValue[$currentRow][$currentCol] = null;
54555460
}

‎tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public static function providerRelative(): array
170170
'absolute row absolute column' => ['c2', 'R2C3'],
171171
'absolute row relative column' => ['a2', 'R2C[-1]'],
172172
'relative row absolute column lowercase' => ['a2', 'rc1'],
173-
'uninitialized cell' => [null, 'RC[+2]'], // Excel result is 0
173+
'uninitialized cell' => [0, 'RC[+2]'], // Excel result is 0, PhpSpreadsheet was null
174174
];
175175
}
176176

‎tests/PhpSpreadsheetTests/Calculation/Functions/TextData/TextSplitTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,21 @@
99

1010
class TextSplitTest extends AllSetupTeardown
1111
{
12+
private string $returnType;
13+
14+
protected function setUp(): void
15+
{
16+
parent::setUp();
17+
$this->returnType = Calculation::getInstance($this->getSpreadsheet())->getArrayReturnType();
18+
Calculation::getInstance($this->getSpreadsheet())->setArrayReturnType(Calculation::RETURN_ARRAY_AS_ARRAY);
19+
}
20+
21+
protected function tearDown(): void
22+
{
23+
Calculation::getInstance($this->getSpreadsheet())->setArrayReturnType($this->returnType);
24+
parent::tearDown();
25+
}
26+
1227
private function setDelimiterArgument(array $argument, string $column): string
1328
{
1429
return '{' . $column . implode(',' . $column, range(1, count($argument))) . '}';

‎tests/PhpSpreadsheetTests/Functional/ArrayFunctionsCellTest.php

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Functional;
6+
7+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
8+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
9+
use PHPUnit\Framework\TestCase;
10+
11+
class ArrayFunctionsCellTest extends TestCase
12+
{
13+
private string $arrayReturnType;
14+
15+
protected function setUp(): void
16+
{
17+
$this->arrayReturnType = Calculation::getArrayReturnType();
18+
}
19+
20+
protected function tearDown(): void
21+
{
22+
Calculation::setArrayReturnType($this->arrayReturnType);
23+
}
24+
25+
public function testArrayAndNonArrayOutput(): void
26+
{
27+
Calculation::setArrayReturnType(Calculation::RETURN_ARRAY_AS_ARRAY);
28+
$spreadsheet = new Spreadsheet();
29+
$calculation = Calculation::getInstance($spreadsheet);
30+
31+
$sheet = $spreadsheet->getActiveSheet();
32+
$sheet->fromArray(
33+
[
34+
[1.0, 0.0, 1.0],
35+
[0.0, 2.0, 0.0],
36+
[0.0, 0.0, 1.0],
37+
],
38+
strictNullComparison: true
39+
);
40+
$sheet->setCellValue('E1', '=MINVERSE(A1:C3)');
41+
$sheet->setCellValue('I1', '=E1#');
42+
$sheet->setCellValue('M1', '=MMULT(E1#,I1#)');
43+
$sheet->setCellValue('E6', '=SUM(E1)');
44+
$sheet->setCellValue('E7', '=SUM(SINGLE(E1))');
45+
$sheet->setCellValue('E8', '=SUM(E1#)');
46+
$sheet->setCellValue('I6', '=E1+I1');
47+
$sheet->setCellValue('J6', '=E1#+I1#');
48+
49+
$expectedE1 = [
50+
[1.0, 0.0, -1.0],
51+
[0.0, 0.5, 0.0],
52+
[0.0, 0.0, 1.0],
53+
];
54+
self::assertSame($expectedE1, $sheet->getCell('E1')->getCalculatedValue(), 'MINVERSE function');
55+
self::assertSame($expectedE1, $sheet->getCell('I1')->getCalculatedValue(), 'Assignment with spill operator');
56+
57+
$expectedM1 = [
58+
[1.0, 0.0, -2.0],
59+
[0.0, 0.25, 0.0],
60+
[0.0, 0.0, 1.0],
61+
];
62+
self::assertSame($expectedM1, $sheet->getCell('M1')->getCalculatedValue(), 'MMULT with 2 spill operators');
63+
64+
self::assertSame(1.0, $sheet->getCell('E6')->getCalculatedValue(), 'SUM referring to anchor cell');
65+
self::assertSame(1.0, $sheet->getCell('E7')->getCalculatedValue(), 'SUM referring to anchor cell wrapped in Single');
66+
self::assertSame(1.5, $sheet->getCell('E8')->getCalculatedValue(), 'SUM referring to anchor cell with Spill Operator');
67+
68+
self::assertSame(2.0, $sheet->getCell('I6')->getCalculatedValue(), 'addition operator for 2 anchor cells');
69+
$expectedJ6 = [
70+
[2.0, 0.0, -2.0],
71+
[0.0, 1.0, 0.0],
72+
[0.0, 0.0, 2.0],
73+
];
74+
self::assertSame($expectedJ6, $sheet->getCell('J6')->getCalculatedValue(), 'addition operator for 2 anchor cells with Spill operators');
75+
76+
$spreadsheet->disconnectWorksheets();
77+
}
78+
}

0 commit comments

Comments
 (0)