Skip to content

Commit 06eedfe

Browse files
MarkBakerMarkBaker
committed
Refactoring for checks on strings containing formatted numeric values when used in mathematical operations in the Calculation Engine
1 parent 6f71efc commit 06eedfe

File tree

7 files changed

+261
-211
lines changed

7 files changed

+261
-211
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5110,7 +5110,7 @@ private function validateBinaryOperand(&$operand, &$stack)
51105110
$this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($operand));
51115111

51125112
return false;
5113-
} elseif (!Shared\StringHelper::convertToNumberIfFraction($operand) && !Shared\StringHelper::convertToNumberIfPercent($operand)) {
5113+
} elseif (Engine\FormattedNumber::convertToNumberIfFormatted($operand) === false) {
51145114
// If not a numeric, a fraction or a percentage, then it's a text string, and so can't be used in mathematical binary operations
51155115
$stack->push('Error', '#VALUE!');
51165116
$this->debugLog->writeDebugLog('Evaluation Result is a %s', $this->showTypeDetails('#VALUE!'));
@@ -5120,7 +5120,7 @@ private function validateBinaryOperand(&$operand, &$stack)
51205120
}
51215121
}
51225122

5123-
// return a true if the value of the operand is one that we can use in normal binary operations
5123+
// return a true if the value of the operand is one that we can use in normal binary mathematical operations
51245124
return true;
51255125
}
51265126

