Clientbase
/
PhpSpreadsheet
mirror of https://github.com/basemaster/PhpSpreadsheet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Extract some methods from the Calculation Engine into dedicated classes (#2537) 

* Move binary comparisons out into a dedicated class
This commit is contained in:
Mark Baker 2022-02-04 16:02:29 +01:00 committed by GitHub
parent 0aed245fbc
commit 6b746dc05f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 313 additions and 166 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
phpstan-baseline.neon
View File

@ -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\\<int, string\\>\\|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

181
src/PhpSpreadsheet/Calculation/BinaryComparison.php Normal file
View File

@ -0,0 +1,181 @@
<?php
namespace PhpOffice\PhpSpreadsheet\Calculation;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
class BinaryComparison
{
/**
* Epsilon Precision used for comparisons in calculations.
*/
private const DELTA = 0.1e-12;
/**
* 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
*/
private static function strcmpLowercaseFirst($str1, $str2): int
{
$inversedStr1 = StringHelper::strCaseReverse($str1 ?? '');
$inversedStr2 = 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
*/
private static function strcmpAllowNull($str1, $str2): int
{
return strcmp($str1 ?? '', $str2 ?? '');
}
/**
* @param mixed $operand1
* @param mixed $operand2
*/
public static function compare($operand1, $operand2, string $operator): bool
{
// Simple validate the two operands if they are string values
if (is_string($operand1) && $operand1 > '' && $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;
}
}

141
src/PhpSpreadsheet/Calculation/Calculation.php
View File

@ -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 @@ class Calculation
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 @@ class Calculation
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 @@ class Calculation
}
/**
* @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 @@ class Calculation
// 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 @@ class Calculation
}
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 @@ class Calculation
}
/**
* @param null|string $cellID
* @param mixed $operand1
* @param mixed $operand2
* @param string $operation
@ -5007,97 +4998,14 @@ class Calculation
*
* @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);
return $this->executeArrayComparison($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);
}
}
$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 @@ class Calculation
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

61
tests/PhpSpreadsheetTests/Calculation/BinaryComparisonTest.php Normal file
View File

@ -0,0 +1,61 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\BinaryComparison;
use PhpOffice\PhpSpreadsheet\Calculation\Exception;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
use PHPUnit\Framework\TestCase;
class BinaryComparisonTest extends TestCase
{
/**
* @var string
*/
private $compatibilityMode;
protected function setUp(): void
{
$this->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, '!=');
}
}

6
tests/data/Calculation/BinaryComparisonOperations.php Normal file
View File

@ -0,0 +1,6 @@
<?php
return [
[null, null, '=', true, true],
[null, null, '<>', false, false],
];

30
tests/data/CalculationBinaryComparisonOperation.php
View File

@ -238,6 +238,36 @@ return [
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,