Skip to content

Commit

Permalink
Coerce Bool to Int for Mathematical Operators on Arrays (#3392)
Browse files Browse the repository at this point in the history 
* Coerce Bool to Int for Unary Operation on Arrays

Fix #3389. It seems some functionality was left behind when JAMA was eliminated (PR #3260). In particular, it is apparently a known trick to use double negation on boolean values as arguments to functions like SUMPRODUCT.

* Fix 3396

Treat booleans in arrays as int for mathematical operators as well.

* Edge Case

When array operand was neither numeric nor boolean, PhpSpreadsheet had always been evaluating the operand as #NUM!. It will now propagate an error string like #DIV/0!, and treat non-error strings as #VALUE!, consistent with Excel.
  • Loading branch information
@oleibman
oleibman authored Feb 24, 2023
1 parent 6925b7f commit ab420f4
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 6 deletions.
24 changes: 18 additions & 6 deletions src/PhpSpreadsheet/Calculation/Calculation.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4944,10 +4944,10 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)
[$rows, $columns] = self::checkMatrixOperands($result, $operand2, 0);
for ($row = 0; $row < $rows; ++$row) {
for ($column = 0; $column < $columns; ++$column) {
if (is_numeric($result[$row][$column])) {
if (self::isNumericOrBool($result[$row][$column])) {
$result[$row][$column] *= $multiplier;
} else {
$result[$row][$column] = Information\ExcelError::VALUE();
$result[$row][$column] = self::makeError($result[$row][$column]);
}
}
}
Expand Down Expand Up @@ -5336,15 +5336,15 @@ private function executeNumericBinaryOperation($operand1, $operand2, $operation,
for ($column = 0; $column < $columns; ++$column) {
if ($operand1[$row][$column] === null) {
$operand1[$row][$column] = 0;
} elseif (!is_numeric($operand1[$row][$column])) {
$operand1[$row][$column] = Information\ExcelError::VALUE();
} elseif (!self::isNumericOrBool($operand1[$row][$column])) {
$operand1[$row][$column] = self::makeError($operand1[$row][$column]);

continue;
}
if ($operand2[$row][$column] === null) {
$operand2[$row][$column] = 0;
} elseif (!is_numeric($operand2[$row][$column])) {
$operand1[$row][$column] = Information\ExcelError::VALUE();
} elseif (!self::isNumericOrBool($operand2[$row][$column])) {
$operand1[$row][$column] = self::makeError($operand2[$row][$column]);

continue;
}
Expand Down Expand Up @@ -5764,4 +5764,16 @@ private static function boolToString($operand1)

return $operand1;
}

/** @param mixed $operand */
private static function isNumericOrBool($operand): bool
{
return is_numeric($operand) || is_bool($operand);
}

/** @param mixed $operand */
private static function makeError($operand = ''): string
{
return Information\ErrorValue::isError($operand) ? $operand : Information\ExcelError::VALUE();
}
}
32 changes: 32 additions & 0 deletions tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndexTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\Matrix;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class IndexTest extends TestCase
Expand Down Expand Up @@ -68,4 +69,35 @@ public function providerIndexArray(): array
],
];
}

