Skip to content

Commit

Permalink
Fix style errors and splitting tests
Browse files Browse the repository at this point in the history
  • Loading branch information
frantzmiccoli committed Jan 7, 2019
1 parent 1267900 commit b1ef7ef
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 39 deletions.
15 changes: 3 additions & 12 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -2256,7 +2256,6 @@ public function flushInstance()
{
$this->clearCalculationCache();
$this->clearBranchStore();
$this->branchStoreKeyCounter = 0;
}

/**
Expand Down Expand Up @@ -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(') {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/PhpSpreadsheet/Calculation/Token/Stack.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
53 changes: 30 additions & 23 deletions tests/PhpSpreadsheetTests/Calculation/CalculationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)';
Expand All @@ -166,7 +167,11 @@ public function testBranchPruningFormulaParsing()
}
$this->assertTrue($foundEqualAssociatedToStoreKey);
$this->assertTrue($foundConditionalOnB1);
}

public function testBranchPruningFormulaParsingMultipleIfsCase()
{
$calculation = Calculation::getInstance();
$calculation->flushInstance(); // resets the ids

//
Expand All @@ -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)))';
Expand All @@ -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))';
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}

Expand Down
5 changes: 3 additions & 2 deletions tests/data/Calculation/Calculation.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php

function calculationTestDataGenerator() {
function calculationTestDataGenerator()
{
$dataArray1 = [
['please +', 'please *', 'increment'],
[1, 1, 1], // sum is 3
Expand Down Expand Up @@ -59,4 +60,4 @@ function calculationTestDataGenerator() {
return [$set0, $set1, $set2, $set3, $set4, $set5, $set6, $set7, $set8];
};

return calculationTestDataGenerator();
return calculationTestDataGenerator();

0 comments on commit b1ef7ef

Please sign in to comment.