Skip to content

Commit

Permalink
Save Excel 2010+ Functions Properly
Browse files Browse the repository at this point in the history
For functions introduced in Excel 2010 and beyond, Excel saves them
in formulas with the xlfn_ prefix. PhpSpreadsheet does not do this;
as a result, when a spreadsheet so created is opened, the cells
which use the new functions display a #NAME? error.
This the cause of bug report 1246:
#1246
This change corrects that problem when the Xlsx writer encounters
a 2010+ formula for a cell or a conditional style. A new class
Writer/Xlsx/Xlfn, with 2 static methods,
is introduced to facilitate this change.

As part of the testing for this, I found some additional problems.
When an unknown function name is used, Excel generates a #NAME? error.
However, when an unknown function is used in PhpSpreadsheet:
  - if there are no parameters, it returns #VALUE!, which is wrong
  - if there are parameters, it throws an exception, which is horrible
Both of these situations will now return #NAME?
Tests have been added for these situations.

The MODE (and MODE.SNGL) function is not quite in alignment with Excel.
MODE(3, 3, 4, 4) returns 3 in both Excel and PhpSpreadsheet.
However, MODE(4, 3, 3, 4) returns 4 in Excel, but 3 in PhpSpreadsheet.
Both situations will now match Excel's result.
Also, Excel allows its parameters for MODE to be an array,
but PhpSpreadsheet did not; it now will.
There had not been any tests for MODE. Now there are.

The SHEET and SHEETS functions were introduced in Excel 2013,
but were not introduced in PhpSpreadsheet. They are now introduced
as DUMMY functions so that they can be parsed appropriately.

Finally, in common with the "rate" changes for which I am
creating a pull request at the same time as this one:
samples/Basic/13_CalculationCyclicFormulae
PhpUnit started reporting an error like "too much regression".
The test deals with an infinite cyclic formula, and allowed
the calculation engine to run for 100 cycles. The actual number of cycles
seems irrelevant for the purpose of this test. I changed it to 15,
and PhpUnit no longer complains.
  • Loading branch information
oleibman authored and PowerKiKi committed May 18, 2020
1 parent 414e569 commit 4f6d4af
Show file tree
Hide file tree
Showing 11 changed files with 665 additions and 155 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
- Fix Chart samples by updating chart parameter from 0 to DataSeries::EMPTY_AS_GAP [#1448](https://github.com/PHPOffice/PhpSpreadsheet/pull/1448)
- Fix return type in docblock for the Cells::get() [#1398](https://github.com/PHPOffice/PhpSpreadsheet/pull/1398)
- Fix RATE, PRICE, XIRR, and XNPV Functions [#1456](https://github.com/PHPOffice/PhpSpreadsheet/pull/1456)
- Save Excel 2010+ functions properly in XLSX [#1461](https://github.com/PHPOffice/PhpSpreadsheet/pull/1461)

### Changed

Expand Down
62 changes: 38 additions & 24 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -1853,6 +1853,16 @@ class Calculation
'functionCall' => [MathTrig::class, 'SERIESSUM'],
'argumentCount' => '4',
],
'SHEET' => [
'category' => Category::CATEGORY_INFORMATION,
'functionCall' => [Functions::class, 'DUMMY'],
'argumentCount' => '0,1',
],
'SHEETS' => [
'category' => Category::CATEGORY_INFORMATION,
'functionCall' => [Functions::class, 'DUMMY'],
'argumentCount' => '0,1',
],
'SIGN' => [
'category' => Category::CATEGORY_MATH_AND_TRIG,
'functionCall' => [MathTrig::class, 'SIGN'],
Expand Down Expand Up @@ -2247,6 +2257,10 @@ class Calculation
'argumentCount' => '*',
'functionCall' => [__CLASS__, 'mkMatrix'],
],
'NAME.ERROR' => [
'argumentCount' => '*',
'functionCall' => [Functions::class, 'NAME'],
],
];

public function __construct(Spreadsheet $spreadsheet = null)
Expand Down Expand Up @@ -3615,33 +3629,33 @@ private function _parseFormula($formula, Cell $pCell = null)
$val = preg_replace('/\s/u', '', $val);
if (isset(self::$phpSpreadsheetFunctions[strtoupper($matches[1])]) || isset(self::$controlFunctions[strtoupper($matches[1])])) { // it's a function
$valToUpper = strtoupper($val);
// 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 a if but we good deeper
if (!empty($pendingStoreKey) && array_key_exists($pendingStoreKey, $parenthesisDepthMap)) {
$parenthesisDepthMap[$pendingStoreKey] += 1;
}
} else {
$valToUpper = 'NAME.ERROR(';
}
// 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] += 1;
}
}

$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) {
$stack->push('Operand Count for Function ' . $valToUpper . ')', 0, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot);
$expectingOperator = true;
} else {
$stack->push('Operand Count for Function ' . $valToUpper . ')', 1, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot);
$expectingOperator = false;
}
$stack->push('Brace', '(');
} else { // it's a var w/ implicit multiplication
$output[] = ['type' => 'Value', 'value' => $matches[1], 'reference' => null];
$stack->push('Function', $valToUpper, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot);
// 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);
$expectingOperator = true;
} else {
$stack->push('Operand Count for Function ' . $valToUpper . ')', 1, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot);
$expectingOperator = false;
}
$stack->push('Brace', '(');
} elseif (preg_match('/^' . self::CALCULATION_REGEXP_CELLREF . '$/i', $val, $matches)) {
// Watch for this case-change when modifying to allow cell references in different worksheets...
// Should only be applied to the actual cell column, not the worksheet name
Expand Down
27 changes: 19 additions & 8 deletions src/PhpSpreadsheet/Calculation/Statistical.php
Original file line number Diff line number Diff line change
Expand Up @@ -2468,11 +2468,27 @@ public static function MINIFS(...$args)
private static function modeCalc($data)
{
$frequencyArray = [];
$index = 0;
$maxfreq = 0;
$maxfreqkey = '';
$maxfreqdatum = '';
foreach ($data as $datum) {
$found = false;
++$index;
foreach ($frequencyArray as $key => $value) {
if ((string) $value['value'] == (string) $datum) {
++$frequencyArray[$key]['frequency'];
$freq = $frequencyArray[$key]['frequency'];
if ($freq > $maxfreq) {
$maxfreq = $freq;
$maxfreqkey = $key;
$maxfreqdatum = $datum;
} elseif ($freq == $maxfreq) {
if ($frequencyArray[$key]['index'] < $frequencyArray[$maxfreqkey]['index']) {
$maxfreqkey = $key;
$maxfreqdatum = $datum;
}
}
$found = true;

break;
Expand All @@ -2482,21 +2498,16 @@ private static function modeCalc($data)
$frequencyArray[] = [
'value' => $datum,
'frequency' => 1,
'index' => $index,
];
}
}

foreach ($frequencyArray as $key => $value) {
$frequencyList[$key] = $value['frequency'];
$valueList[$key] = $value['value'];
}
array_multisort($frequencyList, SORT_DESC, $valueList, SORT_ASC, SORT_NUMERIC, $frequencyArray);

if ($frequencyArray[0]['frequency'] == 1) {
if ($maxfreq <= 1) {
return Functions::NA();
}

return $frequencyArray[0]['value'];
return $maxfreqdatum;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/PhpSpreadsheet/Calculation/functionlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ SEC
SECH
SECOND
SERIESSUM
SHEET
SHEETS
SIGN
SIN
SINH
Expand Down
Loading

0 comments on commit 4f6d4af

Please sign in to comment.