From 990e3ec3a3f676f1bae8b2506efc9f5687c6eeb5 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 24 Feb 2025 21:40:12 -0800 Subject: [PATCH] Handle #REF! As Argument to COUNTIF, AVERAGEIF, SUMIF Fix #4381. The report refers to COUNTIF, but AVERAGEIF and SUMIF, which are implemented in the same module, exhibit the same behavior. (There may be others, but, for now, I will just fix those 3.) Most methods which implement Excel functions should accept mixed arguments, so that they won't throw exceptions when calculated. Of course, MS often doesn't give much guidance as to how unexpected arguments should be handled. It at least seems clear that MS will often substitute #REF! for some arguments, and will return #REF! as the result in such cases. My test indicates that a formula using, say, #DIV/0! in lieu of #REF! will cause Excel to deem the spreadsheet corrupt. So, I think I am just going to deal with #REF! and let other unexpected values continue to throw exceptions. --- .../Calculation/Statistical/Conditional.php | 35 ++++++++++++++++--- .../Functions/MathTrig/SumIfTest.php | 17 +++++++++ .../Functions/Statistical/AverageIfTest.php | 3 ++ .../Functions/Statistical/CountIfTest.php | 21 ++++++++++- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/Statistical/Conditional.php b/src/PhpSpreadsheet/Calculation/Statistical/Conditional.php index ae98c60802..b82cb04ded 100644 --- a/src/PhpSpreadsheet/Calculation/Statistical/Conditional.php +++ b/src/PhpSpreadsheet/Calculation/Statistical/Conditional.php @@ -9,6 +9,7 @@ use PhpOffice\PhpSpreadsheet\Calculation\Database\DSum; use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException; use PhpOffice\PhpSpreadsheet\Calculation\Functions; +use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; class Conditional { @@ -24,13 +25,17 @@ class Conditional * Excel Function: * AVERAGEIF(range,condition[, average_range]) * - * @param mixed $range Data values + * @param mixed $range Data values, expect array * @param null|array|string $condition the criteria that defines which cells will be checked * @param mixed $averageRange Data values */ public static function AVERAGEIF(mixed $range, null|array|string $condition, mixed $averageRange = []): null|int|float|string { if (!is_array($range) || !is_array($averageRange) || array_key_exists(0, $range) || array_key_exists(0, $averageRange)) { + if ($range === ExcelError::REF()) { + return $range; + } + throw new CalcException('Must specify range of cells, not any kind of literal'); } $database = self::databaseFromRangeAndValue($range, $averageRange); @@ -76,11 +81,21 @@ public static function AVERAGEIFS(mixed ...$args): null|int|float|string * Excel Function: * COUNTIF(range,condition) * - * @param mixed[] $range Data values + * @param mixed $range Data values, expect array * @param null|array|string $condition the criteria that defines which cells will be counted */ - public static function COUNTIF(array $range, null|array|string $condition): string|int + public static function COUNTIF(mixed $range, null|array|string $condition): string|int { + if ( + !is_array($range) + || array_key_exists(0, $range) + ) { + if ($range === ExcelError::REF()) { + return $range; + } + + throw new CalcException('Must specify range of cells, not any kind of literal'); + } // Filter out any empty values that shouldn't be included in a COUNT $range = array_filter( Functions::flattenArray($range), @@ -169,10 +184,20 @@ public static function MINIFS(mixed ...$args): null|float|string * Excel Function: * SUMIF(range, criteria, [sum_range]) * - * @param array $range Data values + * @param mixed $range Data values, expecting array */ - public static function SUMIF(array $range, mixed $condition, array $sumRange = []): null|float|string + public static function SUMIF(mixed $range, mixed $condition, array $sumRange = []): null|float|string { + if ( + !is_array($range) + || array_key_exists(0, $range) + ) { + if ($range === ExcelError::REF()) { + return $range; + } + + throw new CalcException('Must specify range of cells, not any kind of literal'); + } $database = self::databaseFromRangeAndValue($range, $sumRange); $condition = [[self::CONDITION_COLUMN_NAME, self::VALUE_COLUMN_NAME], [$condition, null]]; diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumIfTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumIfTest.php index 3ee886f708..43b83051ff 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumIfTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/SumIfTest.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\MathTrig; +use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException; use PHPUnit\Framework\Attributes\DataProvider; class SumIfTest extends AllSetupTeardown @@ -38,4 +39,20 @@ public static function providerSUMIF(): array { return require 'tests/data/Calculation/MathTrig/SUMIF.php'; } + + public function testOutliers(): void + { + $sheet = $this->getSheet(); + $sheet->getCell('A1')->setValue('=SUMIF(5,"<32")'); + + try { + $sheet->getCell('A1')->getCalculatedValue(); + self::fail('Should receive exception for non-array arg'); + } catch (CalcException $e) { + self::assertStringContainsString('Must specify range of cells', $e->getMessage()); + } + + $sheet->getCell('A4')->setValue('=SUMIF(#REF!,"<32")'); + self::assertSame('#REF!', $sheet->getCell('A4')->getCalculatedValue()); + } } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/AverageIfTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/AverageIfTest.php index c25b636b81..5616bd5311 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/AverageIfTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/AverageIfTest.php @@ -41,5 +41,8 @@ public function testOutliers(): void } $sheet->getCell('A3')->setValue('=AVERAGEIF(C1:C1,"<32")'); self::assertSame(5, $sheet->getCell('A3')->getCalculatedValue(), 'first arg is single cell'); + + $sheet->getCell('A4')->setValue('=AVERAGEIF(#REF!,1)'); + self::assertSame('#REF!', $sheet->getCell('A4')->getCalculatedValue()); } } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/CountIfTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/CountIfTest.php index a4e803138b..7193b8ae35 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/CountIfTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/Statistical/CountIfTest.php @@ -4,9 +4,12 @@ namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Statistical; +use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException; +use PHPUnit\Framework\Attributes\DataProvider; + class CountIfTest extends AllSetupTeardown { - #[\PHPUnit\Framework\Attributes\DataProvider('providerCOUNTIF')] + #[DataProvider('providerCOUNTIF')] public function testCOUNTIF(mixed $expectedResult, mixed ...$args): void { $this->runTestCaseNoBracket('COUNTIF', $expectedResult, ...$args); @@ -27,4 +30,20 @@ public static function providerCOUNTIF(): array { return require 'tests/data/Calculation/Statistical/COUNTIF.php'; } + + public function testOutliers(): void + { + $sheet = $this->getSheet(); + $sheet->getCell('A1')->setValue('=COUNTIF(5,"<32")'); + + try { + $sheet->getCell('A1')->getCalculatedValue(); + self::fail('Should receive exception for non-array arg'); + } catch (CalcException $e) { + self::assertStringContainsString('Must specify range of cells', $e->getMessage()); + } + + $sheet->getCell('A4')->setValue('=COUNTIF(#REF!,1)'); + self::assertSame('#REF!', $sheet->getCell('A4')->getCalculatedValue()); + } }