‎src/PhpSpreadsheet/Calculation/Engine/FormattedNumber.php

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheet\Calculation\Engine;
4+
5+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
6+
7+
class FormattedNumber
8+
{
9+
/** Constants */
10+
/** Regular Expressions */
11+
// Fraction
12+
const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~';
13+
14+
const STRING_REGEXP_PERCENT = '~^(?:(?: *(?<PrefixedSign>[-+])? *\% *(?<PrefixedSign2>[-+])? *(?<PrefixedValue>[0-9]+\.?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?<PostfixedSign>[-+])? *(?<PostfixedValue>[0-9]+\.?[0-9]*(?:E[-+]?[0-9]*)?) *\% *))$~i';
15+
16+
/**
17+
* Identify whether a string contains a formatted numeric value,
18+
* and convert it to a numeric if it is.
19+
*
20+
* @param string $operand string value to test
21+
*/
22+
public static function convertToNumberIfFormatted(string &$operand): bool
23+
{
24+
$isFormattedNumber = is_numeric($operand);
25+
if ($isFormattedNumber === true) {
26+
$operand = (float) $operand;
27+
28+
return true;
29+
}
30+
31+
$isFormattedNumber = self::convertToNumberIfFraction($operand);
32+
if ($isFormattedNumber === false) {
33+
$isFormattedNumber = self::convertToNumberIfPercent($operand);
34+
}
35+
36+
return $isFormattedNumber;
37+
}
38+
39+
/**
40+
* Identify whether a string contains a fractional numeric value,
41+
* and convert it to a numeric if it is.
42+
*
43+
* @param string $operand string value to test
44+
*/
45+
public static function convertToNumberIfFraction(string &$operand): bool
46+
{
47+
if (preg_match(self::STRING_REGEXP_FRACTION, $operand, $match)) {
48+
$sign = ($match[1] === '-') ? '-' : '+';
49+
$wholePart = ($match[3] === '') ? '' : ($sign . $match[3]);
50+
$fractionFormula = '=' . $wholePart . $sign . $match[4];
51+
$operand = Calculation::getInstance()->_calculateFormulaValue($fractionFormula);
52+
53+
return true;
54+
}
55+
56+
return false;
57+
}
58+
59+
/**
60+
* Identify whether a string contains a percentage, and if so,
61+
* convert it to a numeric.
62+
*
63+
* @param string $operand string value to test
64+
*/
65+
public static function convertToNumberIfPercent(string &$operand): bool
66+
{
67+
$match = [];
68+
if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) {
69+
//Calculate the percentage
70+
$sign = ($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '';
71+
$operand = (float) ($sign . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100;
72+
73+
return true;
74+
}
75+
76+
return false;
77+
}
78+
}

‎src/PhpSpreadsheet/Shared/JAMA/Matrix.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Shared\JAMA;
44

5+
use PhpOffice\PhpSpreadsheet\Calculation\Engine\FormattedNumber;
56
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
67
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
78
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
8-
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
99

1010
/**
1111
* Matrix class.
@@ -1169,7 +1169,7 @@ private function validateExtractedValue($value, bool $validValues): array
11691169
}
11701170
if ((is_string($value)) && (strlen($value) > 0) && (!is_numeric($value))) {
11711171
$value = trim($value, '"');
1172-
$validValues &= StringHelper::convertToNumberIfFraction($value);
1172+
$validValues &= FormattedNumber::convertToNumberIfFormatted($value);
11731173
}
11741174

11751175
return [$value, $validValues];

‎src/PhpSpreadsheet/Shared/StringHelper.php

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,8 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Shared;
44

5-
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
6-
75
class StringHelper
86
{
9-
/** Constants */
10-
/** Regular Expressions */
11-
// Fraction
12-
const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~';
13-
14-
const STRING_REGEXP_PERCENT = '~^(?:(?: *(?<PrefixedSign>[-+])? *\% *(?<PrefixedSign2>[-+])? *(?<PrefixedValue>[0-9]+\.?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?<PostfixedSign>[-+])? *(?<PostfixedValue>[0-9]+\.?[0-9]*(?:E[-+]?[0-9]*)?) *\% *))$~i';
15-
167
/**
178
* Control characters array.
189
*
@@ -540,46 +531,6 @@ public static function strCaseReverse(string $textValue): string
540531
return implode('', $characters);
541532
}
542533

543-
/**
544-
* Identify whether a string contains a fractional numeric value,
545-
* and convert it to a numeric if it is.
546-
*
547-
* @param string $operand string value to test
548-
*/
549-
public static function convertToNumberIfFraction(string &$operand): bool
550-
{
551-
if (preg_match(self::STRING_REGEXP_FRACTION, $operand, $match)) {
552-
$sign = ($match[1] == '-') ? '-' : '+';
553-
$wholePart = ($match[3] === '') ? '' : ($sign . $match[3]);
554-
$fractionFormula = '=' . $wholePart . $sign . $match[4];
555-
$operand = Calculation::getInstance()->_calculateFormulaValue($fractionFormula);
556-
557-
return true;
558-
}
559-
560-
return false;
561-
}
562-
563-
/**
564-
* Identify whether a string contains a percentage, and if so,
565-
* convert it to a numeric.
566-
*
567-
* @param string $operand string value to test
568-
*/
569-
public static function convertToNumberIfPercent(string &$operand): bool
570-
{
571-
$match = [];
572-
if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) {
573-
//Calculate the percentage
574-
$sign = ($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '';
575-
$operand = (float) ($sign . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100;
576-
577-
return true;
578-
}
579-
580-
return false;
581-
}
582-
583534
/**
584535
* Get the decimal separator. If it has not yet been set explicitly, try to obtain number
585536
* formatting information from locale.

‎tests/PhpSpreadsheetTests/Calculation/CalculationTest.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,18 @@ public function testCellWithFormulaTwoIndirect(): void
180180
self::assertEquals('9', $cell3->getCalculatedValue());
181181
}
182182

183+
public function testCellWithStringFraction(): void
184+
{
185+
$spreadsheet = new Spreadsheet();
186+
$workSheet = $spreadsheet->getActiveSheet();
187+
$cell1 = $workSheet->getCell('A1');
188+
$cell1->setValue('3/4');
189+
$cell2 = $workSheet->getCell('B1');
190+
$cell2->setValue('=100*A1');
191+
192+
self::assertSame(75.0, $cell2->getCalculatedValue());
193+
}
194+
183195
public function testCellWithStringPercentage(): void
184196
{
185197
$spreadsheet = new Spreadsheet();
@@ -189,7 +201,7 @@ public function testCellWithStringPercentage(): void
189201
$cell2 = $workSheet->getCell('B1');
190202
$cell2->setValue('=100*A1');
191203

192-
self::assertEquals('2', $cell2->getCalculatedValue());
204+
self::assertSame(2.0, $cell2->getCalculatedValue());
193205
}
194206

195207
public function testBranchPruningFormulaParsingSimpleCase(): void

‎tests/PhpSpreadsheetTests/Calculation/Engine/FormattedNumberTest.php

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine;
4+
5+
use PhpOffice\PhpSpreadsheet\Calculation\Engine\FormattedNumber;
6+
use PHPUnit\Framework\TestCase;
7+
8+
class FormattedNumberTest extends TestCase
9+
{
10+
/**
11+
* @dataProvider providerFractions
12+
*/
13+
public function testFraction(string $expected, string $value): void
14+
{
15+
$originalValue = $value;
16+
$result = FormattedNumber::convertToNumberIfFraction($value);
17+
if ($result === false) {
18+
self::assertSame($expected, $originalValue);
19+
self::assertSame($expected, $value);
20+
} else {
21+
self::assertSame($expected, (string) $value);
22+
self::assertNotEquals($value, $originalValue);
23+
}
24+
}
25+
26+
public function providerFractions(): array
27+
{
28+
return [
29+
'non-fraction' => ['1', '1'],
30+
'common fraction' => ['1.5', '1 1/2'],
31+
'fraction between -1 and 0' => ['-0.5', '-1/2'],
32+
'fraction between -1 and 0 with space' => ['-0.5', ' - 1/2'],
33+
'fraction between 0 and 1' => ['0.75', '3/4 '],
34+
'fraction between 0 and 1 with space' => ['0.75', ' 3/4'],
35+
'improper fraction' => ['1.75', '7/4'],
36+
];
37+
}
38+
39+
/**
40+
* @dataProvider providerPercentages
41+
*/
42+
public function testPercentage(string $expected, string $value): void
43+
{
44+
$originalValue = $value;
45+
$result = FormattedNumber::convertToNumberIfPercent($value);
46+
if ($result === false) {
47+
self::assertSame($expected, $originalValue);
48+
self::assertSame($expected, $value);
49+
} else {
50+
self::assertSame($expected, (string) $value);
51+
self::assertNotEquals($value, $originalValue);
52+
}
53+
}
54+
55+
public function providerPercentages(): array
56+
{
57+
return [
58+
'non-percentage' => ['10', '10'],
59+
'single digit percentage' => ['0.02', '2%'],
60+
'two digit percentage' => ['0.13', '13%'],
61+
'negative single digit percentage' => ['-0.07', '-7%'],
62+
'negative two digit percentage' => ['-0.75', '-75%'],
63+
'large percentage' => ['98.45', '9845%'],
64+
'small percentage' => ['0.0005', '0.05%'],
65+
'percentage with decimals' => ['0.025', '2.5%'],
66+
'trailing percent with space' => ['0.02', '2 %'],
67+
'trailing percent with leading and trailing space' => ['0.02', ' 2 % '],
68+
'leading percent with decimals' => ['0.025', ' % 2.5'],
69+
70+
//These should all fail
71+
'percent only' => ['%', '%'],
72+
'nonsense percent' => ['2%2', '2%2'],
73+
'negative leading percent' => ['-0.02', '-%2'],
74+
75+
//Percent position permutations
76+
'permutation_1' => ['0.02', '2%'],
77+
'permutation_2' => ['0.02', ' 2%'],
78+
'permutation_3' => ['0.02', '2% '],
79+
'permutation_4' => ['0.02', ' 2 % '],
80+
'permutation_5' => ['0.0275', '2.75% '],
81+
'permutation_6' => ['0.0275', ' 2.75% '],
82+
'permutation_7' => ['0.0275', ' 2.75 % '],
83+
'permutation_8' => [' 2 . 75 %', ' 2 . 75 %'],
84+
'permutation_9' => [' 2.7 5 % ', ' 2.7 5 % '],
85+
'permutation_10' => ['-0.02', '-2%'],
86+
'permutation_11' => ['-0.02', ' -2% '],
87+
'permutation_12' => ['-0.02', '- 2% '],
88+
'permutation_13' => ['-0.02', '-2 % '],
89+
'permutation_14' => ['-0.0275', '-2.75% '],
90+
'permutation_15' => ['-0.0275', ' -2.75% '],
91+
'permutation_16' => ['-0.0275', '-2.75 % '],
92+
'permutation_17' => ['-0.0275', ' - 2.75 % '],
93+
'permutation_18' => ['0.02', '2%'],
94+
'permutation_19' => ['0.02', '% 2 '],
95+
'permutation_20' => ['0.02', ' %2 '],
96+
'permutation_21' => ['0.02', ' % 2 '],
97+
'permutation_22' => ['0.0275', '%2.75 '],
98+
'permutation_23' => ['0.0275', ' %2.75 '],
99+
'permutation_24' => ['0.0275', ' % 2.75 '],
100+
'permutation_25' => [' %2 . 75 ', ' %2 . 75 '],
101+
'permutation_26' => [' %2.7 5 ', ' %2.7 5 '],
102+
'permutation_27' => [' % 2 . 75 ', ' % 2 . 75 '],
103+
'permutation_28' => [' % 2.7 5 ', ' % 2.7 5 '],
104+
'permutation_29' => ['-0.0275', '-%2.75 '],
105+
'permutation_30' => ['-0.0275', ' - %2.75 '],
106+
'permutation_31' => ['-0.0275', '- % 2.75 '],
107+
'permutation_32' => ['-0.0275', ' - % 2.75 '],
108+
'permutation_33' => ['0.02', '2%'],
109+
'permutation_34' => ['0.02', '2 %'],
110+
'permutation_35' => ['0.02', ' 2%'],
111+
'permutation_36' => ['0.02', ' 2 % '],
112+
'permutation_37' => ['0.0275', '2.75%'],
113+
'permutation_38' => ['0.0275', ' 2.75 % '],
114+
'permutation_39' => ['2 . 75 % ', '2 . 75 % '],
115+
'permutation_40' => ['-0.0275', '-2.75% '],
116+
'permutation_41' => ['-0.0275', '- 2.75% '],
117+
'permutation_42' => ['-0.0275', ' - 2.75% '],
118+
'permutation_43' => ['-0.0275', ' -2.75 % '],
119+
'permutation_44' => ['-2. 75 % ', '-2. 75 % '],
120+
'permutation_45' => ['%', '%'],
121+
'permutation_46' => ['0.02', '%2 '],
122+
'permutation_47' => ['0.02', '% 2 '],
123+
'permutation_48' => ['0.02', ' %2 '],
124+
'permutation_49' => ['0.02', '% 2 '],
125+
'permutation_50' => ['0.02', ' % 2 '],
126+
'permutation_51' => ['0.02', ' 2 % '],
127+
'permutation_52' => ['-0.02', '-2%'],
128+
'permutation_53' => ['-0.02', '- %2'],
129+
'permutation_54' => ['-0.02', ' -%2 '],
130+
'permutation_55' => ['2%2', '2%2'],
131+
'permutation_56' => [' 2% %', ' 2% %'],
132+
'permutation_57' => [' % 2 -', ' % 2 -'],
133+
'permutation_58' => ['-0.02', '%-2'],
134+
'permutation_59' => ['-0.02', ' % - 2'],
135+
'permutation_60' => ['-0.0275', '%-2.75 '],
136+
'permutation_61' => ['-0.0275', ' % - 2.75 '],
137+
'permutation_62' => ['-0.0275', ' % - 2.75 '],
138+
'permutation_63' => ['-0.0275', ' % - 2.75 '],
139+
'permutation_64' => ['0.0275', ' % + 2.75 '],
140+
'permutation_65' => ['0.0275', ' % + 2.75 '],
141+
'permutation_66' => ['0.0275', ' % + 2.75 '],
142+
'permutation_67' => ['0.02', '+2%'],
143+
'permutation_68' => ['0.02', ' +2% '],
144+
'permutation_69' => ['0.02', '+ 2% '],
145+
'permutation_70' => ['0.02', '+2 % '],
146+
'permutation_71' => ['0.0275', '+2.75% '],
147+
'permutation_72' => ['0.0275', ' +2.75% '],
148+
'permutation_73' => ['0.0275', '+2.75 % '],
149+
'permutation_74' => ['0.0275', ' + 2.75 % '],
150+
'permutation_75' => ['-2.5E-6', '-2.5E-4%'],
151+
'permutation_76' => ['200', '2E4%'],
152+
'permutation_77' => ['-2.5E-8', '-%2.50E-06'],
153+
'permutation_78' => [' - % 2.50 E -06 ', ' - % 2.50 E -06 '],
154+
'permutation_79' => ['-2.5E-8', ' - % 2.50E-06 '],
155+
'permutation_80' => [' - % 2.50E- 06 ', ' - % 2.50E- 06 '],
156+
'permutation_81' => [' - % 2.50E - 06 ', ' - % 2.50E - 06 '],
157+
'permutation_82' => ['-2.5E-6', '-2.5e-4%'],
158+
'permutation_83' => ['200', '2e4%'],
159+
'permutation_84' => ['-2.5E-8', '-%2.50e-06'],
160+
'permutation_85' => [' - % 2.50 e -06 ', ' - % 2.50 e -06 '],
161+
'permutation_86' => ['-2.5E-8', ' - % 2.50e-06 '],
162+
'permutation_87' => [' - % 2.50e- 06 ', ' - % 2.50e- 06 '],
163+
'permutation_88' => [' - % 2.50e - 06 ', ' - % 2.50e - 06 '],
164+
];
165+
}
166+
}

0 commit comments

Comments
 (0)