From e243476f77b254d37c30741e786e77a54258c8e6 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 3 Mar 2022 13:51:18 +0100 Subject: [PATCH] Initial work on refactoring branch pruning logic --- phpstan-baseline.neon | 134 ++++------- .../Calculation/Calculation.php | 205 ++++++---------- .../Calculation/Engine/BranchPruner.php | 223 ++++++++++++++++++ .../Calculation/Token/Stack.php | 92 +++----- 4 files changed, 379 insertions(+), 275 deletions(-) create mode 100644 src/PhpSpreadsheet/Calculation/Engine/BranchPruner.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 484cbe57..11e82195 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -95,11 +95,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:getUnusedBranchStoreKey\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:localeFunc\\(\\) has no return type specified\\.$#" count: 1 @@ -125,6 +120,16 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php + - + message: "#^Offset 'type' does not exist on array\\|null\\.$#" + count: 3 + path: src/PhpSpreadsheet/Calculation/Calculation.php + + - + message: "#^Offset 'value' does not exist on array\\|null\\.$#" + count: 3 + path: src/PhpSpreadsheet/Calculation/Calculation.php + - message: "#^Parameter \\#1 \\$haystack of function stripos expects string, float\\|int\\|string given\\.$#" count: 1 @@ -146,12 +151,12 @@ parameters: path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Parameter \\#1 \\$str(ing)? of function trim expects string, int\\|string given\\.$#" + message: "#^Parameter \\#1 \\$str of function trim expects string, int\\|string given\\.$#" count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Parameter \\#1 \\$str(ing)? of function trim expects string, null given\\.$#" + message: "#^Parameter \\#1 \\$str of function trim expects string, null given\\.$#" count: 2 path: src/PhpSpreadsheet/Calculation/Calculation.php @@ -165,11 +170,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Calculation.php - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:\\$branchPruningEnabled has no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:\\$cellStack has no type specified\\.$#" count: 1 @@ -246,7 +246,12 @@ parameters: path: src/PhpSpreadsheet/Calculation/Calculation.php - - message: "#^Strict comparison using \\=\\=\\= between mixed and null will always evaluate to false\\.$#" + message: "#^Strict comparison using \\=\\=\\= between array and '\\(' will always evaluate to false\\.$#" + count: 1 + path: src/PhpSpreadsheet/Calculation/Calculation.php + + - + message: "#^Strict comparison using \\=\\=\\= between non\\-empty\\-array and null will always evaluate to false\\.$#" count: 2 path: src/PhpSpreadsheet/Calculation/Calculation.php @@ -361,12 +366,12 @@ parameters: path: src/PhpSpreadsheet/Calculation/Engineering/BitWise.php - - message: "#^Parameter \\#1 \\$num(ber)? of function floor expects float(.int)?, float\\|int\\<0, 281474976710655\\>\\|string given\\.$#" + message: "#^Parameter \\#1 \\$number of function floor expects float, float\\|int\\<0, 281474976710655\\>\\|string given\\.$#" count: 1 path: src/PhpSpreadsheet/Calculation/Engineering/BitWise.php - - message: "#^Parameter \\#1 \\$num(ber)? of function floor expects float(.int)?, float\\|int\\|string given\\.$#" + message: "#^Parameter \\#1 \\$number of function floor expects float, float\\|int\\|string given\\.$#" count: 1 path: src/PhpSpreadsheet/Calculation/Engineering/BitWise.php @@ -821,12 +826,12 @@ parameters: path: src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php - - message: "#^Parameter \\#1 \\$(low|start) of function range expects float\\|int\\|string, string\\|null given\\.$#" + message: "#^Parameter \\#1 \\$low of function range expects float\\|int\\|string, string\\|null given\\.$#" count: 1 path: src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php - - message: "#^Parameter \\#2 \\$(high|end) of function range expects float\\|int\\|string, string\\|null given\\.$#" + message: "#^Parameter \\#2 \\$high of function range expects float\\|int\\|string, string\\|null given\\.$#" count: 1 path: src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php @@ -1180,41 +1185,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/TextData/Text.php - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has parameter \\$onlyIf with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has parameter \\$onlyIfNot with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has parameter \\$reference with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has parameter \\$storeKey with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has parameter \\$type with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Token\\\\Stack\\:\\:getStackItem\\(\\) has parameter \\$value with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Token/Stack.php - - message: "#^Elseif branch is unreachable because previous condition is always true\\.$#" count: 1 @@ -1256,12 +1226,12 @@ parameters: path: src/PhpSpreadsheet/Cell/Coordinate.php - - message: "#^Parameter \\#1 \\$(input|array) of function array_chunk expects array, array\\\\|false given\\.$#" + message: "#^Parameter \\#1 \\$input of function array_chunk expects array, array\\\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Cell/Coordinate.php - - message: "#^Parameter \\#2 \\$str(ing)? of function explode expects string, array\\\\|string given\\.$#" + message: "#^Parameter \\#2 \\$str of function explode expects string, array\\\\|string given\\.$#" count: 1 path: src/PhpSpreadsheet/Cell/Coordinate.php @@ -1946,7 +1916,7 @@ parameters: path: src/PhpSpreadsheet/Helper/Html.php - - message: "#^Parameter \\#1 \\$(function|callback) of function call_user_func expects callable\\(\\)\\: mixed, array\\{\\$this\\(PhpOffice\\\\PhpSpreadsheet\\\\Helper\\\\Html\\), mixed\\} given\\.$#" + message: "#^Parameter \\#1 \\$function of function call_user_func expects callable\\(\\)\\: mixed, array\\{\\$this\\(PhpOffice\\\\PhpSpreadsheet\\\\Helper\\\\Html\\), mixed\\} given\\.$#" count: 1 path: src/PhpSpreadsheet/Helper/Html.php @@ -2521,7 +2491,7 @@ parameters: path: src/PhpSpreadsheet/Reader/Xls/MD5.php - - message: "#^Parameter \\#1 \\$(input|array) of function array_values expects array, array\\|false given\\.$#" + message: "#^Parameter \\#1 \\$input of function array_values expects array, array\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Reader/Xls/MD5.php @@ -3336,37 +3306,37 @@ parameters: path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Drawing\\:\\:imagecreatefrombmp\\(\\) should return GdImage\\|resource but returns (GdImage|resource)\\|false\\.$#" + message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Drawing\\:\\:imagecreatefrombmp\\(\\) should return GdImage\\|resource but returns resource\\|false\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#1 \\$(fp|stream) of function feof expects resource, resource\\|false given\\.$#" + message: "#^Parameter \\#1 \\$fp of function feof expects resource, resource\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#1 \\$(fp|stream) of function fread expects resource, resource\\|false given\\.$#" + message: "#^Parameter \\#1 \\$fp of function fread expects resource, resource\\|false given\\.$#" count: 2 path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#1 \\$im(age)? of function imagecolorallocate expects (GdImage|resource), (GdImage|resource)\\|false given\\.$#" + message: "#^Parameter \\#1 \\$im of function imagecolorallocate expects resource, resource\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#1 \\$im(age)? of function imagesetpixel expects (GdImage|resource), (GdImage|resource)\\|false given\\.$#" + message: "#^Parameter \\#1 \\$im of function imagesetpixel expects resource, resource\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#1 \\$(x_size|width) of function imagecreatetruecolor expects int, float\\|int given\\.$#" + message: "#^Parameter \\#1 \\$x_size of function imagecreatetruecolor expects int, float\\|int given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#2 \\$(data|string) of function unpack expects string, string\\|false given\\.$#" + message: "#^Parameter \\#2 \\$data of function unpack expects string, string\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php @@ -3376,7 +3346,7 @@ parameters: path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#2 \\$(y_size|height) of function imagecreatetruecolor expects int, float\\|int given\\.$#" + message: "#^Parameter \\#2 \\$y_size of function imagecreatetruecolor expects int, float\\|int given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php @@ -3396,7 +3366,7 @@ parameters: path: src/PhpSpreadsheet/Shared/Drawing.php - - message: "#^Parameter \\#4 \\$col(or)? of function imagesetpixel expects int, int\\|false given\\.$#" + message: "#^Parameter \\#4 \\$col of function imagesetpixel expects int, int\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Drawing.php @@ -3596,7 +3566,7 @@ parameters: path: src/PhpSpreadsheet/Shared/JAMA/Matrix.php - - message: "#^Parameter \\#1 \\$str(ing)? of function trim expects string, float\\|int given\\.$#" + message: "#^Parameter \\#1 \\$str of function trim expects string, float\\|int given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/JAMA/Matrix.php @@ -3676,7 +3646,7 @@ parameters: path: src/PhpSpreadsheet/Shared/OLE.php - - message: "#^Parameter \\#2 \\$(data|string) of function unpack expects string, string\\|false given\\.$#" + message: "#^Parameter \\#2 \\$data of function unpack expects string, string\\|false given\\.$#" count: 3 path: src/PhpSpreadsheet/Shared/OLE.php @@ -3716,7 +3686,7 @@ parameters: path: src/PhpSpreadsheet/Shared/OLE.php - - message: "#^Parameter \\#1 \\$(var|value) of function count expects array\\|Countable, string given\\.$#" + message: "#^Parameter \\#1 \\$var of function count expects array\\|Countable, string given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/OLE/ChainedBlockStream.php @@ -4061,7 +4031,7 @@ parameters: path: src/PhpSpreadsheet/Shared/Trend/PolynomialBestFit.php - - message: "#^Parameter \\#2 \\.\\.\\.\\$(args|arrays) of function array_merge expects array, float given\\.$#" + message: "#^Parameter \\#2 \\.\\.\\.\\$args of function array_merge expects array, float given\\.$#" count: 1 path: src/PhpSpreadsheet/Shared/Trend/PolynomialBestFit.php @@ -4361,7 +4331,7 @@ parameters: path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php - - message: "#^Parameter \\#2 \\$str(ing)? of function explode expects string, string\\|null given\\.$#" + message: "#^Parameter \\#2 \\$str of function explode expects string, string\\|null given\\.$#" count: 1 path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php @@ -4486,7 +4456,7 @@ parameters: path: src/PhpSpreadsheet/Worksheet/PageSetup.php - - message: "#^Parameter \\#2 \\$str(ing)? of function explode expects string, string\\|null given\\.$#" + message: "#^Parameter \\#2 \\$str of function explode expects string, string\\|null given\\.$#" count: 5 path: src/PhpSpreadsheet/Worksheet/PageSetup.php @@ -4581,7 +4551,7 @@ parameters: path: src/PhpSpreadsheet/Worksheet/Worksheet.php - - message: "#^Parameter \\#1 \\$(input|array) of function array_splice expects array, ArrayObject\\ given\\.$#" + message: "#^Parameter \\#1 \\$input of function array_splice expects array, ArrayObject\\ given\\.$#" count: 1 path: src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -4606,7 +4576,7 @@ parameters: path: src/PhpSpreadsheet/Worksheet/Worksheet.php - - message: "#^Parameter \\#2 \\$(start|offset) of function substr expects int, int(\\<0, max\\>)?\\|false given\\.$#" + message: "#^Parameter \\#2 \\$start of function substr expects int, int\\<0, max\\>\\|false given\\.$#" count: 2 path: src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -4931,12 +4901,12 @@ parameters: path: src/PhpSpreadsheet/Writer/Html.php - - message: "#^Parameter \\#1 \\$im(age)? of function imagepng expects (GdImage|resource), GdImage\\|resource given\\.$#" + message: "#^Parameter \\#1 \\$im of function imagepng expects resource, GdImage\\|resource given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Html.php - - message: "#^Parameter \\#1 \\$str(ing)? of function base64_encode expects string, string\\|false given\\.$#" + message: "#^Parameter \\#1 \\$str of function base64_encode expects string, string\\|false given\\.$#" count: 2 path: src/PhpSpreadsheet/Writer/Html.php @@ -5066,12 +5036,12 @@ parameters: path: src/PhpSpreadsheet/Writer/Xls.php - - message: "#^Parameter \\#1 \\$im(age)? of function imagepng expects (GdImage|resource), GdImage\\|resource given\\.$#" + message: "#^Parameter \\#1 \\$im of function imagepng expects resource, GdImage\\|resource given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xls.php - - message: "#^Parameter \\#1 \\$im(age)? of function imagepng expects (GdImage|resource), (GdImage|resource)\\|false given\\.$#" + message: "#^Parameter \\#1 \\$im of function imagepng expects resource, resource\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xls.php @@ -5256,7 +5226,7 @@ parameters: path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php - - message: "#^Parameter \\#1 \\$im(age)? of function imagecolorat expects (GdImage|resource), GdImage\\|resource given\\.$#" + message: "#^Parameter \\#1 \\$im of function imagecolorat expects resource, GdImage\\|resource given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -5271,12 +5241,12 @@ parameters: path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php - - message: "#^Parameter \\#2 \\$col(or)? of function imagecolorsforindex expects int, int\\|false given\\.$#" + message: "#^Parameter \\#2 \\$col of function imagecolorsforindex expects int, int\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php - - message: "#^Parameter \\#2 \\$(data|string) of function unpack expects string, string\\|false given\\.$#" + message: "#^Parameter \\#2 \\$data of function unpack expects string, string\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -5286,7 +5256,7 @@ parameters: path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php - - message: "#^Parameter \\#2 \\$(pieces|array) of function implode expects array(\\|null)?, array\\\\|false given\\.$#" + message: "#^Parameter \\#2 \\$pieces of function implode expects array, array\\\\|false given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -5371,7 +5341,7 @@ parameters: path: src/PhpSpreadsheet/Writer/Xlsx.php - - message: "#^Parameter \\#1 \\$(function|callback) of function call_user_func expects callable\\(\\)\\: mixed, string given\\.$#" + message: "#^Parameter \\#1 \\$function of function call_user_func expects callable\\(\\)\\: mixed, string given\\.$#" count: 1 path: src/PhpSpreadsheet/Writer/Xlsx.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index b2384fb5..691d49b7 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -2,9 +2,11 @@ namespace PhpOffice\PhpSpreadsheet\Calculation; +use PhpOffice\PhpSpreadsheet\Calculation\Engine\BranchPruner; use PhpOffice\PhpSpreadsheet\Calculation\Engine\CyclicReferenceStack; use PhpOffice\PhpSpreadsheet\Calculation\Engine\Logger; use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue; +use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; use PhpOffice\PhpSpreadsheet\Calculation\Information\Value; use PhpOffice\PhpSpreadsheet\Calculation\Token\Stack; use PhpOffice\PhpSpreadsheet\Cell\Cell; @@ -87,12 +89,13 @@ class Calculation private $calculationCacheEnabled = true; /** - * Used to generate unique store keys. - * - * @var int + * @var BranchPruner */ - private $branchStoreKeyCounter = 0; + private $branchPruner; + /** + * @var bool + */ private $branchPruningEnabled = true; /** @@ -2761,6 +2764,7 @@ class Calculation $this->spreadsheet = $spreadsheet; $this->cyclicReferenceStack = new CyclicReferenceStack(); $this->debugLog = new Logger($this->cyclicReferenceStack); + $this->branchPruner = new BranchPruner($this->branchPruningEnabled); self::$referenceHelper = ReferenceHelper::getInstance(); } @@ -2804,7 +2808,7 @@ class Calculation public function flushInstance(): void { $this->clearCalculationCache(); - $this->clearBranchStore(); + $this->branchPruner->clearBranchStore(); } /** @@ -2956,6 +2960,7 @@ class Calculation public function setBranchPruningEnabled($enabled): void { $this->branchPruningEnabled = $enabled; + $this->branchPruner = new BranchPruner($this->branchPruningEnabled); } public function enableBranchPruning(): void @@ -2968,11 +2973,6 @@ class Calculation $this->setBranchPruningEnabled(false); } - public function clearBranchStore(): void - { - $this->branchStoreKeyCounter = 0; - } - /** * Get the currently defined locale code. * @@ -3924,58 +3924,19 @@ class Calculation // Start with initialisation $index = 0; - $stack = new Stack(); + $stack = new Stack($this->branchPruner); $output = []; $expectingOperator = false; // We use this test in syntax-checking the expression to determine when a // - is a negation or + is a positive operator rather than an operation $expectingOperand = false; // We use this test in syntax-checking the expression to determine whether an operand // should be null in a function call - // IF branch pruning - // currently pending storeKey (last item of the storeKeysStack - $pendingStoreKey = null; - // stores a list of storeKeys (string[]) - $pendingStoreKeysStack = []; - $expectingConditionMap = []; // ['storeKey' => true, ...] - $expectingThenMap = []; // ['storeKey' => true, ...] - $expectingElseMap = []; // ['storeKey' => true, ...] - $parenthesisDepthMap = []; // ['storeKey' => 4, ...] - // The guts of the lexical parser // Loop through the formula extracting each operator and operand in turn while (true) { // Branch pruning: we adapt the output item to the context (it will // be used to limit its computation) - $currentCondition = null; - $currentOnlyIf = null; - $currentOnlyIfNot = null; - $previousStoreKey = null; - $pendingStoreKey = end($pendingStoreKeysStack); - - if ($this->branchPruningEnabled) { - // this is a condition ? - if (isset($expectingConditionMap[$pendingStoreKey]) && $expectingConditionMap[$pendingStoreKey]) { - $currentCondition = $pendingStoreKey; - $stackDepth = count($pendingStoreKeysStack); - if ($stackDepth > 1) { // nested if - $previousStoreKey = $pendingStoreKeysStack[$stackDepth - 2]; - } - } - if (isset($expectingThenMap[$pendingStoreKey]) && $expectingThenMap[$pendingStoreKey]) { - $currentOnlyIf = $pendingStoreKey; - } elseif (isset($previousStoreKey)) { - if (isset($expectingThenMap[$previousStoreKey]) && $expectingThenMap[$previousStoreKey]) { - $currentOnlyIf = $previousStoreKey; - } - } - if (isset($expectingElseMap[$pendingStoreKey]) && $expectingElseMap[$pendingStoreKey]) { - $currentOnlyIfNot = $pendingStoreKey; - } elseif (isset($previousStoreKey)) { - if (isset($expectingElseMap[$previousStoreKey]) && $expectingElseMap[$previousStoreKey]) { - $currentOnlyIfNot = $previousStoreKey; - } - } - } + $this->branchPruner->initialiseForLoop(); $opCharacter = $formula[$index]; // Get the first character of the value at the current index position @@ -3987,11 +3948,11 @@ class Calculation if ($opCharacter == '-' && !$expectingOperator) { // Is it a negation instead of a minus? // Put a negation on the stack - $stack->push('Unary Operator', '~', null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $stack->push('Unary Operator', '~'); ++$index; // and drop the negation symbol } elseif ($opCharacter == '%' && $expectingOperator) { // Put a percentage on the stack - $stack->push('Unary Operator', '%', null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $stack->push('Unary Operator', '%'); ++$index; } elseif ($opCharacter == '+' && !$expectingOperator) { // Positive (unary plus rather than binary operator plus) can be discarded? ++$index; // Drop the redundant plus symbol @@ -4008,13 +3969,13 @@ class Calculation } // Finally put our current operator onto the stack - $stack->push('Binary Operator', $opCharacter, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $stack->push('Binary Operator', $opCharacter); ++$index; $expectingOperator = false; - } elseif ($opCharacter == ')' && $expectingOperator) { // Are we expecting to close a parenthesis? + } elseif ($opCharacter == ')' && $expectingOperator) { // Are we expecting to close a parenthesis? $expectingOperand = false; - while (($o2 = $stack->pop()) && $o2['value'] != '(') { // Pop off the stack back to the last ( + while (($o2 = $stack->pop()) && $o2['value'] !== '(') { // Pop off the stack back to the last ( if ($o2 === null) { return $this->raiseFormulaError('Formula Error: Unexpected closing brace ")"'); } @@ -4024,30 +3985,19 @@ class Calculation // Branch pruning we decrease the depth whether is it a function // call or a parenthesis - if (!empty($pendingStoreKey)) { - --$parenthesisDepthMap[$pendingStoreKey]; - } + $this->branchPruner->decrementDepth(); if (is_array($d) && preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/miu', $d['value'], $matches)) { // Did this parenthesis just close a function? - if (!empty($pendingStoreKey) && $parenthesisDepthMap[$pendingStoreKey] == -1) { - // we are closing an IF( - if ($d['value'] !== 'IF(') { - return $this->raiseFormulaError('Parser bug we should be in an "IF("'); - } - if ($expectingConditionMap[$pendingStoreKey]) { - return $this->raiseFormulaError('We should not be expecting a condition'); - } - $expectingThenMap[$pendingStoreKey] = false; - $expectingElseMap[$pendingStoreKey] = false; - --$parenthesisDepthMap[$pendingStoreKey]; - array_pop($pendingStoreKeysStack); - unset($pendingStoreKey); + try { + $this->branchPruner->closingBrace($d['value']); + } catch (Exception $e) { + return $this->raiseFormulaError($e->getMessage()); } $functionName = $matches[1]; // Get the function name $d = $stack->pop(); - $argumentCount = $d['value']; // See how many arguments there were (argument count is the next value stored on the stack) + $argumentCount = $d['value'] ?? 0; // See how many arguments there were (argument count is the next value stored on the stack) $output[] = $d; // Dump the argument count on the output $output[] = $stack->pop(); // Pop the function and push onto the output if (isset(self::$controlFunctions[$functionName])) { @@ -4105,23 +4055,14 @@ class Calculation } } ++$index; - } elseif ($opCharacter == ',') { // Is this the separator for function arguments? - if ( - !empty($pendingStoreKey) && - $parenthesisDepthMap[$pendingStoreKey] == 0 - ) { - // We must go to the IF next argument - if ($expectingConditionMap[$pendingStoreKey]) { - $expectingConditionMap[$pendingStoreKey] = false; - $expectingThenMap[$pendingStoreKey] = true; - } elseif ($expectingThenMap[$pendingStoreKey]) { - $expectingThenMap[$pendingStoreKey] = false; - $expectingElseMap[$pendingStoreKey] = true; - } elseif ($expectingElseMap[$pendingStoreKey]) { - return $this->raiseFormulaError('Reaching fourth argument of an IF'); - } + } elseif ($opCharacter == ',') { // Is this the separator for function arguments? + try { + $this->branchPruner->argumentSeparator(); + } catch (Exception $e) { + return $this->raiseFormulaError($e->getMessage()); } - while (($o2 = $stack->pop()) && $o2['value'] != '(') { // Pop off the stack back to the last ( + + while (($o2 = $stack->pop()) && $o2['value'] !== '(') { // Pop off the stack back to the last ( if ($o2 === null) { return $this->raiseFormulaError('Formula Error: Unexpected ,'); } @@ -4130,27 +4071,32 @@ class Calculation // If we've a comma when we're expecting an operand, then what we actually have is a null operand; // so push a null onto the stack if (($expectingOperand) || (!$expectingOperator)) { - $output[] = ['type' => 'Empty Argument', 'value' => self::$excelConstants['NULL'], 'reference' => null]; + $output[] = ['type' => 'Empty Argument', 'value' => self::$excelConstants['NULL'], 'reference' => 'NULL']; } // make sure there was a function $d = $stack->last(2); - if (!preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/miu', $d['value'], $matches)) { + if (!preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/miu', $d['value'] ?? '', $matches)) { + // Can we inject a dummy function at this point so that the braces at least have some context + // because at least the braces are paired up (at this stage in the formula) + // MS Excel allows this if the content is cell references; but doesn't allow actual values, + // but at this point, we can't differentiate (so allow both) return $this->raiseFormulaError('Formula Error: Unexpected ,'); } + + /** @var array $d */ $d = $stack->pop(); - $itemStoreKey = $d['storeKey'] ?? null; - $itemOnlyIf = $d['onlyIf'] ?? null; - $itemOnlyIfNot = $d['onlyIfNot'] ?? null; - $stack->push($d['type'], ++$d['value'], $d['reference'], $itemStoreKey, $itemOnlyIf, $itemOnlyIfNot); // increment the argument count - $stack->push('Brace', '(', null, $itemStoreKey, $itemOnlyIf, $itemOnlyIfNot); // put the ( back on, we'll need to pop back to it again + ++$d['value']; // increment the argument count + + $stack->pushStackItem($d); + $stack->push('Brace', '('); // put the ( back on, we'll need to pop back to it again + $expectingOperator = false; $expectingOperand = true; ++$index; } elseif ($opCharacter == '(' && !$expectingOperator) { - if (!empty($pendingStoreKey)) { // Branch pruning: we go deeper - ++$parenthesisDepthMap[$pendingStoreKey]; - } - $stack->push('Brace', '(', null, $currentCondition, $currentOnlyIf, $currentOnlyIf); + // Branch pruning: we go deeper + $this->branchPruner->incrementDepth(); + $stack->push('Brace', '(', null); ++$index; } elseif ($isOperandOrFunction && !$expectingOperator) { // do we now have a function/variable/number? $expectingOperator = true; @@ -4167,25 +4113,17 @@ class Calculation } // here $matches[1] will contain values like "IF" // and $val "IF(" - if ($this->branchPruningEnabled && ($valToUpper == 'IF(')) { // we handle a new if - $pendingStoreKey = $this->getUnusedBranchStoreKey(); - $pendingStoreKeysStack[] = $pendingStoreKey; - $expectingConditionMap[$pendingStoreKey] = true; - $parenthesisDepthMap[$pendingStoreKey] = 0; - } else { // this is not an if but we go deeper - if (!empty($pendingStoreKey) && array_key_exists($pendingStoreKey, $parenthesisDepthMap)) { - ++$parenthesisDepthMap[$pendingStoreKey]; - } - } - $stack->push('Function', $valToUpper, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $this->branchPruner->functionCall($valToUpper); + + $stack->push('Function', $valToUpper); // tests if the function is closed right after opening $ax = preg_match('/^\s*\)/u', substr($formula, $index + $length)); if ($ax) { - $stack->push('Operand Count for Function ' . $valToUpper . ')', 0, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $stack->push('Operand Count for Function ' . $valToUpper . ')', 0); $expectingOperator = true; } else { - $stack->push('Operand Count for Function ' . $valToUpper . ')', 1, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $stack->push('Operand Count for Function ' . $valToUpper . ')', 1); $expectingOperator = false; } $stack->push('Brace', '('); @@ -4217,7 +4155,7 @@ class Calculation } // unescape any apostrophes or double quotes in worksheet name $val = str_replace(["''", '""'], ["'", '"'], $val); - $outputItem = $stack->getStackItem('Cell Reference', $val, $val, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $outputItem = $stack->getStackItem('Cell Reference', $val, $val); $output[] = $outputItem; } else { // it's a variable, constant, string, number or boolean @@ -4307,18 +4245,18 @@ class Calculation } } - $details = $stack->getStackItem($stackItemType, $val, $stackItemReference, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $details = $stack->getStackItem($stackItemType, $val, $stackItemReference); if ($localeConstant) { $details['localeValue'] = $localeConstant; } $output[] = $details; } $index += $length; - } elseif ($opCharacter == '$') { // absolute row or column range + } elseif ($opCharacter == '$') { // absolute row or column range ++$index; - } elseif ($opCharacter == ')') { // miscellaneous error checking + } elseif ($opCharacter == ')') { // miscellaneous error checking if ($expectingOperand) { - $output[] = ['type' => 'Empty Argument', 'value' => self::$excelConstants['NULL'], 'reference' => null]; + $output[] = ['type' => 'Empty Argument', 'value' => self::$excelConstants['NULL'], 'reference' => 'NULL']; $expectingOperand = false; $expectingOperator = true; } else { @@ -4345,7 +4283,7 @@ class Calculation } if ($formula[$index] == ' ') { - while ($formula[$index] == ' ') { + while ($formula[$index] === ' ') { ++$index; } @@ -4355,9 +4293,9 @@ class Calculation ($expectingOperator) && ( (preg_match('/^' . self::CALCULATION_REGEXP_CELLREF . '.*/Ui', substr($formula, $index), $match)) && - ($output[count($output) - 1]['type'] == 'Cell Reference') || + ($output[count($output) - 1]['type'] === 'Cell Reference') || (preg_match('/^' . self::CALCULATION_REGEXP_DEFINEDNAME . '.*/miu', substr($formula, $index), $match)) && - ($output[count($output) - 1]['type'] == 'Defined Name' || $output[count($output) - 1]['type'] == 'Value') + ($output[count($output) - 1]['type'] === 'Defined Name' || $output[count($output) - 1]['type'] === 'Value') ) ) { while ( @@ -4374,7 +4312,8 @@ class Calculation } } - while (($op = $stack->pop()) !== null) { // pop everything off the stack and push onto output + while (($op = $stack->pop()) !== null) { + // pop everything off the stack and push onto output if ((is_array($op) && $op['value'] == '(') || ($op === '(')) { return $this->raiseFormulaError("Formula Error: Expecting ')'"); // if there are any opening braces on the stack, then braces were unbalanced } @@ -4416,7 +4355,7 @@ class Calculation */ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null) { - if ($tokens == false) { + if ($tokens === false) { return false; } @@ -4424,7 +4363,7 @@ class Calculation // so we store the parent cell collection so that we can re-attach it when necessary $pCellWorksheet = ($cell !== null) ? $cell->getWorksheet() : null; $pCellParent = ($cell !== null) ? $cell->getParent() : null; - $stack = new Stack(); + $stack = new Stack($this->branchPruner); // Stores branches that have been pruned $fakedForBranchPruning = []; @@ -4433,7 +4372,6 @@ class Calculation // Loop through each token in turn foreach ($tokens as $tokenData) { $token = $tokenData['value']; - // Branch pruning: skip useless resolutions $storeKey = $tokenData['storeKey'] ?? null; if ($this->branchPruningEnabled && isset($tokenData['onlyIf'])) { @@ -4447,7 +4385,7 @@ class Calculation } if ( - isset($storeValue) + (isset($storeValue) || $tokenData['reference'] === 'NULL') && (!$storeValueAsBool || ErrorValue::isError($storeValue) || ($storeValue === 'Pruned branch')) ) { // If branching value is not true, we don't need to compute @@ -4477,8 +4415,9 @@ class Calculation $wrappedItem = end($storeValue); $storeValue = end($wrappedItem); } + if ( - isset($storeValue) + (isset($storeValue) || $tokenData['reference'] === 'NULL') && ($storeValueAsBool || ErrorValue::isError($storeValue) || ($storeValue === 'Pruned branch')) ) { // If branching value is true, we don't need to compute @@ -5106,8 +5045,8 @@ class Calculation case '/': if ($operand2 == 0) { // Trap for Divide by Zero error - $stack->push('Error', '#DIV/0!'); - $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails('#DIV/0!')); + $stack->push('Error', ExcelError::DIV0()); + $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails(ExcelError::DIV0())); return false; } @@ -5390,14 +5329,6 @@ class Calculation return $args; } - private function getUnusedBranchStoreKey() - { - $storeKeyValue = 'storeKey-' . $this->branchStoreKeyCounter; - ++$this->branchStoreKeyCounter; - - return $storeKeyValue; - } - private function getTokensAsString($tokens) { $tokensStr = array_map(function ($token) { diff --git a/src/PhpSpreadsheet/Calculation/Engine/BranchPruner.php b/src/PhpSpreadsheet/Calculation/Engine/BranchPruner.php new file mode 100644 index 00000000..9cd767e6 --- /dev/null +++ b/src/PhpSpreadsheet/Calculation/Engine/BranchPruner.php @@ -0,0 +1,223 @@ +branchPruningEnabled = $branchPruningEnabled; + } + + public function clearBranchStore(): void + { + $this->branchStoreKeyCounter = 0; + } + + public function initialiseForLoop(): void + { + $this->currentCondition = null; + $this->currentOnlyIf = null; + $this->currentOnlyIfNot = null; + $this->previousStoreKey = null; + $this->pendingStoreKey = empty($this->storeKeysStack) ? null : end($this->storeKeysStack); + + if ($this->branchPruningEnabled) { + $this->initialiseCondition(); + $this->initialiseThen(); + $this->initialiseElse(); + } + } + + private function initialiseCondition(): void + { + if (isset($this->conditionMap[$this->pendingStoreKey]) && $this->conditionMap[$this->pendingStoreKey]) { + $this->currentCondition = $this->pendingStoreKey; + $stackDepth = count($this->storeKeysStack); + if ($stackDepth > 1) { + // nested if + $this->previousStoreKey = $this->storeKeysStack[$stackDepth - 2]; + } + } + } + + private function initialiseThen(): void + { + if (isset($this->thenMap[$this->pendingStoreKey]) && $this->thenMap[$this->pendingStoreKey]) { + $this->currentOnlyIf = $this->pendingStoreKey; + } elseif ( + isset($this->previousStoreKey, $this->thenMap[$this->previousStoreKey]) + && $this->thenMap[$this->previousStoreKey] + ) { + $this->currentOnlyIf = $this->previousStoreKey; + } + } + + private function initialiseElse(): void + { + if (isset($this->elseMap[$this->pendingStoreKey]) && $this->elseMap[$this->pendingStoreKey]) { + $this->currentOnlyIfNot = $this->pendingStoreKey; + } elseif ( + isset($this->previousStoreKey, $this->elseMap[$this->previousStoreKey]) + && $this->elseMap[$this->previousStoreKey] + ) { + $this->currentOnlyIfNot = $this->previousStoreKey; + } + } + + public function decrementDepth(): void + { + if (!empty($this->pendingStoreKey)) { + --$this->braceDepthMap[$this->pendingStoreKey]; + } + } + + public function incrementDepth(): void + { + if (!empty($this->pendingStoreKey)) { + ++$this->braceDepthMap[$this->pendingStoreKey]; + } + } + + public function functionCall(string $functionName): void + { + if ($this->branchPruningEnabled && ($functionName === 'IF(')) { + // we handle a new if + $this->pendingStoreKey = $this->getUnusedBranchStoreKey(); + $this->storeKeysStack[] = $this->pendingStoreKey; + $this->conditionMap[$this->pendingStoreKey] = true; + $this->braceDepthMap[$this->pendingStoreKey] = 0; + } elseif (!empty($this->pendingStoreKey) && array_key_exists($this->pendingStoreKey, $this->braceDepthMap)) { + // this is not an if but we go deeper + ++$this->braceDepthMap[$this->pendingStoreKey]; + } + } + + public function argumentSeparator(): void + { + if (!empty($this->pendingStoreKey) && $this->braceDepthMap[$this->pendingStoreKey] === 0) { + // We must go to the IF next argument + if ($this->conditionMap[$this->pendingStoreKey]) { + $this->conditionMap[$this->pendingStoreKey] = false; + $this->thenMap[$this->pendingStoreKey] = true; + } elseif ($this->thenMap[$this->pendingStoreKey]) { + $this->thenMap[$this->pendingStoreKey] = false; + $this->elseMap[$this->pendingStoreKey] = true; + } elseif ($this->elseMap[$this->pendingStoreKey]) { + throw new Exception('Reaching fourth argument of an IF'); + } + } + } + + /** + * @param mixed $value + */ + public function closingBrace($value): void + { + if (!empty($this->pendingStoreKey) && $this->braceDepthMap[$this->pendingStoreKey] === -1) { + // we are closing an IF( + if ($value !== 'IF(') { + throw new Exception('Parser bug we should be in an "IF("'); + } + + if ($this->conditionMap[$this->pendingStoreKey]) { + throw new Exception('We should not be expecting a condition'); + } + + $this->thenMap[$this->pendingStoreKey] = false; + $this->elseMap[$this->pendingStoreKey] = false; + --$this->braceDepthMap[$this->pendingStoreKey]; + array_pop($this->storeKeysStack); + $this->pendingStoreKey = null; + } + } + + public function currentCondition(): ?string + { + return $this->currentCondition; + } + + public function currentOnlyIf(): ?string + { + return $this->currentOnlyIf; + } + + public function currentOnlyIfNot(): ?string + { + return $this->currentOnlyIfNot; + } + + private function getUnusedBranchStoreKey(): string + { + $storeKeyValue = 'storeKey-' . $this->branchStoreKeyCounter; + ++$this->branchStoreKeyCounter; + + return $storeKeyValue; + } +} diff --git a/src/PhpSpreadsheet/Calculation/Token/Stack.php b/src/PhpSpreadsheet/Calculation/Token/Stack.php index 941e1ad7..26ffc011 100644 --- a/src/PhpSpreadsheet/Calculation/Token/Stack.php +++ b/src/PhpSpreadsheet/Calculation/Token/Stack.php @@ -3,9 +3,15 @@ namespace PhpOffice\PhpSpreadsheet\Calculation\Token; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; +use PhpOffice\PhpSpreadsheet\Calculation\Engine\BranchPruner; class Stack { + /** + * @var BranchPruner + */ + private $branchPruner; + /** * The parser stack for formulae. * @@ -20,12 +26,15 @@ class Stack */ private $count = 0; + public function __construct(BranchPruner $branchPruner) + { + $this->branchPruner = $branchPruner; + } + /** * Return the number of entries on the stack. - * - * @return int */ - public function count() + public function count(): int { return $this->count; } @@ -33,25 +42,11 @@ class Stack /** * Push a new entry onto the stack. * - * @param mixed $type * @param mixed $value - * @param mixed $reference - * @param null|string $storeKey will store the result under this alias - * @param null|string $onlyIf will only run computation if the matching - * store key is true - * @param null|string $onlyIfNot will only run computation if the matching - * store key is false */ - public function push( - $type, - $value, - $reference = null, - $storeKey = null, - $onlyIf = null, - $onlyIfNot = null - ): void { - $stackItem = $this->getStackItem($type, $value, $reference, $storeKey, $onlyIf, $onlyIfNot); - + public function push(string $type, $value, ?string $reference = null): void + { + $stackItem = $this->getStackItem($type, $value, $reference); $this->stack[$this->count++] = $stackItem; if ($type == 'Function') { @@ -62,29 +57,37 @@ class Stack } } - public function getStackItem( - $type, - $value, - $reference = null, - $storeKey = null, - $onlyIf = null, - $onlyIfNot = null - ) { + public function pushStackItem(array $stackItem): void + { + $this->stack[$this->count++] = $stackItem; + } + + /** + * @param mixed $value + */ + public function getStackItem(string $type, $value, ?string $reference = null): array + { $stackItem = [ 'type' => $type, 'value' => $value, 'reference' => $reference, ]; - if (isset($storeKey)) { + // will store the result under this alias + $storeKey = $this->branchPruner->currentCondition(); + if (isset($storeKey) || $reference === 'NULL') { $stackItem['storeKey'] = $storeKey; } - if (isset($onlyIf)) { + // will only run computation if the matching store key is true + $onlyIf = $this->branchPruner->currentOnlyIf(); + if (isset($onlyIf) || $reference === 'NULL') { $stackItem['onlyIf'] = $onlyIf; } - if (isset($onlyIfNot)) { + // will only run computation if the matching store key is false + $onlyIfNot = $this->branchPruner->currentOnlyIfNot(); + if (isset($onlyIfNot) || $reference === 'NULL') { $stackItem['onlyIfNot'] = $onlyIfNot; } @@ -93,10 +96,8 @@ class Stack /** * Pop the last entry from the stack. - * - * @return mixed */ - public function pop() + public function pop(): ?array { if ($this->count > 0) { return $this->stack[--$this->count]; @@ -107,12 +108,8 @@ class Stack /** * Return an entry from the stack without removing it. - * - * @param int $n number indicating how far back in the stack we want to look - * - * @return mixed */ - public function last($n = 1) + public function last(int $n = 1): ?array { if ($this->count - $n < 0) { return null; @@ -129,21 +126,4 @@ class Stack $this->stack = []; $this->count = 0; } - - public function __toString() - { - $str = 'Stack: '; - foreach ($this->stack as $index => $item) { - if ($index > $this->count - 1) { - break; - } - $value = $item['value'] ?? 'no value'; - while (is_array($value)) { - $value = array_pop($value); - } - $str .= $value . ' |> '; - } - - return $str; - } }