From fda48554b2c249970307781947c96b0af0562271 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 9 Jul 2024 20:50:43 -0700 Subject: [PATCH] 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. --- .../Calculation/Calculation.php | 53 +++++++------ .../Functions/LookupRef/IndirectTest.php | 2 +- .../Functions/TextData/TextSplitTest.php | 15 ++++ .../Functional/ArrayFunctionsCellTest.php | 78 +++++++++++++++++++ 4 files changed, 123 insertions(+), 25 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Functional/ArrayFunctionsCellTest.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 36d74df97..bbba6de5f 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -124,6 +124,8 @@ class Calculation private bool $suppressFormulaErrors = false; + private bool $processingAnchorArray = false; + /** * Error message for any error that was raised/thrown by the calculation engine. */ @@ -3450,15 +3452,12 @@ class Calculation return null; } - $returnArrayAsType = self::$returnArrayAsType; if ($resetLog) { // Initialise the logging settings if requested $this->formulaError = null; $this->debugLog->clearLog(); $this->cyclicReferenceStack->clear(); $this->cyclicFormulaCounter = 1; - - self::$returnArrayAsType = self::RETURN_ARRAY_AS_ARRAY; } // Execute the calculation for the cell formula @@ -3503,36 +3502,17 @@ class Calculation $testSheet->getCell($cellAddress['cell']); } } - self::$returnArrayAsType = $returnArrayAsType; throw new Exception($e->getMessage(), $e->getCode(), $e); } if ((is_array($result)) && (self::$returnArrayAsType != self::RETURN_ARRAY_AS_ARRAY)) { - self::$returnArrayAsType = $returnArrayAsType; $testResult = Functions::flattenArray($result); if (self::$returnArrayAsType == self::RETURN_ARRAY_AS_ERROR) { return ExcelError::VALUE(); } - // If there's only a single cell in the array, then we allow it - if (count($testResult) != 1) { - // If keys are numeric, then it's a matrix result rather than a cell range result, so we permit it - $r = array_keys($result); - $r = array_shift($r); - if (!is_numeric($r)) { - return ExcelError::VALUE(); - } - if (is_array($result[$r])) { - $c = array_keys($result[$r]); - $c = array_shift($c); - if (!is_numeric($c)) { - return ExcelError::VALUE(); - } - } - } $result = array_shift($testResult); } - self::$returnArrayAsType = $returnArrayAsType; if ($result === null && $cell->getWorksheet()->getSheetView()->getShowZeros()) { return 0; @@ -4576,7 +4556,13 @@ class Calculation // help us to know when pruning ['branchTestId' => true/false] $branchStore = []; // Loop through each token in turn + $tokenIdx = -1; foreach ($tokens as $tokenData) { + ++$tokenIdx; + $this->processingAnchorArray = false; + if ($tokenData['type'] === 'Cell Reference' && isset($tokens[$tokenIdx + 1]) && $tokens[$tokenIdx + 1]['type'] === 'Operand Count for Function ANCHORARRAY()') { + $this->processingAnchorArray = true; + } $token = $tokenData['value']; // Branch pruning: skip useless resolutions $storeKey = $tokenData['storeKey'] ?? null; @@ -4983,6 +4969,13 @@ class Calculation } } + if (self::$returnArrayAsType === self::RETURN_ARRAY_AS_ARRAY && !$this->processingAnchorArray && is_array($cellValue)) { + while (is_array($cellValue)) { + $cellValue = array_shift($cellValue); + } + $this->debugLog->writeDebugLog('Scalar Result for cell %s is %s', $cellRef, $this->showTypeDetails($cellValue)); + } + $this->processingAnchorArray = false; $stack->push('Cell Value', $cellValue, $cellRef); if (isset($storeKey)) { $branchStore[$storeKey] = $cellValue; @@ -5439,7 +5432,13 @@ class Calculation // Single cell in range sscanf($aReferences[0], '%[A-Z]%d', $currentCol, $currentRow); if ($worksheet !== null && $worksheet->cellExists($aReferences[0])) { - $returnValue[$currentRow][$currentCol] = $worksheet->getCell($aReferences[0])->getCalculatedValue($resetLog); + $temp = $worksheet->getCell($aReferences[0])->getCalculatedValue($resetLog); + if (self::$returnArrayAsType === self::RETURN_ARRAY_AS_ARRAY) { + while (is_array($temp)) { + $temp = array_shift($temp); + } + } + $returnValue[$currentRow][$currentCol] = $temp; } else { $returnValue[$currentRow][$currentCol] = null; } @@ -5449,7 +5448,13 @@ class Calculation // Extract range sscanf($reference, '%[A-Z]%d', $currentCol, $currentRow); if ($worksheet !== null && $worksheet->cellExists($reference)) { - $returnValue[$currentRow][$currentCol] = $worksheet->getCell($reference)->getCalculatedValue($resetLog); + $temp = $worksheet->getCell($reference)->getCalculatedValue($resetLog); + if (self::$returnArrayAsType === self::RETURN_ARRAY_AS_ARRAY) { + while (is_array($temp)) { + $temp = array_shift($temp); + } + } + $returnValue[$currentRow][$currentCol] = $temp; } else { $returnValue[$currentRow][$currentCol] = null; } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php index e58c9d5bf..4b29a787c 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php @@ -170,7 +170,7 @@ class IndirectTest extends AllSetupTeardown 'absolute row absolute column' => ['c2', 'R2C3'], 'absolute row relative column' => ['a2', 'R2C[-1]'], 'relative row absolute column lowercase' => ['a2', 'rc1'], - 'uninitialized cell' => [null, 'RC[+2]'], // Excel result is 0 + 'uninitialized cell' => [0, 'RC[+2]'], // Excel result is 0, PhpSpreadsheet was null ]; } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/TextSplitTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/TextSplitTest.php index 985e08567..5ccaa7c17 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/TextSplitTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/TextSplitTest.php @@ -9,6 +9,21 @@ use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class TextSplitTest extends AllSetupTeardown { + private string $returnType; + + protected function setUp(): void + { + parent::setUp(); + $this->returnType = Calculation::getInstance($this->getSpreadsheet())->getArrayReturnType(); + Calculation::getInstance($this->getSpreadsheet())->setArrayReturnType(Calculation::RETURN_ARRAY_AS_ARRAY); + } + + protected function tearDown(): void + { + Calculation::getInstance($this->getSpreadsheet())->setArrayReturnType($this->returnType); + parent::tearDown(); + } + private function setDelimiterArgument(array $argument, string $column): string { return '{' . $column . implode(',' . $column, range(1, count($argument))) . '}'; diff --git a/tests/PhpSpreadsheetTests/Functional/ArrayFunctionsCellTest.php b/tests/PhpSpreadsheetTests/Functional/ArrayFunctionsCellTest.php new file mode 100644 index 000000000..459c889c6 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Functional/ArrayFunctionsCellTest.php @@ -0,0 +1,78 @@ +arrayReturnType = Calculation::getArrayReturnType(); + } + + protected function tearDown(): void + { + Calculation::setArrayReturnType($this->arrayReturnType); + } + + public function testArrayAndNonArrayOutput(): void + { + Calculation::setArrayReturnType(Calculation::RETURN_ARRAY_AS_ARRAY); + $spreadsheet = new Spreadsheet(); + $calculation = Calculation::getInstance($spreadsheet); + + $sheet = $spreadsheet->getActiveSheet(); + $sheet->fromArray( + [ + [1.0, 0.0, 1.0], + [0.0, 2.0, 0.0], + [0.0, 0.0, 1.0], + ], + strictNullComparison: true + ); + $sheet->setCellValue('E1', '=MINVERSE(A1:C3)'); + $sheet->setCellValue('I1', '=E1#'); + $sheet->setCellValue('M1', '=MMULT(E1#,I1#)'); + $sheet->setCellValue('E6', '=SUM(E1)'); + $sheet->setCellValue('E7', '=SUM(SINGLE(E1))'); + $sheet->setCellValue('E8', '=SUM(E1#)'); + $sheet->setCellValue('I6', '=E1+I1'); + $sheet->setCellValue('J6', '=E1#+I1#'); + + $expectedE1 = [ + [1.0, 0.0, -1.0], + [0.0, 0.5, 0.0], + [0.0, 0.0, 1.0], + ]; + self::assertSame($expectedE1, $sheet->getCell('E1')->getCalculatedValue(), 'MINVERSE function'); + self::assertSame($expectedE1, $sheet->getCell('I1')->getCalculatedValue(), 'Assignment with spill operator'); + + $expectedM1 = [ + [1.0, 0.0, -2.0], + [0.0, 0.25, 0.0], + [0.0, 0.0, 1.0], + ]; + self::assertSame($expectedM1, $sheet->getCell('M1')->getCalculatedValue(), 'MMULT with 2 spill operators'); + + self::assertSame(1.0, $sheet->getCell('E6')->getCalculatedValue(), 'SUM referring to anchor cell'); + self::assertSame(1.0, $sheet->getCell('E7')->getCalculatedValue(), 'SUM referring to anchor cell wrapped in Single'); + self::assertSame(1.5, $sheet->getCell('E8')->getCalculatedValue(), 'SUM referring to anchor cell with Spill Operator'); + + self::assertSame(2.0, $sheet->getCell('I6')->getCalculatedValue(), 'addition operator for 2 anchor cells'); + $expectedJ6 = [ + [2.0, 0.0, -2.0], + [0.0, 1.0, 0.0], + [0.0, 0.0, 2.0], + ]; + self::assertSame($expectedJ6, $sheet->getCell('J6')->getCalculatedValue(), 'addition operator for 2 anchor cells with Spill operators'); + + $spreadsheet->disconnectWorksheets(); + } +}