From a91acec5d9ba08322fb4109693d665b03252fe01 Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Sun, 14 Jul 2019 16:22:31 +0200 Subject: [PATCH] Countif strict comparison (#1078) * Stricter-typed comparison testing in COUNTIF() and COUNTIFS() evaluation [Issue #1046](https://github.com/PHPOffice/PhpSpreadsheet/issues/1046) * Codestyle * Codestyle * Codestyle in tests --- CHANGELOG.md | 1 + src/PhpSpreadsheet/Calculation/Functions.php | 4 ++- .../Calculation/Statistical.php | 16 +++++++++ .../Calculation/StatisticalTest.php | 16 +++++++++ .../data/Calculation/Statistical/COUNTIF.php | 34 +++++++++++++++++++ .../data/Calculation/Statistical/COUNTIFS.php | 32 +++++++---------- 6 files changed, 82 insertions(+), 21 deletions(-) create mode 100644 tests/data/Calculation/Statistical/COUNTIF.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f1cff4e9..b626d2c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Stricter-typed comparison testing in COUNTIF() and COUNTIFS() evaluation [Issue #1046](https://github.com/PHPOffice/PhpSpreadsheet/issues/1046) - COUPNUM should not return zero when settlement is in the last period - [Issue #1020](https://github.com/PHPOffice/PhpSpreadsheet/issues/1020) and [PR #1021](https://github.com/PHPOffice/PhpSpreadsheet/pull/1021) ## [1.8.2] - 2019-07-08 diff --git a/src/PhpSpreadsheet/Calculation/Functions.php b/src/PhpSpreadsheet/Calculation/Functions.php index 0a607c70..3e18eb8e 100644 --- a/src/PhpSpreadsheet/Calculation/Functions.php +++ b/src/PhpSpreadsheet/Calculation/Functions.php @@ -280,7 +280,9 @@ class Functions preg_match('/(=|<[>=]?|>=?)(.*)/', $condition, $matches); list(, $operator, $operand) = $matches; - if (!is_numeric($operand)) { + if (is_numeric(trim($operand, '"'))) { + $operand = trim($operand, '"'); + } elseif (!is_numeric($operand)) { $operand = str_replace('"', '""', $operand); $operand = Calculation::wrapResult(strtoupper($operand)); } diff --git a/src/PhpSpreadsheet/Calculation/Statistical.php b/src/PhpSpreadsheet/Calculation/Statistical.php index a7f7bb63..c50d8b69 100644 --- a/src/PhpSpreadsheet/Calculation/Statistical.php +++ b/src/PhpSpreadsheet/Calculation/Statistical.php @@ -1119,10 +1119,16 @@ class Statistical $aArgs = Functions::flattenArray($aArgs); $condition = Functions::ifCondition($condition); + $conditionIsNumeric = strpos($condition, '"') === false; // Loop through arguments foreach ($aArgs as $arg) { if (!is_numeric($arg)) { + if ($conditionIsNumeric) { + continue; + } $arg = Calculation::wrapResult(strtoupper($arg)); + } elseif (!$conditionIsNumeric) { + continue; } $testCondition = '=' . $arg . $condition; if (Calculation::getInstance()->_calculateFormulaValue($testCondition)) { @@ -1172,11 +1178,21 @@ class Statistical $valid = true; foreach ($conditions as $cidx => $condition) { + $conditionIsNumeric = strpos($condition, '"') === false; $arg = $aArgsArray[$cidx][$index]; // Loop through arguments if (!is_numeric($arg)) { + if ($conditionIsNumeric) { + $valid = false; + + break; // if false found, don't need to check other conditions + } $arg = Calculation::wrapResult(strtoupper($arg)); + } elseif (!$conditionIsNumeric) { + $valid = false; + + break; // if false found, don't need to check other conditions } $testCondition = '=' . $arg . $condition; if (!Calculation::getInstance()->_calculateFormulaValue($testCondition)) { diff --git a/tests/PhpSpreadsheetTests/Calculation/StatisticalTest.php b/tests/PhpSpreadsheetTests/Calculation/StatisticalTest.php index ea25b3f5..78a7a2ac 100644 --- a/tests/PhpSpreadsheetTests/Calculation/StatisticalTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/StatisticalTest.php @@ -13,6 +13,22 @@ class StatisticalTest extends TestCase Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); } + /** + * @dataProvider providerCOUNTIF + * + * @param mixed $expectedResult + */ + public function testCOUNTIF($expectedResult, ...$args) + { + $result = Statistical::COUNTIF(...$args); + self::assertEquals($expectedResult, $result, '', 1E-12); + } + + public function providerCOUNTIF() + { + return require 'data/Calculation/Statistical/COUNTIF.php'; + } + /** * @dataProvider providerCOUNTIFS * diff --git a/tests/data/Calculation/Statistical/COUNTIF.php b/tests/data/Calculation/Statistical/COUNTIF.php new file mode 100644 index 00000000..c0e69c67 --- /dev/null +++ b/tests/data/Calculation/Statistical/COUNTIF.php @@ -0,0 +1,34 @@ +55', + ], + [ + 3, + [32, 54, 75, 86], + '<=75', + ], + [ + 2, + [6, 3, 4, 'X', ''], + '<=4', + ], + [ + 2, + [6, 3, 4, 'X', ''], + '<="4"', + ], +]; diff --git a/tests/data/Calculation/Statistical/COUNTIFS.php b/tests/data/Calculation/Statistical/COUNTIFS.php index 3f949b5a..32f64d71 100644 --- a/tests/data/Calculation/Statistical/COUNTIFS.php +++ b/tests/data/Calculation/Statistical/COUNTIFS.php @@ -3,30 +3,22 @@ return [ [ 2, - [ - ['Y'], - ['Y'], - ['N'], - ], + ['Y', 'Y', 'N'], '=Y', ], [ 3, - [ - ['A'], - ['B'], - ['C'], - ['B'], - ['B'], - ], - '=B', - [ - ['C'], - ['B'], - ['A'], - ['B'], - ['B'], - ], + ['A', 'B', 'C', 'B', 'B'], '=B', ], + [ + 3, + ['C', 'B', 'A', 'B', 'B'], + '=B', + ], + [ + 2, + [1, 2, 3, 'B', '', false], + '<=2', + ], ];