diff --git a/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php b/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php index dda2c705..4b9f818f 100644 --- a/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php +++ b/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php @@ -47,7 +47,7 @@ class ErrorValue return false; } - return in_array($value, ExcelError::$errorCodes, true); + return in_array($value, ExcelError::ERROR_CODES, true); } /** diff --git a/src/PhpSpreadsheet/Calculation/Information/ExcelError.php b/src/PhpSpreadsheet/Calculation/Information/ExcelError.php index 5ca74a3e..06f38663 100644 --- a/src/PhpSpreadsheet/Calculation/Information/ExcelError.php +++ b/src/PhpSpreadsheet/Calculation/Information/ExcelError.php @@ -13,7 +13,7 @@ class ExcelError * * @var array */ - public static $errorCodes = [ + public const ERROR_CODES = [ 'null' => '#NULL!', // 1 'divisionbyzero' => '#DIV/0!', // 2 'value' => '#VALUE!', // 3 @@ -30,12 +30,23 @@ class ExcelError 'calculation' => '#CALC!', //14 ]; + /** + * List of error codes. Replaced by constant; + * previously it was public and updateable, allowing + * user to make inappropriate alterations. + * + * @deprecated 1.25.0 Use ERROR_CODES constant instead. + * + * @var array + */ + public static $errorCodes = self::ERROR_CODES; + /** * @param mixed $value */ public static function throwError($value): string { - return in_array($value, self::$errorCodes, true) ? $value : self::$errorCodes['value']; + return in_array($value, self::ERROR_CODES, true) ? $value : self::ERROR_CODES['value']; } /** @@ -52,7 +63,7 @@ class ExcelError } $i = 1; - foreach (self::$errorCodes as $errorCode) { + foreach (self::ERROR_CODES as $errorCode) { if ($value === $errorCode) { return $i; } @@ -71,7 +82,7 @@ class ExcelError */ public static function null(): string { - return self::$errorCodes['null']; + return self::ERROR_CODES['null']; } /** @@ -83,7 +94,7 @@ class ExcelError */ public static function NAN(): string { - return self::$errorCodes['num']; + return self::ERROR_CODES['num']; } /** @@ -95,7 +106,7 @@ class ExcelError */ public static function REF(): string { - return self::$errorCodes['reference']; + return self::ERROR_CODES['reference']; } /** @@ -111,7 +122,7 @@ class ExcelError */ public static function NA(): string { - return self::$errorCodes['na']; + return self::ERROR_CODES['na']; } /** @@ -123,7 +134,7 @@ class ExcelError */ public static function VALUE(): string { - return self::$errorCodes['value']; + return self::ERROR_CODES['value']; } /** @@ -135,7 +146,7 @@ class ExcelError */ public static function NAME(): string { - return self::$errorCodes['name']; + return self::ERROR_CODES['name']; } /** @@ -145,7 +156,7 @@ class ExcelError */ public static function DIV0(): string { - return self::$errorCodes['divisionbyzero']; + return self::ERROR_CODES['divisionbyzero']; } /** @@ -155,6 +166,6 @@ class ExcelError */ public static function CALC(): string { - return self::$errorCodes['calculation']; + return self::ERROR_CODES['calculation']; } } diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/HLookup.php b/src/PhpSpreadsheet/Calculation/LookupRef/HLookup.php index d67718ce..e2d27bde 100644 --- a/src/PhpSpreadsheet/Calculation/LookupRef/HLookup.php +++ b/src/PhpSpreadsheet/Calculation/LookupRef/HLookup.php @@ -66,7 +66,7 @@ class HLookup extends LookupBase */ private static function hLookupSearch($lookupValue, array $lookupArray, $column, bool $notExactMatch): ?int { - $lookupLower = StringHelper::strToLower($lookupValue); + $lookupLower = StringHelper::strToLower((string) $lookupValue); $rowNumber = null; foreach ($lookupArray[$column] as $rowKey => $rowData) { diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/LookupBase.php b/src/PhpSpreadsheet/Calculation/LookupRef/LookupBase.php index 8e451fe4..a001540c 100644 --- a/src/PhpSpreadsheet/Calculation/LookupRef/LookupBase.php +++ b/src/PhpSpreadsheet/Calculation/LookupRef/LookupBase.php @@ -19,8 +19,16 @@ abstract class LookupBase protected static function validateIndexLookup(array $lookup_array, $index_number): int { - // index_number must be a number greater than or equal to 1 - if (!is_numeric($index_number) || $index_number < 1) { + // index_number must be a number greater than or equal to 1. + // Excel results are inconsistent when index is non-numeric. + // VLOOKUP(whatever, whatever, SQRT(-1)) yields NUM error, but + // VLOOKUP(whatever, whatever, cellref) yields REF error + // when cellref is '=SQRT(-1)'. So just try our best here. + // Similar results if string (literal yields VALUE, cellRef REF). + if (!is_numeric($index_number)) { + throw new Exception(ExcelError::throwError($index_number)); + } + if ($index_number < 1) { throw new Exception(ExcelError::VALUE()); } diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php b/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php index 53a7badc..edeb1aa8 100644 --- a/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php +++ b/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php @@ -68,8 +68,8 @@ class VLookup extends LookupBase { reset($a); $firstColumn = key($a); - $aLower = StringHelper::strToLower($a[$firstColumn]); - $bLower = StringHelper::strToLower($b[$firstColumn]); + $aLower = StringHelper::strToLower((string) $a[$firstColumn]); + $bLower = StringHelper::strToLower((string) $b[$firstColumn]); if ($aLower == $bLower) { return 0; @@ -84,7 +84,7 @@ class VLookup extends LookupBase */ private static function vLookupSearch($lookupValue, array $lookupArray, $column, bool $notExactMatch): ?int { - $lookupLower = StringHelper::strToLower($lookupValue); + $lookupLower = StringHelper::strToLower((string) $lookupValue); $rowNumber = null; foreach ($lookupArray as $rowKey => $rowData) { diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php index 4e05ea8a..5b7647be 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php @@ -3,26 +3,42 @@ namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\LookupRef; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; -use PhpOffice\PhpSpreadsheet\Calculation\Functions; -use PhpOffice\PhpSpreadsheet\Calculation\LookupRef; +use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; class VLookupTest extends TestCase { - protected function setUp(): void - { - Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); - } - /** * @dataProvider providerVLOOKUP * * @param mixed $expectedResult + * @param mixed $value + * @param mixed $table + * @param mixed $index */ - public function testVLOOKUP($expectedResult, ...$args): void + public function testVLOOKUP($expectedResult, $value, $table, $index, ?bool $lookup = null): void { - $result = LookupRef::VLOOKUP(...$args); + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + if (is_array($table)) { + $sheet->fromArray($table); + $dimension = $sheet->calculateWorksheetDimension(); + } else { + $sheet->getCell('A1')->setValue($table); + $dimension = 'A1'; + } + if ($lookup === null) { + $lastarg = ''; + } else { + $lastarg = $lookup ? ',TRUE' : ',FALSE'; + } + $sheet->getCell('Z98')->setValue($value); + $sheet->getCell('Z97')->setValue($index); + + $sheet->getCell('Z99')->setValue("=VLOOKUP(Z98,$dimension,Z97$lastarg)"); + $result = $sheet->getCell('Z99')->getCalculatedValue(); self::assertEquals($expectedResult, $result); + $spreadsheet->disconnectWorksheets(); } public function providerVLOOKUP(): array diff --git a/tests/data/Calculation/LookupRef/HLOOKUP.php b/tests/data/Calculation/LookupRef/HLOOKUP.php index 078ed007..ee1b919e 100644 --- a/tests/data/Calculation/LookupRef/HLOOKUP.php +++ b/tests/data/Calculation/LookupRef/HLOOKUP.php @@ -186,4 +186,14 @@ return [ 3, true, ], + 'issue2934' => [ + 'Red', + 102, + [ + [null, 102], + [null, 'Red'], + ], + 2, + false, + ], ]; diff --git a/tests/data/Calculation/LookupRef/VLOOKUP.php b/tests/data/Calculation/LookupRef/VLOOKUP.php index 2162d49a..21146638 100644 --- a/tests/data/Calculation/LookupRef/VLOOKUP.php +++ b/tests/data/Calculation/LookupRef/VLOOKUP.php @@ -98,7 +98,7 @@ return [ ['10y1', 7.0], ['10y2', 10.0], ], - 'NaN', + -5, ], [ '#REF!', @@ -111,9 +111,9 @@ return [ '#REF!', '10y2', [ - 2.0, - 7.0, - 10.0, + [2.0], + [7.0], + [10.0], ], 2.0, ], @@ -163,4 +163,34 @@ return [ 3, null, ], + 'issue2934' => [ + 'Red', + 102, + [ + [null, null], + [102, 'Red'], + ], + 2, + false, + ], + 'string supplied as index' => [ + '#VALUE!', + 102, + [ + [null, null], + [102, 'Red'], + ], + 'xyz', + false, + ], + 'num error propagated' => [ + '#NUM!', + 102, + [ + [null, null], + [102, 'Red'], + ], + '=SQRT(-1)', + false, + ], ];