public function testPropagateDiv0(): void
{
// issue 3396 error was always being treated as #VALUE!
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->getCell('A1')->setValue(0);
$sheet->getCell('A3')->setValue(1);
$sheet->getCell('B3')->setValue(1);
$sheet->getCell('C3')->setValue('=1/A1');
$sheet->getCell('D3')->setValue('=1/A1');
$sheet->getCell('E3')->setValue('xyz');
$sheet->getCell('A4')->setValue(false);
$sheet->getCell('B4')->setValue(true);
$sheet->getCell('C4')->setValue(true);
$sheet->getCell('D4')->setValue(false);
$sheet->getCell('E4')->setValue(false);
$sheet->getCell('A6')->setValue('=INDEX(A3:E3/A4:E4,1,1)');
$sheet->getCell('B6')->setValue('=INDEX(A3:E3/A4:E4,1,2)');
$sheet->getCell('C6')->setValue('=INDEX(A3:E3/A4:E4,1,3)');
$sheet->getCell('D6')->setValue('=INDEX(A3:E3/A4:E4,1,4)');
$sheet->getCell('E6')->setValue('=INDEX(A3:E3/A4:E4,1,5)');

self::assertSame('#DIV/0!', $sheet->getCell('A6')->getCalculatedValue());
self::assertSame(1, $sheet->getCell('B6')->getCalculatedValue());
self::assertSame('#DIV/0!', $sheet->getCell('C6')->getCalculatedValue());
self::assertSame('#DIV/0!', $sheet->getCell('D6')->getCalculatedValue());
self::assertSame('#VALUE!', $sheet->getCell('E6')->getCalculatedValue());

$spreadsheet->disconnectWorksheets();
}
}
30 changes: 30 additions & 0 deletions tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/LookupTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

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

class LookupTest extends TestCase
Expand Down Expand Up @@ -47,4 +48,33 @@ public function providerLookupArray(): array
],
];
}

public function testBoolsAsInt(): void
{
// issue 3396 not handling math operation for bool in array
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->getCell('A59')->setValue('start');
$sheet->getCell('B59')->setValue('end');
$sheet->getCell('C59')->setValue('percent');
$sheet->getCell('A60')->setValue('=DATEVALUE("1950-01-01")');
$sheet->getCell('B60')->setValue('=DATEVALUE("2016-06-03")');
$sheet->getCell('C60')->setValue(0.05);
$sheet->getCell('A61')->setValue('=DATEVALUE("2016-06-04")');
$sheet->getCell('B61')->setValue('=DATEVALUE("2021-01-05")');
$sheet->getCell('C61')->setValue(0.08);
$sheet->getCell('A62')->setValue('=DATEVALUE("2021-01-16")');
$sheet->getCell('B62')->setValue('=DATEVALUE("2022-04-08")');
$sheet->getCell('C62')->setValue(0.03);
$sheet->getCell('A63')->setValue('=DATEVALUE("2022-04-09")');
$sheet->getCell('B63')->setValue('=DATEVALUE("2500-12-31")');
$sheet->getCell('C63')->setValue(0.04);

$sheet->getCell('D5')->setValue(5);
$sheet->getCell('E5')->setValue('=DATEVALUE("2023-01-01")');
$sheet->getCell('D7')->setValue('=IF(E5<>"",LOOKUP(2,1/($A$60:$A$63<=E5)/($B$60:$B$63>=E5),$C$60:$C$63)*D5,"")');

self::assertSame(0.2, $sheet->getCell('D7')->getCalculatedValue());
$spreadsheet->disconnectWorksheets();
}
}
24 changes: 24 additions & 0 deletions tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumProductTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,28 @@ public function providerSUMPRODUCT(): array
{
return require 'tests/data/Calculation/MathTrig/SUMPRODUCT.php';
}

public function testBoolsAsInt(): void
{
// issue 3389 not handling unary minus with boolean value
$sheet = $this->getSheet();
$sheet->fromArray(
[
['Range 1', 'Range 2', null, 'Blue matches', 'Red matches'],
[0, 'Red', null, '=SUMPRODUCT(--(B3:B10=1), --(C3:C10="BLUE"))', '=SUMPRODUCT(--(B3:B10=1), --(C3:C10="RED"))'],
[1, 'Blue'],
[0, 'Blue'],
[1, 'Red'],
[1, 'Blue'],
[0, 'Blue'],
[1, 'Red'],
[1, 'Blue'],
],
null, // null value
'B2', // start cell
true // strict null comparison
);
self::assertSame(3, $sheet->getCell('E3')->getCalculatedValue());
self::assertSame(2, $sheet->getCell('F3')->getCalculatedValue());
}
}

0 comments on commit ab420f4

Please sign in to comment.