Issue 2066, highlighting more validation needed for LookupRef Functions (#2069)

* Issue 2066, highlighting more validation needed for LookupRef Functions
* Additional test cases
This commit is contained in:
Mark Baker 2021-05-07 11:20:38 +02:00 committed by GitHub
parent 5ee4fbf090
commit 115e39ae0c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 103 additions and 26 deletions

View File

@ -975,11 +975,6 @@ parameters:
count: 1 count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/LookupBase.php path: src/PhpSpreadsheet/Calculation/LookupRef/LookupBase.php
-
message: "#^Parameter \\#3 \\$rowNum of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Matrix\\:\\:extractRowValue\\(\\) expects int, float\\|int\\<0, max\\>\\|string given\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/Matrix.php
- -
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Matrix\\:\\:extractRowValue\\(\\) has no return typehint specified\\.$#" message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Matrix\\:\\:extractRowValue\\(\\) has no return typehint specified\\.$#"
count: 1 count: 1

View File

@ -0,0 +1,39 @@
<?php
namespace PhpOffice\PhpSpreadsheet\Calculation\LookupRef;
use PhpOffice\PhpSpreadsheet\Calculation\Exception;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
class LookupRefValidations
{
/**
* @param mixed $value
*/
public static function validateInt($value): int
{
if (!is_numeric($value)) {
if (Functions::isError($value)) {
throw new Exception($value);
}
throw new Exception(Functions::VALUE());
}
return (int) floor((float) $value);
}
/**
* @param mixed $value
*/
public static function validatePositiveInt($value, bool $allowZero = true): int
{
$value = self::validateInt($value);
if (($allowZero === false && $value <= 0) || $value < 0) {
throw new Exception(Functions::VALUE());
}
return $value;
}
}

View File

@ -2,6 +2,7 @@
namespace PhpOffice\PhpSpreadsheet\Calculation\LookupRef; namespace PhpOffice\PhpSpreadsheet\Calculation\LookupRef;
use PhpOffice\PhpSpreadsheet\Calculation\Exception;
use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Functions;
class Matrix class Matrix
@ -39,7 +40,7 @@ class Matrix
* Uses an index to choose a value from a reference or array * Uses an index to choose a value from a reference or array
* *
* Excel Function: * Excel Function:
* =INDEX(range_array, row_num, [column_num]) * =INDEX(range_array, row_num, [column_num], [area_num])
* *
* @param mixed $matrix A range of cells or an array constant * @param mixed $matrix A range of cells or an array constant
* @param mixed $rowNum The row in the array or range from which to return a value. * @param mixed $rowNum The row in the array or range from which to return a value.
@ -47,15 +48,20 @@ class Matrix
* @param mixed $columnNum The column in the array or range from which to return a value. * @param mixed $columnNum The column in the array or range from which to return a value.
* If column_num is omitted, row_num is required. * If column_num is omitted, row_num is required.
* *
* TODO Provide support for area_num, currently not supported
*
* @return mixed the value of a specified cell or array of cells * @return mixed the value of a specified cell or array of cells
*/ */
public static function index($matrix, $rowNum = 0, $columnNum = 0) public static function index($matrix, $rowNum = 0, $columnNum = 0)
{ {
$rowNum = Functions::flattenSingleValue($rowNum); $rowNum = ($rowNum === null) ? 0 : Functions::flattenSingleValue($rowNum);
$columnNum = Functions::flattenSingleValue($columnNum); $columnNum = ($columnNum === null) ? 0 : Functions::flattenSingleValue($columnNum);
if (!is_numeric($rowNum) || !is_numeric($columnNum) || ($rowNum < 0) || ($columnNum < 0)) { try {
return Functions::VALUE(); $rowNum = LookupRefValidations::validatePositiveInt($rowNum);
$columnNum = LookupRefValidations::validatePositiveInt($columnNum);
} catch (Exception $e) {
return $e->getMessage();
} }
if (!is_array($matrix) || ($rowNum > count($matrix))) { if (!is_array($matrix) || ($rowNum > count($matrix))) {
@ -69,12 +75,12 @@ class Matrix
return Functions::REF(); return Functions::REF();
} }
if ($columnNum == 0) { if ($columnNum === 0) {
return self::extractRowValue($matrix, $rowKeys, $rowNum); return self::extractRowValue($matrix, $rowKeys, $rowNum);
} }
$columnNum = $columnKeys[--$columnNum]; $columnNum = $columnKeys[--$columnNum];
if ($rowNum == 0) { if ($rowNum === 0) {
return array_map( return array_map(
function ($value) { function ($value) {
return [$value]; return [$value];
@ -89,7 +95,7 @@ class Matrix
private static function extractRowValue(array $matrix, array $rowKeys, int $rowNum) private static function extractRowValue(array $matrix, array $rowKeys, int $rowNum)
{ {
if ($rowNum == 0) { if ($rowNum === 0) {
return $matrix; return $matrix;
} }

View File

@ -6,7 +6,7 @@ return [
// Input // Input
[20 => ['R' => 1]], [20 => ['R' => 1]],
], ],
[ 'Negative Row' => [
'#VALUE!', // Expected '#VALUE!', // Expected
// Input // Input
[ [
@ -15,7 +15,7 @@ return [
], ],
-1, -1,
], ],
[ 'Row > matrix rows' => [
'#REF!', // Expected '#REF!', // Expected
// Input // Input
[ [
@ -24,7 +24,25 @@ return [
], ],
10, 10,
], ],
[ 'Row is not a number' => [
'#VALUE!', // Expected
// Input
[
20 => ['R' => 1],
21 => ['R' => 2],
],
'NaN',
],
'Row is Error' => [
'#N/A', // Expected
// Input
[
20 => ['R' => 1],
21 => ['R' => 2],
],
'#N/A',
],
'Return row 2' => [
[21 => ['R' => 2]], // Expected [21 => ['R' => 2]], // Expected
// Input // Input
[ [
@ -33,7 +51,7 @@ return [
], ],
2, 2,
], ],
[ 'Return row 2 from larger matrix' => [
[21 => ['R' => 2, 'S' => 4]], // Expected [21 => ['R' => 2, 'S' => 4]], // Expected
// Input // Input
[ [
@ -43,17 +61,17 @@ return [
2, 2,
0, 0,
], ],
[ 'Negative Column' => [
'#VALUE!', // Expected '#VALUE!', // Expected
// Input // Input
[ [
'20' => ['R' => 1, 'S' => 3], '20' => ['R' => 1, 'S' => 3],
'21' => ['R' => 2, 'S' => 4], '21' => ['R' => 2, 'S' => 4],
], ],
2, 0,
-1, -1,
], ],
[ 'Column > matrix columns' => [
'#REF!', // Expected '#REF!', // Expected
// Input // Input
[ [
@ -63,15 +81,25 @@ return [
2, 2,
10, 10,
], ],
[ 'Column is not a number' => [
'#REF!', // Expected '#VALUE!', // Expected
// Input // Input
[ [
'20' => ['R' => 1, 'S' => 3], 20 => ['R' => 1],
'21' => ['R' => 2, 'S' => 4], 21 => ['R' => 2],
], ],
10, 1,
2, 'NaN',
],
'Column is Error' => [
'#N/A', // Expected
// Input
[
20 => ['R' => 1],
21 => ['R' => 2],
],
1,
'#N/A',
], ],
[ [
4, // Expected 4, // Expected
@ -115,6 +143,15 @@ return [
2, 2,
1, 1,
], ],
[
[1 => ['Bananas', 'Pears']],
[
['Apples', 'Lemons'],
['Bananas', 'Pears'],
],
2,
0,
],
[ [
3, 3,
[ [