diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ddaee5a169..ee0135496e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,5 +1,25 @@ parameters: ignoreErrors: + - + message: "#^Parameter \\#1 \\$str1 of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\BinaryComparison\\:\\:strcmpAllowNull\\(\\) expects string\\|null, mixed given\\.$#" + count: 3 + path: src/PhpSpreadsheet/Calculation/BinaryComparison.php + + - + message: "#^Parameter \\#1 \\$str1 of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\BinaryComparison\\:\\:strcmpLowercaseFirst\\(\\) expects string\\|null, mixed given\\.$#" + count: 2 + path: src/PhpSpreadsheet/Calculation/BinaryComparison.php + + - + message: "#^Parameter \\#2 \\$str2 of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\BinaryComparison\\:\\:strcmpAllowNull\\(\\) expects string\\|null, mixed given\\.$#" + count: 3 + path: src/PhpSpreadsheet/Calculation/BinaryComparison.php + + - + message: "#^Parameter \\#2 \\$str2 of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\BinaryComparison\\:\\:strcmpLowercaseFirst\\(\\) expects string\\|null, mixed given\\.$#" + count: 2 + path: src/PhpSpreadsheet/Calculation/BinaryComparison.php + - message: "#^Argument of an invalid type array\\\\|false supplied for foreach, only iterables are supported\\.$#" count: 3 @@ -100,6 +120,11 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php + - + message: "#^Cannot cast mixed to string\\.$#" + count: 1 + path: src/PhpSpreadsheet/Calculation/Calculation.php + - message: "#^Cannot use array destructuring on mixed\\.$#" count: 6 @@ -235,36 +260,16 @@ parameters: count: 2 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Parameter \\#1 \\$str1 of method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:strcmpAllowNull\\(\\) expects string\\|null, mixed given\\.$#" - count: 4 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - - - message: "#^Parameter \\#1 \\$str1 of method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:strcmpLowercaseFirst\\(\\) expects string\\|null, mixed given\\.$#" - count: 2 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Parameter \\#1 \\$string of function strlen expects string, mixed given\\.$#" count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Parameter \\#1 \\$textValue of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\StringHelper\\:\\:strCaseReverse\\(\\) expects string, string\\|null given\\.$#" - count: 2 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Parameter \\#1 \\$worksheetName of method PhpOffice\\\\PhpSpreadsheet\\\\Spreadsheet\\:\\:getSheetByName\\(\\) expects string, mixed given\\.$#" count: 3 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Parameter \\#2 \\$str2 of method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:strcmpAllowNull\\(\\) expects string\\|null, mixed given\\.$#" - count: 4 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Parameter \\#2 \\$subject of function preg_match expects string, mixed given\\.$#" count: 4 @@ -285,11 +290,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Parameter \\#4 \\$operation of method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:executeBinaryComparisonOperation\\(\\) expects string, mixed given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Parameter \\#4 \\$storeKey of method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:push\\(\\) expects string\\|null, mixed given\\.$#" count: 2 @@ -405,16 +405,6 @@ parameters: count: 2 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Variable \\$inversedStr1 on left side of \\?\\? always exists and is not nullable\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - - - message: "#^Variable \\$inversedStr2 on left side of \\?\\? always exists and is not nullable\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Database\\:\\:DMAX\\(\\) should return float but returns float\\|string\\|null\\.$#" count: 1 diff --git a/src/PhpSpreadsheet/Calculation/BinaryComparison.php b/src/PhpSpreadsheet/Calculation/BinaryComparison.php new file mode 100644 index 0000000000..5d4f261fc1 --- /dev/null +++ b/src/PhpSpreadsheet/Calculation/BinaryComparison.php @@ -0,0 +1,181 @@ + '' && $operand1[0] == Calculation::FORMULA_STRING_QUOTE) { + $operand1 = Calculation::unwrapResult($operand1); + } + if (is_string($operand2) && $operand2 > '' && $operand2[0] == Calculation::FORMULA_STRING_QUOTE) { + $operand2 = Calculation::unwrapResult($operand2); + } + + // Use case insensitive comparaison if not OpenOffice mode + if (Functions::getCompatibilityMode() != Functions::COMPATIBILITY_OPENOFFICE) { + if (is_string($operand1)) { + $operand1 = StringHelper::strToUpper($operand1); + } + if (is_string($operand2)) { + $operand2 = StringHelper::strToUpper($operand2); + } + } + + $useLowercaseFirstComparison = is_string($operand1) && + is_string($operand2) && + Functions::getCompatibilityMode() === Functions::COMPATIBILITY_OPENOFFICE; + + return self::evaluateComparison($operand1, $operand2, $operator, $useLowercaseFirstComparison); + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function evaluateComparison($operand1, $operand2, string $operator, bool $useLowercaseFirstComparison): bool + { + switch ($operator) { + // Equality + case '=': + return self::equal($operand1, $operand2); + // Greater than + case '>': + return self::greaterThan($operand1, $operand2, $useLowercaseFirstComparison); + // Less than + case '<': + return self::lessThan($operand1, $operand2, $useLowercaseFirstComparison); + // Greater than or equal + case '>=': + return self::greaterThanOrEqual($operand1, $operand2, $useLowercaseFirstComparison); + // Less than or equal + case '<=': + return self::lessThanOrEqual($operand1, $operand2, $useLowercaseFirstComparison); + // Inequality + case '<>': + return self::notEqual($operand1, $operand2); + default: + throw new Exception('Unsupported binary comparison operator'); + } + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function equal($operand1, $operand2): bool + { + if (is_numeric($operand1) && is_numeric($operand2)) { + $result = (abs($operand1 - $operand2) < self::DELTA); + } elseif (($operand1 === null && is_numeric($operand2)) || ($operand2 === null && is_numeric($operand1))) { + $result = $operand1 == $operand2; + } else { + $result = self::strcmpAllowNull($operand1, $operand2) == 0; + } + + return $result; + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function greaterThanOrEqual($operand1, $operand2, bool $useLowercaseFirstComparison): bool + { + if (is_numeric($operand1) && is_numeric($operand2)) { + $result = ((abs($operand1 - $operand2) < self::DELTA) || ($operand1 > $operand2)); + } elseif (($operand1 === null && is_numeric($operand2)) || ($operand2 === null && is_numeric($operand1))) { + $result = $operand1 >= $operand2; + } elseif ($useLowercaseFirstComparison) { + $result = self::strcmpLowercaseFirst($operand1, $operand2) >= 0; + } else { + $result = self::strcmpAllowNull($operand1, $operand2) >= 0; + } + + return $result; + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function lessThanOrEqual($operand1, $operand2, bool $useLowercaseFirstComparison): bool + { + if (is_numeric($operand1) && is_numeric($operand2)) { + $result = ((abs($operand1 - $operand2) < self::DELTA) || ($operand1 < $operand2)); + } elseif (($operand1 === null && is_numeric($operand2)) || ($operand2 === null && is_numeric($operand1))) { + $result = $operand1 <= $operand2; + } elseif ($useLowercaseFirstComparison) { + $result = self::strcmpLowercaseFirst($operand1, $operand2) <= 0; + } else { + $result = self::strcmpAllowNull($operand1, $operand2) <= 0; + } + + return $result; + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function greaterThan($operand1, $operand2, bool $useLowercaseFirstComparison): bool + { + return self::lessThanOrEqual($operand1, $operand2, $useLowercaseFirstComparison) !== true; + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function lessThan($operand1, $operand2, bool $useLowercaseFirstComparison): bool + { + return self::greaterThanOrEqual($operand1, $operand2, $useLowercaseFirstComparison) !== true; + } + + /** + * @param mixed $operand1 + * @param mixed $operand2 + */ + private static function notEqual($operand1, $operand2): bool + { + return self::equal($operand1, $operand2) !== true; + } +} diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 9851c0f4ce..3efa6ad958 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -173,13 +173,6 @@ class Calculation */ public $cyclicFormulaCount = 1; - /** - * Epsilon Precision used for comparisons in calculations. - * - * @var float - */ - private $delta = 0.1e-12; - /** * The current locale setting. * @@ -2759,8 +2752,6 @@ class Calculation public function __construct(?Spreadsheet $spreadsheet = null) { - $this->delta = 1 * 10 ** (0 - ini_get('precision')); - $this->spreadsheet = $spreadsheet; $this->cyclicReferenceStack = new CyclicReferenceStack(); $this->debugLog = new Logger($this->cyclicReferenceStack); @@ -3742,6 +3733,8 @@ private function showValue($value) return self::FORMULA_STRING_QUOTE . $value . self::FORMULA_STRING_QUOTE; } elseif (is_bool($value)) { return ($value) ? self::$localeBoolean['TRUE'] : self::$localeBoolean['FALSE']; + } elseif ($value === null) { + return self::$localeBoolean['NULL']; } } @@ -4509,7 +4502,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null) case '<=': // Less than or Equal to case '=': // Equality case '<>': // Inequality - $result = $this->executeBinaryComparisonOperation($cellID, $operand1, $operand2, $token, $stack); + $result = $this->executeBinaryComparisonOperation($operand1, $operand2, (string) $token, $stack); if (isset($storeKey)) { $branchStore[$storeKey] = $result; } @@ -4952,21 +4945,20 @@ private function validateBinaryOperand(&$operand, &$stack) } /** - * @param null|string $cellID * @param mixed $operand1 * @param mixed $operand2 * @param string $operation * * @return array */ - private function executeArrayComparison($cellID, $operand1, $operand2, $operation, Stack &$stack, bool $recursingArrays) + private function executeArrayComparison($operand1, $operand2, $operation, Stack &$stack, bool $recursingArrays) { $result = []; if (!is_array($operand2)) { // Operand 1 is an array, Operand 2 is a scalar foreach ($operand1 as $x => $operandData) { $this->debugLog->writeDebugLog('Evaluating Comparison ', $this->showValue($operandData), ' ', $operation, ' ', $this->showValue($operand2)); - $this->executeBinaryComparisonOperation($cellID, $operandData, $operand2, $operation, $stack); + $this->executeBinaryComparisonOperation($operandData, $operand2, $operation, $stack); $r = $stack->pop(); $result[$x] = $r['value']; } @@ -4974,7 +4966,7 @@ private function executeArrayComparison($cellID, $operand1, $operand2, $operatio // Operand 1 is a scalar, Operand 2 is an array foreach ($operand2 as $x => $operandData) { $this->debugLog->writeDebugLog('Evaluating Comparison ', $this->showValue($operand1), ' ', $operation, ' ', $this->showValue($operandData)); - $this->executeBinaryComparisonOperation($cellID, $operand1, $operandData, $operation, $stack); + $this->executeBinaryComparisonOperation($operand1, $operandData, $operation, $stack); $r = $stack->pop(); $result[$x] = $r['value']; } @@ -4985,7 +4977,7 @@ private function executeArrayComparison($cellID, $operand1, $operand2, $operatio } foreach ($operand1 as $x => $operandData) { $this->debugLog->writeDebugLog('Evaluating Comparison ', $this->showValue($operandData), ' ', $operation, ' ', $this->showValue($operand2[$x])); - $this->executeBinaryComparisonOperation($cellID, $operandData, $operand2[$x], $operation, $stack, true); + $this->executeBinaryComparisonOperation($operandData, $operand2[$x], $operation, $stack, true); $r = $stack->pop(); $result[$x] = $r['value']; } @@ -4999,7 +4991,6 @@ private function executeArrayComparison($cellID, $operand1, $operand2, $operatio } /** - * @param null|string $cellID * @param mixed $operand1 * @param mixed $operand2 * @param string $operation @@ -5007,97 +4998,14 @@ private function executeArrayComparison($cellID, $operand1, $operand2, $operatio * * @return mixed */ - private function executeBinaryComparisonOperation($cellID, $operand1, $operand2, $operation, Stack &$stack, $recursingArrays = false) + private function executeBinaryComparisonOperation($operand1, $operand2, $operation, Stack &$stack, $recursingArrays = false) { // If we're dealing with matrix operations, we want a matrix result if ((is_array($operand1)) || (is_array($operand2))) { - return $this->executeArrayComparison($cellID, $operand1, $operand2, $operation, $stack, $recursingArrays); - } - - // Simple validate the two operands if they are string values - if (is_string($operand1) && $operand1 > '' && $operand1[0] == self::FORMULA_STRING_QUOTE) { - $operand1 = self::unwrapResult($operand1); - } - if (is_string($operand2) && $operand2 > '' && $operand2[0] == self::FORMULA_STRING_QUOTE) { - $operand2 = self::unwrapResult($operand2); - } - - // Use case insensitive comparaison if not OpenOffice mode - if (Functions::getCompatibilityMode() != Functions::COMPATIBILITY_OPENOFFICE) { - if (is_string($operand1)) { - $operand1 = Shared\StringHelper::strToUpper($operand1); - } - if (is_string($operand2)) { - $operand2 = Shared\StringHelper::strToUpper($operand2); - } + return $this->executeArrayComparison($operand1, $operand2, $operation, $stack, $recursingArrays); } - $useLowercaseFirstComparison = is_string($operand1) && is_string($operand2) && Functions::getCompatibilityMode() == Functions::COMPATIBILITY_OPENOFFICE; - - // execute the necessary operation - switch ($operation) { - // Greater than - case '>': - if ($useLowercaseFirstComparison) { - $result = $this->strcmpLowercaseFirst($operand1, $operand2) > 0; - } else { - $result = ($operand1 > $operand2); - } - - break; - // Less than - case '<': - if ($useLowercaseFirstComparison) { - $result = $this->strcmpLowercaseFirst($operand1, $operand2) < 0; - } else { - $result = ($operand1 < $operand2); - } - - break; - // Equality - case '=': - if (is_numeric($operand1) && is_numeric($operand2)) { - $result = (abs($operand1 - $operand2) < $this->delta); - } else { - $result = $this->strcmpAllowNull($operand1, $operand2) == 0; - } - - break; - // Greater than or equal - case '>=': - if (is_numeric($operand1) && is_numeric($operand2)) { - $result = ((abs($operand1 - $operand2) < $this->delta) || ($operand1 > $operand2)); - } elseif ($useLowercaseFirstComparison) { - $result = $this->strcmpLowercaseFirst($operand1, $operand2) >= 0; - } else { - $result = $this->strcmpAllowNull($operand1, $operand2) >= 0; - } - - break; - // Less than or equal - case '<=': - if (is_numeric($operand1) && is_numeric($operand2)) { - $result = ((abs($operand1 - $operand2) < $this->delta) || ($operand1 < $operand2)); - } elseif ($useLowercaseFirstComparison) { - $result = $this->strcmpLowercaseFirst($operand1, $operand2) <= 0; - } else { - $result = $this->strcmpAllowNull($operand1, $operand2) <= 0; - } - - break; - // Inequality - case '<>': - if (is_numeric($operand1) && is_numeric($operand2)) { - $result = (abs($operand1 - $operand2) > 1E-14); - } else { - $result = $this->strcmpAllowNull($operand1, $operand2) != 0; - } - - break; - - default: - throw new Exception('Unsupported binary comparison operation'); - } + $result = BinaryComparison::compare($operand1, $operand2, $operation); // Log the result details $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($result)); @@ -5107,35 +5015,6 @@ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2, return $result; } - /** - * Compare two strings in the same way as strcmp() except that lowercase come before uppercase letters. - * - * @param null|string $str1 First string value for the comparison - * @param null|string $str2 Second string value for the comparison - * - * @return int - */ - private function strcmpLowercaseFirst($str1, $str2) - { - $inversedStr1 = Shared\StringHelper::strCaseReverse($str1); - $inversedStr2 = Shared\StringHelper::strCaseReverse($str2); - - return strcmp($inversedStr1 ?? '', $inversedStr2 ?? ''); - } - - /** - * PHP8.1 deprecates passing null to strcmp. - * - * @param null|string $str1 First string value for the comparison - * @param null|string $str2 Second string value for the comparison - * - * @return int - */ - private function strcmpAllowNull($str1, $str2) - { - return strcmp($str1 ?? '', $str2 ?? ''); - } - /** * @param mixed $operand1 * @param mixed $operand2 diff --git a/tests/PhpSpreadsheetTests/Calculation/BinaryComparisonTest.php b/tests/PhpSpreadsheetTests/Calculation/BinaryComparisonTest.php new file mode 100644 index 0000000000..9c06434db1 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/BinaryComparisonTest.php @@ -0,0 +1,61 @@ +compatibilityMode = Functions::getCompatibilityMode(); + Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); + } + + protected function tearDown(): void + { + Functions::setCompatibilityMode($this->compatibilityMode); + } + + /** + * @dataProvider providerBinaryComparison + * + * @param mixed $operand1 + * @param mixed $operand2 + */ + public function testBinaryComparisonOperation( + $operand1, + $operand2, + string $operator, + bool $expectedResultExcel, + bool $expectedResultOpenOffice + ): void { + Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); + $resultExcel = BinaryComparison::compare($operand1, $operand2, $operator); + self::assertEquals($expectedResultExcel, $resultExcel, 'should be Excel compatible'); + + Functions::setCompatibilityMode(Functions::COMPATIBILITY_OPENOFFICE); + $resultOpenOffice = BinaryComparison::compare($operand1, $operand2, $operator); + self::assertEquals($expectedResultOpenOffice, $resultOpenOffice, 'should be OpenOffice compatible'); + } + + public function providerBinaryComparison(): array + { + return require 'tests/data/Calculation/BinaryComparisonOperations.php'; + } + + public function testInvalidOperator(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Unsupported binary comparison operator'); + BinaryComparison::compare(1, 2, '!='); + } +} diff --git a/tests/data/Calculation/BinaryComparisonOperations.php b/tests/data/Calculation/BinaryComparisonOperations.php new file mode 100644 index 0000000000..8a6d79f879 --- /dev/null +++ b/tests/data/Calculation/BinaryComparisonOperations.php @@ -0,0 +1,6 @@ +', false, false], +]; diff --git a/tests/data/CalculationBinaryComparisonOperation.php b/tests/data/CalculationBinaryComparisonOperation.php index dc3be2d680..a8c8e4ece2 100644 --- a/tests/data/CalculationBinaryComparisonOperation.php +++ b/tests/data/CalculationBinaryComparisonOperation.php @@ -238,6 +238,36 @@ false, true, ], + [ + '= NULL = 0', + true, + true, + ], + [ + '= NULL < 0', + false, + false, + ], + [ + '= NULL <= 0', + true, + true, + ], + [ + '= NULL > 0', + false, + false, + ], + [ + '= NULL >= 0', + true, + true, + ], + [ + '= NULL <> 0', + false, + false, + ], [ '="A" > "b"', false,