diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index e2eefd0a63..30432780c4 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -2256,7 +2256,6 @@ public function flushInstance() { $this->clearCalculationCache(); $this->clearBranchStore(); - $this->branchStoreKeyCounter = 0; } /** @@ -3443,7 +3442,6 @@ private function _parseFormula($formula, Cell $pCell = null) } if (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/i', $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(') { @@ -3519,9 +3517,7 @@ private function _parseFormula($formula, Cell $pCell = null) } ++$index; } elseif ($opCharacter == ',') { // Is this the separator for function arguments? - - if ( - !empty($pendingStoreKey) && + if (!empty($pendingStoreKey) && $parenthesisDepthMap[$pendingStoreKey] == 0 ) { // We must go to the IF next argument @@ -3590,8 +3586,7 @@ private function _parseFormula($formula, Cell $pCell = null) } } - $stack->push('Function', $valToUpper, null, - $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $stack->push('Function', $valToUpper, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); // tests if the function is closed right after opening $ax = preg_match('/^\s*(\s*\))/ui', substr($formula, $index + $length), $amatch); if ($ax) { @@ -3626,9 +3621,7 @@ private function _parseFormula($formula, Cell $pCell = null) } } - $outputItem = $stack->getStackItem('Cell Reference', $val, - $val, $currentCondition, $currentOnlyIf, - $currentOnlyIfNot); + $outputItem = $stack->getStackItem('Cell Reference', $val, $val, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); $output[] = $outputItem; } else { // it's a variable, constant, string, number or boolean @@ -3802,7 +3795,6 @@ private function processTokenStack($tokens, $cellID = null, Cell $pCell = null) if (isset($storeValue) && (($storeValue !== true) || ($storeValue === 'Pruned branch')) ) { - // If branching value is not true, we don't need to compute if (!isset($fakedForBranchPruning['onlyIf-' . $onlyIfStoreKey])) { $stack->push('Value', 'Pruned branch (only if ' . $onlyIfStoreKey . ') ' . $token); @@ -3831,7 +3823,6 @@ private function processTokenStack($tokens, $cellID = null, Cell $pCell = null) if (isset($storeValue) && ($storeValue || ($storeValue === 'Pruned branch')) ) { - // If branching value is true, we don't need to compute if (!isset($fakedForBranchPruning['onlyIfNot-' . $onlyIfNotStoreKey])) { $stack->push('Value', 'Pruned branch (only if not ' . $onlyIfNotStoreKey . ') ' . $token); diff --git a/src/PhpSpreadsheet/Calculation/Token/Stack.php b/src/PhpSpreadsheet/Calculation/Token/Stack.php index 1f052423a2..61a93ea095 100644 --- a/src/PhpSpreadsheet/Calculation/Token/Stack.php +++ b/src/PhpSpreadsheet/Calculation/Token/Stack.php @@ -50,8 +50,7 @@ public function push( $onlyIf = null, $onlyIfNot = null ) { - $stackItem = $this->getStackItem($type, $value, $reference, $storeKey, - $onlyIf, $onlyIfNot); + $stackItem = $this->getStackItem($type, $value, $reference, $storeKey, $onlyIf, $onlyIfNot); $this->stack[$this->count++] = $stackItem; diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php index 7fff307509..4ff8465d1b 100644 --- a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php @@ -141,9 +141,10 @@ public function testFormulaWithOptionalArgumentsAndRequiredCellReferenceShouldPa self::assertEquals(5, $cell->getCalculatedValue(), 'missing arguments should be filled with null'); } - public function testBranchPruningFormulaParsing() + public function testBranchPruningFormulaParsingSimpleCase() { $calculation = Calculation::getInstance(); + $calculation->flushInstance(); // resets the ids // Very simple formula $formula = '=IF(A1="please +",B1)'; @@ -166,7 +167,11 @@ public function testBranchPruningFormulaParsing() } $this->assertTrue($foundEqualAssociatedToStoreKey); $this->assertTrue($foundConditionalOnB1); + } + public function testBranchPruningFormulaParsingMultipleIfsCase() + { + $calculation = Calculation::getInstance(); $calculation->flushInstance(); // resets the ids // @@ -189,14 +194,15 @@ public function testBranchPruningFormulaParsing() $isFunction = $token['type'] == 'Function'; $isProductFunction = $token['value'] == 'PRODUCT('; $correctOnlyIf = (isset($token['onlyIf']) ? $token['onlyIf'] : '') == 'storeKey-1'; - $productFunctionCorrectlyTagged = $productFunctionCorrectlyTagged || - ($isFunction && $isProductFunction && $correctOnlyIf); + $productFunctionCorrectlyTagged = $productFunctionCorrectlyTagged || ($isFunction && $isProductFunction && $correctOnlyIf); } - $this->assertFalse($plusGotTagged, - 'chaining IF( should not affect the external operators'); - $this->assertTrue($productFunctionCorrectlyTagged, - 'function nested inside if should be tagged to be processed only if parent branching requires it'); + $this->assertFalse($plusGotTagged, 'chaining IF( should not affect the external operators'); + $this->assertTrue($productFunctionCorrectlyTagged, 'function nested inside if should be tagged to be processed only if parent branching requires it'); + } + public function testBranchPruningFormulaParingNestedIfCase() + { + $calculation = Calculation::getInstance(); $calculation->flushInstance(); // resets the ids $formula = '=IF(A1="please +",SUM(B1:B3),1+IF(NOT(A2="please *"),C2-C1,PRODUCT(C1:C3)))'; @@ -216,26 +222,30 @@ public function testBranchPruningFormulaParsing() $isStoreKeyDepth1 = (array_key_exists('storeKey', $token) ? $token['storeKey'] : null) == 'storeKey-1'; $isOnlyIfNotDepth0 = (array_key_exists('onlyIfNot', $token) ? $token['onlyIfNot'] : null) == 'storeKey-0'; - $plusCorrectlyTagged = $plusCorrectlyTagged || - ($isPlus && $isOnlyIfNotDepth0); - $notFunctionCorrectlyTagged = $notFunctionCorrectlyTagged || - ($isNotFunction && $isOnlyIfNotDepth0 && $isStoreKeyDepth1); - $productFunctionCorrectlyTagged = $productFunctionCorrectlyTagged || - ($isProductFunction && $isOnlyIfNotDepth1 && !$isStoreKeyDepth1 && !$isOnlyIfNotDepth0); - $findOneOperandCountTagged = $findOneOperandCountTagged || - ($isIfOperand && $isOnlyIfNotDepth0); + $plusCorrectlyTagged = $plusCorrectlyTagged || ($isPlus && $isOnlyIfNotDepth0); + $notFunctionCorrectlyTagged = $notFunctionCorrectlyTagged || ($isNotFunction && $isOnlyIfNotDepth0 && $isStoreKeyDepth1); + $productFunctionCorrectlyTagged = $productFunctionCorrectlyTagged || ($isProductFunction && $isOnlyIfNotDepth1 && !$isStoreKeyDepth1 && !$isOnlyIfNotDepth0); + $findOneOperandCountTagged = $findOneOperandCountTagged || ($isIfOperand && $isOnlyIfNotDepth0); } $this->assertTrue($plusCorrectlyTagged); $this->assertTrue($productFunctionCorrectlyTagged); $this->assertTrue($notFunctionCorrectlyTagged); + } + public function testBranchPruningFormulaParsingNoArgumentFunctionCase() + { + $calculation = Calculation::getInstance(); $calculation->flushInstance(); // resets the ids $formula = '=IF(AND(TRUE(),A1="please +"),2,3)'; // this used to raise a parser error, we keep it even though we don't // test the output - $tokens = $calculation->parseFormula($formula); + $calculation->parseFormula($formula); + } + public function testBranchPruningFormulaParsingInequalitiesConditionsCase() + { + $calculation = Calculation::getInstance(); $calculation->flushInstance(); // resets the ids $formula = '=IF(A1="flag",IF(A2<10, 0) + IF(A3<10000, 0))'; @@ -270,16 +280,14 @@ public function testFullExecution( $formula, $cellCoordinates, $shouldBeSetInCacheCells = [], - $shouldNotBeSetInCacheCells = []) - { + $shouldNotBeSetInCacheCells = [] + ) { $spreadsheet = new Spreadsheet(); $sheet = $spreadsheet->getActiveSheet(); $sheet->fromArray($dataArray); $cell = $sheet->getCell($cellCoordinates); - $calculation = Calculation::getInstance( - $cell->getWorksheet()->getParent() - ); + $calculation = Calculation::getInstance($cell->getWorksheet()->getParent()); $cell->setValue($formula); $calculated = $cell->getCalculatedValue(); @@ -294,8 +302,7 @@ public function testFullExecution( foreach ($shouldNotBeSetInCacheCells as $notSetCell) { unset($inCache); - $calculation->getValueFromCache('Worksheet!' . $notSetCell, - $inCache); + $calculation->getValueFromCache('Worksheet!' . $notSetCell, $inCache); $this->assertEmpty($inCache); } diff --git a/tests/data/Calculation/Calculation.php b/tests/data/Calculation/Calculation.php index d54729f0e5..09538c3cd9 100644 --- a/tests/data/Calculation/Calculation.php +++ b/tests/data/Calculation/Calculation.php @@ -1,6 +1,7 @@