Skip to content

Commit 99f488e

Browse files
author
MarkBaker
committed
Resolve translateSeparator() method to handle separators (row and column) for array functions as well as for function argument separators; and cleanly handle nesting levels
Note that this method is used when translating Excel functions between en and other locale languages, as well as when converting formulae between different spreadsheet formats (e.g. Ods to Excel) Nor is this a perfect solution, as there may still be issues when function calls have array arguments that themselves contain function calls; but it's still better than the current logic
1 parent e6047cf commit 99f488e

File tree

3 files changed

+65
-44
lines changed

3 files changed

+65
-44
lines changed

‎phpstan-baseline.neon

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ parameters:
162162

163163
-
164164
message: "#^Parameter \\#3 \\$formula of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:translateSeparator\\(\\) expects string, string\\|null given\\.$#"
165-
count: 2
165+
count: 1
166166
path: src/PhpSpreadsheet/Calculation/Calculation.php
167167

168168
-

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 59 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ class Calculation
5050
const RETURN_ARRAY_AS_VALUE = 'value';
5151
const RETURN_ARRAY_AS_ARRAY = 'array';
5252

53-
const FORMULA_OPEN_FUNCTION_BRACE = '{';
54-
const FORMULA_CLOSE_FUNCTION_BRACE = '}';
53+
const FORMULA_OPEN_FUNCTION_BRACE = '(';
54+
const FORMULA_CLOSE_FUNCTION_BRACE = ')';
55+
const FORMULA_OPEN_MATRIX_BRACE = '{';
56+
const FORMULA_CLOSE_MATRIX_BRACE = '}';
5557
const FORMULA_STRING_QUOTE = '"';
5658

5759
private static $returnArrayAsType = self::RETURN_ARRAY_AS_VALUE;
@@ -3084,30 +3086,28 @@ public function setLocale(string $locale)
30843086
return false;
30853087
}
30863088

