Skip to content

Commit 7758d91

Browse files
oleibmanoleibman
authored
Merge pull request PHPOffice#4657 from oleibman/issue4656
Handling Unions as Function Arguments
2 parents 293de4d + 51d72ad commit 7758d91

File tree

9 files changed

+322
-21
lines changed

9 files changed

+322
-21
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). Thia is a
1111

1212
- Add checkbox style (Xlsx and Html). [PR #4781](https://github.com/PHPOffice/PhpSpreadsheet/pull/4781)
1313
- Writer/Html add ability to set line ending. [PR #4779](https://github.com/PHPOffice/PhpSpreadsheet/pull/4779)
14+
- Writer/Html optionally save formulas as data attributes. [PR #4783](https://github.com/PHPOffice/PhpSpreadsheet/pull/4783)
1415

1516
### Removed
1617

@@ -27,10 +28,12 @@ and this project adheres to [Semantic Versioning](https://semver.org). Thia is a
2728
### Deprecated
2829

2930
- Writer/Html constant BODY_LINE no longer makes sense with a configurable line ending. No replacement.
31+
- Calculation classes FormulaParser and FormulaToken are unused. No replacement.
3032

3133
### Fixed
3234

3335
- Improve performance in value binders. [PR #4780](https://github.com/PHPOffice/PhpSpreadsheet/pull/4780)
36+
- Handle Unions as Function Arguments. [Issue #4656](https://github.com/PHPOffice/PhpSpreadsheet/issues/4656) [Issue #316](https://github.com/PHPOffice/PhpSpreadsheet/issues/316) [Issue #503](https://github.com/PHPOffice/PhpSpreadsheet/issues/503) [PR #4657](https://github.com/PHPOffice/PhpSpreadsheet/pull/4657)
3437

3538
## 2026-01-10 - 5.4.0
3639

‎phpstan.neon.dist‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@ parameters:
1515
- infra/
1616
- bin/
1717
excludePaths:
18+
- src/PhpSpreadsheet/Calculation/FormulaParser.php
19+
- src/PhpSpreadsheet/Calculation/FormulaToken.php
1820
- src/PhpSpreadsheet/Chart/Renderer/JpGraph.php
1921
- src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php
2022
- src/PhpSpreadsheet/Collection/Memory/SimpleCache1.php
2123
- src/PhpSpreadsheet/Collection/Memory/SimpleCache3.php
2224
- src/PhpSpreadsheet/Writer/ZipStream2.php
2325
- src/PhpSpreadsheet/Writer/ZipStream3.php
26+
- tests/PhpSpreadsheetTests/Calculation/FormulaParserTest.php
2427
- tests/PhpSpreadsheetTests/Writer/Xlsx/ArrayFunctions2Test.php
2528
parallel:
2629
processTimeout: 300.0

‎src/PhpSpreadsheet/Calculation/Calculation.php‎

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Calculation;
44

5+
use Composer\Pcre\Preg; // many pregs in this program use u modifier, which has side-effects which make it unsuitable for this
56
use PhpOffice\PhpSpreadsheet\Calculation\Engine\BranchPruner;
67
use PhpOffice\PhpSpreadsheet\Calculation\Engine\CyclicReferenceStack;
78
use PhpOffice\PhpSpreadsheet\Calculation\Engine\Logger;
@@ -36,7 +37,7 @@ class Calculation extends CalculationLocale
3637
const CALCULATION_REGEXP_OPENBRACE = '\(';
3738
// Function (allow for the old @ symbol that could be used to prefix a function, but we'll ignore it)
3839
const CALCULATION_REGEXP_FUNCTION = '@?(?:_xlfn\.)?(?:_xlws\.)?((?:__xludf\.)?[\p{L}][\p{L}\p{N}\.]*)[\s]*\(';
39-
// Cell reference (cell or range of cells, with or without a sheet reference)
40+
// Cell reference, with or without a sheet reference)
4041
const CALCULATION_REGEXP_CELLREF = '((([^\s,!&%^\/\*\+<>=:`-]*)|(\'(?:[^\']|\'[^!])+?\')|(\"(?:[^\"]|\"[^!])+?\"))!)?\$?\b([a-z]{1,3})\$?(\d{1,7})(?![\w.])';
4142
// Used only to detect spill operator #
4243
const CALCULATION_REGEXP_CELLREF_SPILL = '/' . self::CALCULATION_REGEXP_CELLREF . '#/i';
@@ -415,7 +416,7 @@ public static function wrapResult(mixed $value): mixed
415416
{
416417
if (is_string($value)) {
417418
// Error values cannot be "wrapped"
418-
if (preg_match('/^' . self::CALCULATION_REGEXP_ERROR . '$/i', $value, $match)) {
419+
if (Preg::isMatch('/^' . self::CALCULATION_REGEXP_ERROR . '$/i', $value, $match)) {
419420
// Return Excel errors "as is"
420421
return $value;
421422
}
@@ -494,7 +495,7 @@ public function calculateCellValue(?Cell $cell = null, bool $resetLog = true): m
494495
try {
495496
$value = $cell->getValue();
496497
if (is_string($value) && $cell->getDataType() === DataType::TYPE_FORMULA) {
497-
$value = preg_replace_callback(
498+
$value = Preg::replaceCallback(
498499
self::CALCULATION_REGEXP_CELLREF_SPILL,
499500
fn (array $matches) => 'ANCHORARRAY(' . substr($matches[0], 0, -1) . ')',
500501
$value
@@ -557,11 +558,11 @@ public function calculateCellValue(?Cell $cell = null, bool $resetLog = true): m
557558
*/
558559
public function parseFormula(string $formula): array|bool
559560
{
560-
$formula = preg_replace_callback(
561+
$formula = Preg::replaceCallback(
561562
self::CALCULATION_REGEXP_CELLREF_SPILL,
562563
fn (array $matches) => 'ANCHORARRAY(' . substr($matches[0], 0, -1) . ')',
563564
$formula
564-
) ?? $formula;
565+
);
565566
// Basic validation that this is indeed a formula
566567
// We return an empty array if not
567568
$formula = trim($formula);
@@ -657,8 +658,9 @@ public function _calculateFormulaValue(string $formula, ?string $cellID = null,
657658
return self::wrapResult((string) $formula);
658659
}
659660

661+
// https://www.reddit.com/r/excel/comments/chr41y/cmd_formula_stopped_working_since_last_update/
660662
if (preg_match('/^=\s*cmd\s*\|/miu', $formula) !== 0) {
661-
return self::wrapResult($formula);
663+
return ExcelError::REF(); // returns #BLOCKED in newer versions
662664
}
663665

664666
// Basic validation that this is indeed a formula
@@ -1052,6 +1054,28 @@ private function convertMatrixReferences(string $formula): false|string
10521054
'>' => 0, '<' => 0, '=' => 0, '>=' => 0, '<=' => 0, '<>' => 0, // Comparison
10531055
];
10541056

1057+
/** @param string[] $matches */
1058+
private static function unionForComma(array $matches): string
1059+
{
1060+
return $matches[1] . str_replace(',', '∪', $matches[2]);
1061+
}
1062+
1063+
private const CELL_OR_CELLRANGE_OR_DEFINED_NAME
1064+
= '(?:'
1065+
. self::CALCULATION_REGEXP_CELLREF // cell address
1066+
. '(?::' . self::CALCULATION_REGEXP_CELLREF . ')?' // optional range address, non-capturing
1067+
. '|' . self::CALCULATION_REGEXP_DEFINEDNAME
1068+
. ')'
1069+
;
1070+
1071+
public const UNIONABLE_COMMAS = '/((?:[,(]|^)\s*)' // comma or open paren or start of string, followed by optional whitespace
1072+
. '([(]' // open paren
1073+
. self::CELL_OR_CELLRANGE_OR_DEFINED_NAME // cell address
1074+
. '(?:\s*,\s*' // optioonal whitespace, comma, optional whitespace, non-capturing
1075+
. self::CELL_OR_CELLRANGE_OR_DEFINED_NAME // cell address
1076+
. ')+' // one or more occurrences
1077+
. '\s*[)])/i'; // optional whitespace, end paren
1078+
10551079
/**
10561080
* @return array<int, mixed>|false
10571081
*/
@@ -1060,6 +1084,12 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
10601084
if (($formula = $this->convertMatrixReferences(trim($formula))) === false) {
10611085
return false;
10621086
}
1087+
1088+
$oldFormula = $formula;
1089+
$formula = Preg::replaceCallback(self::UNIONABLE_COMMAS, self::unionForComma(...), $formula); // @phpstan-ignore-line
1090+
if ($oldFormula !== $formula) {
1091+
$this->debugLog->writeDebugLog('Reformulated as %s', $formula);
1092+
}
10631093
$phpSpreadsheetFunctions = &self::getFunctionsAddress();
10641094

10651095
// If we're using cell caching, then $pCell may well be flushed back to the cache (which detaches the parent worksheet),
@@ -1179,7 +1209,7 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
11791209
}
11801210
}
11811211
} elseif (is_string($expectedArgumentCount) && $expectedArgumentCount !== '*') {
1182-
if (1 !== preg_match('/(\d*)([-+,])(\d*)/', $expectedArgumentCount, $argMatch)) {
1212+
if (!Preg::isMatch('/(\d*)([-+,])(\d*)/', $expectedArgumentCount, $argMatch)) {
11831213
$argMatch = ['', '', '', ''];
11841214
}
11851215
switch ($argMatch[2]) {
@@ -1236,16 +1266,14 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
12361266
// because at least the braces are paired up (at this stage in the formula)
12371267
// MS Excel allows this if the content is cell references; but doesn't allow actual values,
12381268
// but at this point, we can't differentiate (so allow both)
1239-
return $this->raiseFormulaError('Formula Error: Unexpected ,');
1240-
/* The following code may be a better choice, but, with
1241-
the other changes for this PR, I can no longer come up
1242-
with a test case that gets here
1269+
//return $this->raiseFormulaError('Formula Error: Unexpected ,');
1270+
12431271
$stack->push('Binary Operator', '∪');
12441272

12451273
++$index;
12461274
$expectingOperator = false;
12471275

1248-
continue;*/
1276+
continue;
12491277
}
12501278

12511279
/** @var array<string, int> $d */
@@ -1271,7 +1299,8 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
12711299
$length = strlen($val);
12721300

12731301
if (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/miu', $val, $matches)) {
1274-
$val = (string) preg_replace('/\s/u', '', $val);
1302+
// $val is known to be valid unicode from statement above, so Preg::replace is okay even with u modifier
1303+
$val = Preg::replace('/\s/u', '', $val);
12751304
if (isset($phpSpreadsheetFunctions[strtoupper($matches[1])]) || isset(self::$controlFunctions[strtoupper($matches[1])])) { // it's a function
12761305
$valToUpper = strtoupper($val);
12771306
} else {
@@ -1988,20 +2017,27 @@ private function processTokenStack(false|array $tokens, ?string $cellID = null,
19882017
} else {
19892018
$this->executeNumericBinaryOperation($multiplier, $arg, '*', $stack);
19902019
}
1991-
} elseif (preg_match('/^' . self::CALCULATION_REGEXP_CELLREF . '$/i', StringHelper::convertToString($token ?? ''), $matches)) {
2020+
} elseif (Preg::isMatch('/^' . self::CALCULATION_REGEXP_CELLREF . '$/i', StringHelper::convertToString($token ?? ''), $matches)) {
19922021
$cellRef = null;
19932022

19942023
/* Phpstan says matches[8/9/10] is never set,
19952024
and code coverage report seems to confirm.
1996-
Appease PhpStan for now;
1997-
probably delete this block later.
2025+
regex101.com confirms - only 7 capturing groups.
2026+
My theory is that this code expected regexp to
2027+
match cell *or* cellRange, but it does not
2028+
match the latter. Retain the code for now in case
2029+
we do want to add the range match later.
2030+
Probably delete this block later.
2031+
Until delete happens, turn code coverage off.
19982032
*/
19992033
if (isset($matches[self::$matchIndex8])) {
2034+
// @codeCoverageIgnoreStart
20002035
if ($cell === null) {
20012036
// We can't access the range, so return a REF error
20022037
$cellValue = ExcelError::REF();
20032038
} else {
20042039
$cellRef = $matches[6] . $matches[7] . ':' . $matches[self::$matchIndex9] . $matches[self::$matchIndex10];
2040+
$matches[2] = (string) $matches[2];
20052041
if ($matches[2] > '') {
20062042
$matches[2] = trim($matches[2], "\"'");
20072043
if ((str_contains($matches[2], '[')) || (str_contains($matches[2], ']'))) {
@@ -2026,12 +2062,14 @@ private function processTokenStack(false|array $tokens, ?string $cellID = null,
20262062
$this->debugLog->writeDebugLog('Evaluation Result for cells %s is %s', $cellRef, $this->showTypeDetails($cellValue));
20272063
}
20282064
}
2065+
// @codeCoverageIgnoreEnd
20292066
} else {
20302067
if ($cell === null) {
20312068
// We can't access the cell, so return a REF error
20322069
$cellValue = ExcelError::REF();
20332070
} else {
20342071
$cellRef = $matches[6] . $matches[7];
2072+
$matches[2] = (string) $matches[2];
20352073
if ($matches[2] > '') {
20362074
$matches[2] = trim($matches[2], "\"'");
20372075
if ((str_contains($matches[2], '[')) || (str_contains($matches[2], ']'))) {
@@ -2073,7 +2111,7 @@ private function processTokenStack(false|array $tokens, ?string $cellID = null,
20732111
$cellValue = array_shift($cellValue);
20742112
}
20752113
if (is_string($cellValue)) {
2076-
$cellValue = preg_replace('/"/', '""', $cellValue);
2114+
$cellValue = Preg::replace('/"/', '""', $cellValue);
20772115
}
20782116
$this->debugLog->writeDebugLog('Scalar Result for cell %s is %s', $cellRef, $this->showTypeDetails($cellValue));
20792117
}
@@ -2819,10 +2857,12 @@ private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksh
28192857
$definedNameValue = $namedRange->getValue();
28202858
$definedNameType = $namedRange->isFormula() ? 'Formula' : 'Range';
28212859
if ($definedNameType === 'Range') {
2822-
if (preg_match('/^(.*!)?(.*)$/', $definedNameValue, $matches) === 1) {
2823-
$matches2 = trim($matches[2]);
2824-
$matches2 = preg_replace('/ +/', ' ∩ ', $matches2) ?? $matches2;
2825-
$matches2 = preg_replace('/,/', ' ∪ ', $matches2) ?? $matches2;
2860+
if (Preg::isMatch('/^(.*!)?(.*)$/', $definedNameValue, $matches)) {
2861+
$matches2 = Preg::replace(
2862+
['/ +/', '/,/'],
2863+
[' ∩ ', ' ∪ '],
2864+
trim($matches[2])
2865+
);
28262866
$definedNameValue = $matches[1] . $matches2;
28272867
}
28282868
}

‎src/PhpSpreadsheet/Calculation/FormulaParser.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@
2121
* whether in an action of contract, tort or otherwise, arising from, out of or in connection with the
2222
* software or the use or other dealings in the software.
2323
*
24+
* The following links are no longer valid.
2425
* https://ewbi.blogs.com/develops/2007/03/excel_formula_p.html
2526
* https://ewbi.blogs.com/develops/2004/12/excel_formula_p.html
27+
*
28+
* @deprecated 5.5.0 No replacement.
2629
*/
2730
class FormulaParser
2831
{

‎src/PhpSpreadsheet/Calculation/FormulaToken.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@
2121
* whether in an action of contract, tort or otherwise, arising from, out of or in connection with the
2222
* software or the use or other dealings in the software.
2323
*
24+
* The following links are no longer valid.
2425
* https://ewbi.blogs.com/develops/2007/03/excel_formula_p.html
2526
* https://ewbi.blogs.com/develops/2004/12/excel_formula_p.html
27+
*
28+
* @deprecated 5.5.0 No replacement.
2629
*/
2730
class FormulaToken
2831
{

‎tests/PhpSpreadsheetTests/Calculation/CalculationCoverageTest.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
88
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException;
99
use PhpOffice\PhpSpreadsheet\Calculation\ExceptionHandler;
10+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
1011
use PhpOffice\PhpSpreadsheet\NamedRange;
1112
use PhpOffice\PhpSpreadsheet\Spreadsheet;
1213
use PHPUnit\Framework\TestCase;
@@ -123,6 +124,26 @@ public function testExtractNamedRange(): void
123124
$spreadsheet->disconnectWorksheets();
124125
}
125126

127+
public function testBlockCmd(): void
128+
{
129+
$spreadsheet = new Spreadsheet();
130+
$sheet = $spreadsheet->getActiveSheet();
131+
$formula = "=cmd|'/C calc'!A0";
132+
$sheet->setCellValueExplicit('A1', $formula, DataType::TYPE_FORMULA);
133+
self::assertSame(
134+
'#REF!',
135+
$sheet->getCell('A1')->getCalculatedValue(),
136+
'cmd, blocked by default in Excel, forced error result in Calculation Engine'
137+
);
138+
$sheet->setCellValue('A2', $formula);
139+
self::assertSame(
140+
$formula,
141+
$sheet->getCell('A2')->getCalculatedValue(),
142+
'cmd, blocked by default in Excel, treated as string not formula by DefaultValueBinder'
143+
);
144+
$spreadsheet->disconnectWorksheets();
145+
}
146+
126147
protected static int $winMinPhpToSkip = 80300;
127148

128149
protected static int $winMaxPhpToSkip = 80499;

‎tests/PhpSpreadsheetTests/Calculation/Discussion1950Test.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
6+
7+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
8+
use PHPUnit\Framework\TestCase;
9+
10+
class Discussion1950Test extends TestCase
11+
{
12+
public function testMultipleUnions(): void
13+
{
14+
$spreadsheet = new Spreadsheet();
15+
$sheet = $spreadsheet->getActiveSheet();
16+
$sheet->fromArray([
17+
[1, 2],
18+
[3, 4],
19+
]);
20+
$sheet->setCellValue('A5', '=SUM((A1,A2),(B1,B2))');
21+
self::assertSame(10, $sheet->getCell('A5')->getCalculatedValue());
22+
$spreadsheet->disconnectWorksheets();
23+
}
24+
25+
public function testUnexpectedUnion(): void
26+
{
27+
$spreadsheet = new Spreadsheet();
28+
$sheet = $spreadsheet->getActiveSheet();
29+
$sheet->fromArray([
30+
[1],
31+
[-1],
32+
[2],
33+
[1],
34+
['=RANK(A1,(A2,A3,A4))'],
35+
]);
36+
self::assertSame(2, $sheet->getCell('A5')->getCalculatedValue());
37+
$spreadsheet->disconnectWorksheets();
38+
}
39+
}

0 commit comments

Comments
 (0)