WIP Bug in resizeMatricesExtend

Fix #4451. The reporter's analysis is correct - the method has a bug. The solution is relatively easy. Nevertheless, proving that the fix works as desired is tricky. The submitter's Reflection test is good, but it would be better to find one within Excel. My additional tests are contrived, for reasons explained in the test member. So, I will leave this as a work in progress while I search for something better. If that search doesn't succeed, I will nevertheless merge this in about a month.
This commit is contained in:
oleibman
2025-05-16 20:46:09 -07:00
parent 3922d9ad00
commit 7b38d8c07f
3 changed files with 137 additions and 9 deletions
@@ -845,15 +845,15 @@ class Calculation extends CalculationLocale
if (($matrix2Columns < $matrix1Columns) || ($matrix2Rows < $matrix1Rows)) {
if ($matrix2Columns < $matrix1Columns) {
for ($i = 0; $i < $matrix2Rows; ++$i) {
$x = $matrix2[$i][$matrix2Columns - 1];
$x = ($matrix2Columns === 1) ? $matrix2[$i][0] : null;
for ($j = $matrix2Columns; $j < $matrix1Columns; ++$j) {
$matrix2[$i][$j] = $x;
}
}
}
if ($matrix2Rows < $matrix1Rows) {
$x = $matrix2[$matrix2Rows - 1];
for ($i = 0; $i < $matrix1Rows; ++$i) {
$x = ($matrix2Rows === 1) ? $matrix2[0] : array_fill(0, $matrix2Columns, null);
for ($i = $matrix2Rows; $i < $matrix1Rows; ++$i) {
$matrix2[$i] = $x;
}
}
@@ -862,15 +862,15 @@ class Calculation extends CalculationLocale
if (($matrix1Columns < $matrix2Columns) || ($matrix1Rows < $matrix2Rows)) {
if ($matrix1Columns < $matrix2Columns) {
for ($i = 0; $i < $matrix1Rows; ++$i) {
$x = $matrix1[$i][$matrix1Columns - 1];
$x = ($matrix1Columns === 1) ? $matrix1[$i][0] : null;
for ($j = $matrix1Columns; $j < $matrix2Columns; ++$j) {
$matrix1[$i][$j] = $x;
}
}
}
if ($matrix1Rows < $matrix2Rows) {
$x = $matrix1[$matrix1Rows - 1];
for ($i = 0; $i < $matrix2Rows; ++$i) {
$x = ($matrix1Rows === 1) ? $matrix1[0] : array_fill(0, $matrix1Columns, null);
for ($i = $matrix1Rows; $i < $matrix2Rows; ++$i) {
$matrix1[$i] = $x;
}
}
@@ -2334,14 +2334,14 @@ class Calculation extends CalculationLocale
for ($row = 0; $row < $rows; ++$row) {
for ($column = 0; $column < $columns; ++$column) {
if ($operand1[$row][$column] === null) {
if (($operand1[$row][$column] ?? null) === null) {
$operand1[$row][$column] = 0;
} elseif (!self::isNumericOrBool($operand1[$row][$column])) {
$operand1[$row][$column] = self::makeError($operand1[$row][$column]);
continue;
}
if ($operand2[$row][$column] === null) {
if (($operand2[$row][$column] ?? null) === null) {
$operand2[$row][$column] = 0;
} elseif (!self::isNumericOrBool($operand2[$row][$column])) {
$operand1[$row][$column] = self::makeError($operand2[$row][$column]);
@@ -45,7 +45,7 @@ class Filter
return array_filter(
array_values($lookupArray),
fn ($index): bool => (bool) $matchArray[$index],
fn ($index): bool => (bool) ($matchArray[$index] ?? null),
ARRAY_FILTER_USE_KEY
);
}
@@ -0,0 +1,128 @@
<?php
declare(strict_types=1);
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;
use ReflectionMethod;
class Issue4451Test extends TestCase
{
public static function testReflect(): void
{
// Sample matrices to test with
$matrix1 = [[1], [3]];
$matrix2 = [[5], [8], [11]];
// Use reflection to make the protected method accessible
$calculation = new Calculation();
$reflectionMethod = new ReflectionMethod(Calculation::class, 'resizeMatricesExtend');
$reflectionMethod->setAccessible(true);
// Call the method using reflection
$reflectionMethod->invokeArgs($calculation, [&$matrix1, &$matrix2, count($matrix1), 1, count($matrix2), 1]);
self::assertSame([[1], [3], [null]], $matrix1); //* @phpstan-ignore-line
}
/**
* These 2 tests are contrived. They prove that method
* works as desired, but Excel will actually return
* a CALC error, a result I don't know how to duplicate.
*/
public static function testExtendFirstColumn(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setTitle('Products');
$calculationEngine = Calculation::getInstance($spreadsheet);
$calculationEngine->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_ARRAY
);
$sheet->getCell('D5')->setValue(5);
$sheet->getCell('E5')->setValue(20);
$sheet->fromArray(
[
[5, 20, 'Apples'],
[10, 20, 'Bananas'],
[5, 20, 'Cherries'],
[5, 40, 'Grapes'],
[25, 50, 'Peaches'],
[30, 60, 'Pears'],
[35, 70, 'Papayas'],
[40, 80, 'Mangos'],
[null, 20, 'Unknown'],
],
null,
'K1',
true
);
$kRows = $sheet->getHighestDataRow('K');
self::assertSame(8, $kRows);
$lRows = $sheet->getHighestDataRow('L');
self::assertSame(9, $lRows);
$mRows = $sheet->getHighestDataRow('M');
self::assertSame(9, $mRows);
$sheet->getCell('A1')
->setValue(
"=FILTER(Products!M1:M$mRows,"
. "(Products!K1:K$kRows=D5)"
. "*(Products!L1:L$lRows=E5))"
);
$result = $sheet->getCell('A1')->getCalculatedValue();
self::assertSame([['Apples'], ['Cherries']], $result);
$spreadsheet->disconnectWorksheets();
}
public static function testExtendSecondColumn(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setTitle('Products');
$calculationEngine = Calculation::getInstance($spreadsheet);
$calculationEngine->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_ARRAY
);
$sheet->getCell('D5')->setValue(5);
$sheet->getCell('E5')->setValue(20);
$sheet->fromArray(
[
[5, 20, 'Apples'],
[10, 20, 'Bananas'],
[5, 20, 'Cherries'],
[5, 40, 'Grapes'],
[25, 50, 'Peaches'],
[30, 60, 'Pears'],
[35, 70, 'Papayas'],
[40, 80, 'Mangos'],
[null, 20, 'Unknown'],
],
null,
'K1',
true
);
$kRows = $sheet->getHighestDataRow('K');
self::assertSame(8, $kRows);
//$lRows = $sheet->getHighestDataRow('L');
//self::assertSame(9, $lRows);
$lRows = 2;
$mRows = $sheet->getHighestDataRow('M');
self::assertSame(9, $mRows);
$sheet->getCell('A1')
->setValue(
"=FILTER(Products!M1:M$mRows,"
. "(Products!K1:K$kRows=D5)"
. "*(Products!L1:L$lRows=E5))"
);
$result = $sheet->getCell('A1')->getCalculatedValue();
self::assertSame([['Apples']], $result);
$spreadsheet->disconnectWorksheets();
}
}