3087-
/**
3088-
* @param string $fromSeparator
3089-
* @param string $toSeparator
3090-
* @param string $formula
3091-
* @param bool $inBraces
3092-
*
3093-
* @return string
3094-
*/
3095-
public static function translateSeparator($fromSeparator, $toSeparator, $formula, &$inBraces)
3096-
{
3089+
public static function translateSeparator(
3090+
string $fromSeparator,
3091+
string $toSeparator,
3092+
string $formula,
3093+
int &$inBracesLevel,
3094+
string $openBrace = self::FORMULA_OPEN_FUNCTION_BRACE,
3095+
string $closeBrace = self::FORMULA_CLOSE_FUNCTION_BRACE
3096+
): string {
30973097
$strlen = mb_strlen($formula);
30983098
for ($i = 0; $i < $strlen; ++$i) {
30993099
$chr = mb_substr($formula, $i, 1);
31003100
switch ($chr) {
3101-
case self::FORMULA_OPEN_FUNCTION_BRACE:
3102-
$inBraces = true;
3101+
case $openBrace:
3102+
++$inBracesLevel;
31033103

31043104
break;
3105-
case self::FORMULA_CLOSE_FUNCTION_BRACE:
3106-
$inBraces = false;
3105+
case $closeBrace:
3106+
--$inBracesLevel;
31073107

31083108
break;
31093109
case $fromSeparator:
3110-
if (!$inBraces) {
3110+
if ($inBracesLevel > 0) {
31113111
$formula = mb_substr($formula, 0, $i) . $toSeparator . mb_substr($formula, $i + 1);
31123112
}
31133113
}
@@ -3116,40 +3116,55 @@ public static function translateSeparator($fromSeparator, $toSeparator, $formula
31163116
return $formula;
31173117
}
31183118

3119-
/**
3120-
* @param string[] $from
3121-
* @param string[] $to
3122-
* @param string $formula
3123-
* @param string $fromSeparator
3124-
* @param string $toSeparator
3125-
*
3126-
* @return string
3127-
*/
3128-
private static function translateFormula(array $from, array $to, $formula, $fromSeparator, $toSeparator)
3119+
private static function translateFormulaBlock(
3120+
array $from,
3121+
array $to,
3122+
string $formula,
3123+
int &$inFunctionBracesLevel,
3124+
int &$inMatrixBracesLevel,
3125+
string $fromSeparator,
3126+
string $toSeparator
3127+
): string {
3128+
// Function Names
3129+
$formula = preg_replace($from, $to, $formula);
3130+
3131+
// Temporarily adjust matrix separators so that they won't be confused with function arguments
3132+
$formula = self::translateSeparator(';', '|', $formula, $inMatrixBracesLevel, self::FORMULA_OPEN_MATRIX_BRACE, self::FORMULA_CLOSE_MATRIX_BRACE);
3133+
$formula = self::translateSeparator(',', '!', $formula, $inMatrixBracesLevel, self::FORMULA_OPEN_MATRIX_BRACE, self::FORMULA_CLOSE_MATRIX_BRACE);
3134+
// Function Argument Separators
3135+
$formula = self::translateSeparator($fromSeparator, $toSeparator, $formula, $inFunctionBracesLevel);
3136+
// Restore matrix separators
3137+
$formula = self::translateSeparator('|', ';', $formula, $inMatrixBracesLevel, self::FORMULA_OPEN_MATRIX_BRACE, self::FORMULA_CLOSE_MATRIX_BRACE);
3138+
$formula = self::translateSeparator('!', ',', $formula, $inMatrixBracesLevel, self::FORMULA_OPEN_MATRIX_BRACE, self::FORMULA_CLOSE_MATRIX_BRACE);
3139+
3140+
return $formula;
3141+
}
3142+
3143+
private static function translateFormula(array $from, array $to, string $formula, string $fromSeparator, string $toSeparator): string
31293144
{
3130-
// Convert any Excel function names to the required language
3145+
// Convert any Excel function names and constant names to the required language;
3146+
// and adjust function argument separators
31313147
if (self::$localeLanguage !== 'en_us') {
3132-
$inBraces = false;
3133-
// If there is the possibility of braces within a quoted string, then we don't treat those as matrix indicators
3148+
$inFunctionBracesLevel = 0;
3149+
$inMatrixBracesLevel = 0;
3150+
// If there is the possibility of separators within a quoted string, then we treat them as literals
31343151
if (strpos($formula, self::FORMULA_STRING_QUOTE) !== false) {
3135-
// So instead we skip replacing in any quoted strings by only replacing in every other array element after we've exploded
3136-
// the formula
3152+
// So instead we skip replacing in any quoted strings by only replacing in every other array element
3153+
// after we've exploded the formula
31373154
$temp = explode(self::FORMULA_STRING_QUOTE, $formula);
31383155
$i = false;
31393156
foreach ($temp as &$value) {
3140-
// Only count/replace in alternating array entries
3157+
// Only adjust in alternating array entries
31413158
if ($i = !$i) {
3142-
$value = preg_replace($from, $to, $value);
3143-
$value = self::translateSeparator($fromSeparator, $toSeparator, $value, $inBraces);
3159+
$value = self::translateFormulaBlock($from, $to, $value, $inFunctionBracesLevel, $inMatrixBracesLevel, $fromSeparator, $toSeparator);
31443160
}
31453161
}
31463162
unset($value);
31473163
// Then rebuild the formula string
31483164
$formula = implode(self::FORMULA_STRING_QUOTE, $temp);
31493165
} else {
31503166
// If there's no quoted strings, then we do a simple count/replace
3151-
$formula = preg_replace($from, $to, $formula);
3152-
$formula = self::translateSeparator($fromSeparator, $toSeparator, $formula, $inBraces);
3167+
$formula = self::translateFormulaBlock($from, $to, $formula, $inFunctionBracesLevel, $inMatrixBracesLevel, $fromSeparator, $toSeparator);
31533168
}
31543169
}
31553170

@@ -3162,6 +3177,7 @@ private static function translateFormula(array $from, array $to, $formula, $from
31623177

31633178
public function _translateFormulaToLocale($formula)
31643179
{
3180+
// Build list of function names and constants for translation
31653181
if (self::$functionReplaceFromExcel === null) {
31663182
self::$functionReplaceFromExcel = [];
31673183
foreach (array_keys(self::$localeFunctions) as $excelFunctionName) {
@@ -3798,11 +3814,11 @@ private function showTypeDetails($value)
37983814
*/
37993815
private function convertMatrixReferences($formula)
38003816
{
3801-
static $matrixReplaceFrom = [self::FORMULA_OPEN_FUNCTION_BRACE, ';', self::FORMULA_CLOSE_FUNCTION_BRACE];
3817+
static $matrixReplaceFrom = [self::FORMULA_OPEN_MATRIX_BRACE, ';', self::FORMULA_CLOSE_MATRIX_BRACE];
38023818
static $matrixReplaceTo = ['MKMATRIX(MKMATRIX(', '),MKMATRIX(', '))'];
38033819

38043820
// Convert any Excel matrix references to the MKMATRIX() function
3805-
if (strpos($formula, self::FORMULA_OPEN_FUNCTION_BRACE) !== false) {
3821+
if (strpos($formula, self::FORMULA_OPEN_MATRIX_BRACE) !== false) {
38063822
// If there is the possibility of braces within a quoted string, then we don't treat those as matrix indicators
38073823
if (strpos($formula, self::FORMULA_STRING_QUOTE) !== false) {
38083824
// So instead we skip replacing in any quoted strings by only replacing in every other array element after we've exploded
@@ -3814,8 +3830,8 @@ private function convertMatrixReferences($formula)
38143830
foreach ($temp as &$value) {
38153831
// Only count/replace in alternating array entries
38163832
if ($i = !$i) {
3817-
$openCount += substr_count($value, self::FORMULA_OPEN_FUNCTION_BRACE);
3818-
$closeCount += substr_count($value, self::FORMULA_CLOSE_FUNCTION_BRACE);
3833+
$openCount += substr_count($value, self::FORMULA_OPEN_MATRIX_BRACE);
3834+
$closeCount += substr_count($value, self::FORMULA_CLOSE_MATRIX_BRACE);
38193835
$value = str_replace($matrixReplaceFrom, $matrixReplaceTo, $value);
38203836
}
38213837
}
@@ -3824,8 +3840,8 @@ private function convertMatrixReferences($formula)
38243840
$formula = implode(self::FORMULA_STRING_QUOTE, $temp);
38253841
} else {
38263842
// If there's no quoted strings, then we do a simple count/replace
3827-
$openCount = substr_count($formula, self::FORMULA_OPEN_FUNCTION_BRACE);
3828-
$closeCount = substr_count($formula, self::FORMULA_CLOSE_FUNCTION_BRACE);
3843+
$openCount = substr_count($formula, self::FORMULA_OPEN_MATRIX_BRACE);
3844+
$closeCount = substr_count($formula, self::FORMULA_CLOSE_MATRIX_BRACE);
38293845
$formula = str_replace($matrixReplaceFrom, $matrixReplaceTo, $formula);
38303846
}
38313847
// Trap for mismatched braces and trigger an appropriate error

‎tests/data/Calculation/Translations.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,9 @@
7575
'tr',
7676
'=WORKDAY.INTL(B1)',
7777
],
78+
[
79+
'=STØRST(ABS({2,-3;-4,5}); ABS{-2,3;4,-5})',
80+
'nb',
81+
'=MAX(ABS({2,-3;-4,5}), ABS{-2,3;4,-5})',
82+
],
7883
];

0 commit comments

Comments
 (0)