From 1e3f579f37ed7087a66e4eb1baa1338ff4dc1391 Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Thu, 3 Nov 2022 22:01:43 -0400 Subject: [PATCH 1/9] Convert percentages stored as a string to numeric Further to https://github.com/PHPOffice/PhpSpreadsheet/issues/3155, I noticed that percentages stored as strings in Excel were still being used in formulas; Excel is apparently converting them behind the scenes. The same spreadsheet imported into PHPSpreadsheet was outputting a #VALUE! error. Added a function, modeled on the `convertToNumberIfFraction()` function, to detect if a string contains a percentage, and if so, convert it to a numeric. If the last character of the string is a % sign, the function will string the % sign and divide the value by 100, overwriting the original param with the new value. New helper function was added to `StringHelper.php` along the original faction helper. --- src/PhpSpreadsheet/Calculation/Calculation.php | 4 ++-- src/PhpSpreadsheet/Shared/StringHelper.php | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 763a5eaaa4..a597bc64c2 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -5110,8 +5110,8 @@ private function validateBinaryOperand(&$operand, &$stack) $this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($operand)); return false; - } elseif (!Shared\StringHelper::convertToNumberIfFraction($operand)) { - // If not a numeric or a fraction, then it's a text string, and so can't be used in mathematical binary operations + } elseif (!Shared\StringHelper::convertToNumberIfFraction($operand) && !Shared\StringHelper::convertToNumberIfPercent($operand)) { + // 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 $stack->push('Error', '#VALUE!'); $this->debugLog->writeDebugLog('Evaluation Result is a %s', $this->showTypeDetails('#VALUE!')); diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index 16026c3ca0..c882f93c66 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -560,6 +560,22 @@ public static function convertToNumberIfFraction(string &$operand): bool // function convertToNumberIfFraction() + /** + * Identify whether a string contains a percentage, and if so, + * convert it to a numeric. + * + * @param string $operand string value to test + */ + public static function convertToNumberIfPercent(string &$operand): bool + { + if (strpos($operand, '%', -1) !== false) { + $operand = (str_replace('%', '', $operand) / 100.00); + return true; + } + + return false; + } + /** * Get the decimal separator. If it has not yet been set explicitly, try to obtain number * formatting information from locale. From 21772479e12934e2733a7dc51f1f36fcc3513d89 Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Thu, 3 Nov 2022 22:03:24 -0400 Subject: [PATCH 2/9] Adding tests for conversion of string percentage to numeric Adding two test cases: 1) Test the function in `StringHelper.php`. Modeled this test on the test for the `convertToNumberIfFraction` function. 2) Test a spreadsheet with a string percentage in a formula to see if it calculates the formula correctly. --- .../Calculation/CalculationTest.php | 12 ++++++++ .../Shared/StringHelperTest.php | 29 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php index 6a03fa1d1e..0d730073e2 100644 --- a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php @@ -180,6 +180,18 @@ public function testCellWithFormulaTwoIndirect(): void self::assertEquals('9', $cell3->getCalculatedValue()); } + public function testCellWithStringPercentage(): void + { + $spreadsheet = new Spreadsheet(); + $workSheet = $spreadsheet->getActiveSheet(); + $cell1 = $workSheet->getCell('A1'); + $cell1->setValue('2%'); + $cell2 = $workSheet->getCell('B1'); + $cell2->setValue('=100*A1'); + + self::assertEquals('2', $cell2->getCalculatedValue()); + } + public function testBranchPruningFormulaParsingSimpleCase(): void { $calculation = Calculation::getInstance(); diff --git a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php index 85a92613ec..b9677bb3d1 100644 --- a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php +++ b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php @@ -148,4 +148,33 @@ public function providerFractions(): array 'improper fraction' => ['1.75', '7/4'], ]; } + + /** + * @dataProvider providerPercentages + */ + public function testPercentage(string $expected, string $value): void + { + $originalValue = $value; + $result = StringHelper::convertToNumberIfPercent($value); + if ($result === false) { + self::assertSame($expected, $originalValue); + self::assertSame($expected, $value); + } else { + self::assertSame($expected, (string) $value); + self::assertNotEquals($value, $originalValue); + } + } + + public function providerPercentages(): array + { + return [ + 'non-percentage' => ['10', '10'], + 'single digit percentage' => ['0.02', '2%'], + 'two digit percentage' => ['0.13', '13%'], + 'negative single digit percentage' => ['-0.07', '-7%'], + 'negative two digit percentage' => ['-0.75', '-75%'], + 'large percentage' => ['98.45', '9845%'], + 'small percentage' => ['0.0005', '0.05%'], + ]; + } } From 6439be5d14598303a2b4c4e8d15ebcc2fa423adb Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Fri, 4 Nov 2022 08:51:07 -0400 Subject: [PATCH 3/9] Correcting PHPStan type warning PHPStan failed because of a binary operation "/" between string and 100.0. Fixed this by casting the result of `str_replace` to a float. --- src/PhpSpreadsheet/Shared/StringHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index c882f93c66..109895a05e 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -569,7 +569,7 @@ public static function convertToNumberIfFraction(string &$operand): bool public static function convertToNumberIfPercent(string &$operand): bool { if (strpos($operand, '%', -1) !== false) { - $operand = (str_replace('%', '', $operand) / 100.00); + $operand = ((float)(str_replace('%', '', $operand)) / 100.00); return true; } From dc5c6b9f127f40781867479f2504b6db307abb72 Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Sun, 6 Nov 2022 12:31:49 -0500 Subject: [PATCH 4/9] Tightening up percentage check As suggested, the initial check has been tightened up using a regular expression. Expressing allows for leading and trailing spaces, leading and trailing percentage symbols, with spaces mixed throughout. Added 57 permutations of leading and trailing spaces, leading and trailing percent symbols, decimals, and negatives. All seem to be working. I will also attach an Excel worksheet that verifies the behaviour of all 57 test cases aligns with Excel. --- src/PhpSpreadsheet/Shared/StringHelper.php | 6 +- .../Shared/StringHelperTest.php | 68 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index 109895a05e..f72194707e 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -11,6 +11,8 @@ class StringHelper // Fraction const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~'; + const STRING_REGEXP_PERCENT = '~^( *-? *\% *[0-9]+\.?[0-9*]* *| *\-? *[0-9]+\.?[0-9]* *\% *)$~'; + /** * Control characters array. * @@ -568,8 +570,8 @@ public static function convertToNumberIfFraction(string &$operand): bool */ public static function convertToNumberIfPercent(string &$operand): bool { - if (strpos($operand, '%', -1) !== false) { - $operand = ((float)(str_replace('%', '', $operand)) / 100.00); + if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match)) { + $operand = ((float)(str_replace(['%', ' '], '', $match[0])) / 100.00); return true; } diff --git a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php index b9677bb3d1..cf37161e42 100644 --- a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php +++ b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php @@ -175,6 +175,74 @@ public function providerPercentages(): array 'negative two digit percentage' => ['-0.75', '-75%'], 'large percentage' => ['98.45', '9845%'], 'small percentage' => ['0.0005', '0.05%'], + 'percentage with decimals' => ['0.025', '2.5%'], + 'trailing percent with space' => ['0.02', '2 %'], + 'trailing percent with leading and trailing space' => ['0.02', ' 2 % '], + 'leading percent with decimals' => ['0.025', ' % 2.5'], + + //These should all fail + 'percent only' => ['%', '%'], + 'nonsense percent' => ['2%2', '2%2'], + 'negative leading percent' => ['-0.02', '-%2'], + + //Percent position permutations + 'permutation_1' => ['0.02', '2%'], + 'permutation_2' => ['0.02', ' 2%'], + 'permutation_3' => ['0.02', '2% '], + 'permutation_4' => ['0.02', ' 2 % '], + 'permutation_5' => ['0.0275', '2.75% '], + 'permutation_6' => ['0.0275', ' 2.75% '], + 'permutation_7' => ['0.0275', ' 2.75 % '], + 'permutation_8' => [' 2 . 75 %', ' 2 . 75 %'], + 'permutation_9' => [' 2.7 5 % ', ' 2.7 5 % '], + 'permutation_10' => ['-0.02', '-2%'], + 'permutation_11' => ['-0.02', ' -2% '], + 'permutation_12' => ['-0.02', '- 2% '], + 'permutation_13' => ['-0.02', '-2 % '], + 'permutation_14' => ['-0.0275', '-2.75% '], + 'permutation_15' => ['-0.0275', ' -2.75% '], + 'permutation_16' => ['-0.0275', '-2.75 % '], + 'permutation_17' => ['-0.0275', ' - 2.75 % '], + 'permutation_18' => ['0.02', '2%'], + 'permutation_19' => ['0.02', '% 2 '], + 'permutation_20' => ['0.02', ' %2 '], + 'permutation_21' => ['0.02', ' % 2 '], + 'permutation_22' => ['0.0275', '%2.75 '], + 'permutation_23' => ['0.0275', ' %2.75 '], + 'permutation_24' => ['0.0275', ' % 2.75 '], + 'permutation_25' => [' %2 . 75 ', ' %2 . 75 '], + 'permutation_26' => [' %2.7 5 ', ' %2.7 5 '], + 'permutation_27' => [' % 2 . 75 ', ' % 2 . 75 '], + 'permutation_28' => [' % 2.7 5 ', ' % 2.7 5 '], + 'permutation_29' => ['-0.0275', '-%2.75 '], + 'permutation_30' => ['-0.0275', ' - %2.75 '], + 'permutation_31' => ['-0.0275', '- % 2.75 '], + 'permutation_32' => ['-0.0275', ' - % 2.75 '], + 'permutation_33' => ['0.02', '2%'], + 'permutation_34' => ['0.02', '2 %'], + 'permutation_35' => ['0.02', ' 2%'], + 'permutation_36' => ['0.02', ' 2 % '], + 'permutation_37' => ['0.0275', '2.75%'], + 'permutation_38' => ['0.0275', ' 2.75 % '], + 'permutation_39' => ['2 . 75 % ', '2 . 75 % '], + 'permutation_40' => ['-0.0275', '-2.75% '], + 'permutation_41' => ['-0.0275', '- 2.75% '], + 'permutation_42' => ['-0.0275', ' - 2.75% '], + 'permutation_43' => ['-0.0275', ' -2.75 % '], + 'permutation_44' => ['-2. 75 % ', '-2. 75 % '], + 'permutation_45' => ['%', '%'], + 'permutation_46' => ['0.02', '%2 '], + 'permutation_47' => ['0.02', '% 2 '], + 'permutation_48' => ['0.02', ' %2 '], + 'permutation_49' => ['0.02', '% 2 '], + 'permutation_50' => ['0.02', ' % 2 '], + 'permutation_51' => ['0.02', ' 2 % '], + 'permutation_52' => ['-0.02', '-2%'], + 'permutation_53' => ['-0.02', '- %2'], + 'permutation_54' => ['-0.02', ' -%2 '], + 'permutation_55' => ['2%2', '2%2'], + 'permutation_56' => [' 2% %', ' 2% %'], + 'permutation_57' => [' % 2 -', ' % 2 -'], ]; } } From ab7b3b10f9d03f8895f86d2fca69f9d0a8f39572 Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Tue, 8 Nov 2022 12:32:15 -0500 Subject: [PATCH 5/9] Using Named Capture groups to handle edge cases Implemented the suggested named capture groups (nice trick) in the regexp to pull out only the values we need to perform the calculation. Making use of the null coalescing operator to keep things nice and clean. Note that I did have to add the `PREG_UNMATCHED_AS_NULL` to `preg_match` to ensure PHP returned null (since null coalescing only works on null and not '') when a match wasn't found for a capture group. Also extended the regular expression to handle scientific notation (both positive and negative, and positive and negative exponents) as well as a negative symbol before or after a leading percentage sign. --- src/PhpSpreadsheet/Shared/StringHelper.php | 16 ++++++++++--- .../Shared/StringHelperTest.php | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index f72194707e..abd436a341 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -11,7 +11,7 @@ class StringHelper // Fraction const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~'; - const STRING_REGEXP_PERCENT = '~^( *-? *\% *[0-9]+\.?[0-9*]* *| *\-? *[0-9]+\.?[0-9]* *\% *)$~'; + const STRING_REGEXP_PERCENT = '~^(?:(?: *(?[-+])? *\% *(?[-+])? *(?[0-9]+\.?[0-9*]*)(E(?[-+])?(?[0-9]*))? *)|(?: *(?[-+])? *(?[0-9]+\.?[0-9]*)(E(?[-+])?(?[0-9]*))? *\% *))$~'; /** * Control characters array. @@ -570,8 +570,18 @@ public static function convertToNumberIfFraction(string &$operand): bool */ public static function convertToNumberIfPercent(string &$operand): bool { - if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match)) { - $operand = ((float)(str_replace(['%', ' '], '', $match[0])) / 100.00); + $match = []; + if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) { + + //Extract needed named capture groups + $sign = (($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? ''); + $value = ($match['PostfixedValue'] ?? $match['PrefixedValue']); + $exponent = (($match['PrefixExponent'] ?? $match['PostfixExponent']) ?? 0); + $exponentSign = (($match['PrefixExponentSign'] ?? $match['PostfixExponentSign']) ?? ''); + + //Calculate the percentage + $operand = (float) (($sign . $value) * pow(10, ($exponentSign . $exponent))) / 100; + return true; } diff --git a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php index cf37161e42..71ecd90f17 100644 --- a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php +++ b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php @@ -243,6 +243,30 @@ public function providerPercentages(): array 'permutation_55' => ['2%2', '2%2'], 'permutation_56' => [' 2% %', ' 2% %'], 'permutation_57' => [' % 2 -', ' % 2 -'], + 'permutation_58' => ['-0.02', '%-2'], + 'permutation_59' => ['-0.02', ' % - 2'], + 'permutation_60' => ['-0.0275', '%-2.75 '], + 'permutation_61' => ['-0.0275', ' % - 2.75 '], + 'permutation_62' => ['-0.0275', ' % - 2.75 '], + 'permutation_63' => ['-0.0275', ' % - 2.75 '], + 'permutation_64' => ['0.0275', ' % + 2.75 '], + 'permutation_65' => ['0.0275', ' % + 2.75 '], + 'permutation_66' => ['0.0275', ' % + 2.75 '], + 'permutation_67' => ['0.02', '+2%'], + 'permutation_68' => ['0.02', ' +2% '], + 'permutation_69' => ['0.02', '+ 2% '], + 'permutation_70' => ['0.02', '+2 % '], + 'permutation_71' => ['0.0275', '+2.75% '], + 'permutation_72' => ['0.0275', ' +2.75% '], + 'permutation_73' => ['0.0275', '+2.75 % '], + 'permutation_74' => ['0.0275', ' + 2.75 % '], + 'permutation_75' => ['-2.5E-6', '-2.5E-4%'], + 'permutation_76' => ['200', '2E4%'], + 'permutation_77' => ['-2.5E-8', '-%2.50E-06'], + 'permutation_78' => [' - % 2.50 E -06 ', ' - % 2.50 E -06 '], + 'permutation_79' => ['-2.5E-8', ' - % 2.50E-06 '], + 'permutation_80' => [' - % 2.50E- 06 ', ' - % 2.50E- 06 '], + 'permutation_81' => [' - % 2.50E - 06 ', ' - % 2.50E - 06 '], ]; } } From 0b350b144bd3daffd748ae282a93777884757537 Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Wed, 9 Nov 2022 14:27:16 -0500 Subject: [PATCH 6/9] Slight optimization: Only use pow if exponent is greater than zero Originally thought I was being clever by using zero in `pow` to get 1, however after thinking about it all day I think it's probably better to avoid the call to `pow` if the exponent is zero; one less function call that has to be made. I didn't take the time to look into benchmarks of `pow` to the zero vs the ternary to multiply by 1, but my gut tells me this is likely slightly faster. --- src/PhpSpreadsheet/Shared/StringHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index abd436a341..857bdad1f5 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -580,7 +580,7 @@ public static function convertToNumberIfPercent(string &$operand): bool $exponentSign = (($match['PrefixExponentSign'] ?? $match['PostfixExponentSign']) ?? ''); //Calculate the percentage - $operand = (float) (($sign . $value) * pow(10, ($exponentSign . $exponent))) / 100; + $operand = (float) (($sign . $value) * ($exponent > 0 ? pow(10, ($exponentSign . $exponent)) : 1)) / 100; return true; } From bb845fae23db57a286c151770f396ad5d420d3a8 Mon Sep 17 00:00:00 2001 From: "fjohnston@avatarasoftware.com" Date: Wed, 9 Nov 2022 14:57:28 -0500 Subject: [PATCH 7/9] Further Simplification As was pointed out, the entire `pow` piece, as well as extracting exponent and exponent sign, was not necessary as casting to `float` will correctly handle scientific notation in a string. Also added case-insensitive flag to the regexp to handle upper and lower case `e`. --- src/PhpSpreadsheet/Shared/StringHelper.php | 10 ++-------- tests/PhpSpreadsheetTests/Shared/StringHelperTest.php | 7 +++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index 857bdad1f5..a426b077b4 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -11,7 +11,7 @@ class StringHelper // Fraction const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~'; - const STRING_REGEXP_PERCENT = '~^(?:(?: *(?[-+])? *\% *(?[-+])? *(?[0-9]+\.?[0-9*]*)(E(?[-+])?(?[0-9]*))? *)|(?: *(?[-+])? *(?[0-9]+\.?[0-9]*)(E(?[-+])?(?[0-9]*))? *\% *))$~'; + const STRING_REGEXP_PERCENT = '~^(?:(?: *(?[-+])? *\% *(?[-+])? *(?[0-9]+\.?[0-9*]*(E[-+]?[0-9]*)?) *)|(?: *(?[-+])? *(?[0-9]+\.?[0-9]*(E[-+]?[0-9]*)?) *\% *))$~i'; /** * Control characters array. @@ -573,14 +573,8 @@ public static function convertToNumberIfPercent(string &$operand): bool $match = []; if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) { - //Extract needed named capture groups - $sign = (($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? ''); - $value = ($match['PostfixedValue'] ?? $match['PrefixedValue']); - $exponent = (($match['PrefixExponent'] ?? $match['PostfixExponent']) ?? 0); - $exponentSign = (($match['PrefixExponentSign'] ?? $match['PostfixExponentSign']) ?? ''); - //Calculate the percentage - $operand = (float) (($sign . $value) * ($exponent > 0 ? pow(10, ($exponentSign . $exponent)) : 1)) / 100; + $operand = (float)((($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '') . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100; return true; } diff --git a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php index 71ecd90f17..b534782cf8 100644 --- a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php +++ b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php @@ -267,6 +267,13 @@ public function providerPercentages(): array 'permutation_79' => ['-2.5E-8', ' - % 2.50E-06 '], 'permutation_80' => [' - % 2.50E- 06 ', ' - % 2.50E- 06 '], 'permutation_81' => [' - % 2.50E - 06 ', ' - % 2.50E - 06 '], + 'permutation_82' => ['-2.5E-6', '-2.5e-4%'], + 'permutation_83' => ['200', '2e4%'], + 'permutation_84' => ['-2.5E-8', '-%2.50e-06'], + 'permutation_85' => [' - % 2.50 e -06 ', ' - % 2.50 e -06 '], + 'permutation_86' => ['-2.5E-8', ' - % 2.50e-06 '], + 'permutation_87' => [' - % 2.50e- 06 ', ' - % 2.50e- 06 '], + 'permutation_88' => [' - % 2.50e - 06 ', ' - % 2.50e - 06 '], ]; } } From 64a197ee42aa1067cc50be2acb429f2a595bebe9 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 10 Nov 2022 09:45:01 +0100 Subject: [PATCH 8/9] Styling fixes --- src/PhpSpreadsheet/Shared/StringHelper.php | 3 +- .../Shared/StringHelperTest.php | 176 +++++++++--------- 2 files changed, 89 insertions(+), 90 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index a426b077b4..ab124907ba 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -572,9 +572,8 @@ public static function convertToNumberIfPercent(string &$operand): bool { $match = []; if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) { - //Calculate the percentage - $operand = (float)((($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '') . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100; + $operand = (float) ((($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '') . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100; return true; } diff --git a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php index b534782cf8..34bde28824 100644 --- a/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php +++ b/tests/PhpSpreadsheetTests/Shared/StringHelperTest.php @@ -186,94 +186,94 @@ public function providerPercentages(): array 'negative leading percent' => ['-0.02', '-%2'], //Percent position permutations - 'permutation_1' => ['0.02', '2%'], - 'permutation_2' => ['0.02', ' 2%'], - 'permutation_3' => ['0.02', '2% '], - 'permutation_4' => ['0.02', ' 2 % '], - 'permutation_5' => ['0.0275', '2.75% '], - 'permutation_6' => ['0.0275', ' 2.75% '], - 'permutation_7' => ['0.0275', ' 2.75 % '], - 'permutation_8' => [' 2 . 75 %', ' 2 . 75 %'], - 'permutation_9' => [' 2.7 5 % ', ' 2.7 5 % '], - 'permutation_10' => ['-0.02', '-2%'], - 'permutation_11' => ['-0.02', ' -2% '], - 'permutation_12' => ['-0.02', '- 2% '], - 'permutation_13' => ['-0.02', '-2 % '], - 'permutation_14' => ['-0.0275', '-2.75% '], - 'permutation_15' => ['-0.0275', ' -2.75% '], - 'permutation_16' => ['-0.0275', '-2.75 % '], - 'permutation_17' => ['-0.0275', ' - 2.75 % '], - 'permutation_18' => ['0.02', '2%'], - 'permutation_19' => ['0.02', '% 2 '], - 'permutation_20' => ['0.02', ' %2 '], - 'permutation_21' => ['0.02', ' % 2 '], - 'permutation_22' => ['0.0275', '%2.75 '], - 'permutation_23' => ['0.0275', ' %2.75 '], - 'permutation_24' => ['0.0275', ' % 2.75 '], - 'permutation_25' => [' %2 . 75 ', ' %2 . 75 '], - 'permutation_26' => [' %2.7 5 ', ' %2.7 5 '], - 'permutation_27' => [' % 2 . 75 ', ' % 2 . 75 '], - 'permutation_28' => [' % 2.7 5 ', ' % 2.7 5 '], - 'permutation_29' => ['-0.0275', '-%2.75 '], - 'permutation_30' => ['-0.0275', ' - %2.75 '], - 'permutation_31' => ['-0.0275', '- % 2.75 '], - 'permutation_32' => ['-0.0275', ' - % 2.75 '], - 'permutation_33' => ['0.02', '2%'], - 'permutation_34' => ['0.02', '2 %'], - 'permutation_35' => ['0.02', ' 2%'], - 'permutation_36' => ['0.02', ' 2 % '], - 'permutation_37' => ['0.0275', '2.75%'], - 'permutation_38' => ['0.0275', ' 2.75 % '], - 'permutation_39' => ['2 . 75 % ', '2 . 75 % '], - 'permutation_40' => ['-0.0275', '-2.75% '], - 'permutation_41' => ['-0.0275', '- 2.75% '], - 'permutation_42' => ['-0.0275', ' - 2.75% '], - 'permutation_43' => ['-0.0275', ' -2.75 % '], - 'permutation_44' => ['-2. 75 % ', '-2. 75 % '], - 'permutation_45' => ['%', '%'], - 'permutation_46' => ['0.02', '%2 '], - 'permutation_47' => ['0.02', '% 2 '], - 'permutation_48' => ['0.02', ' %2 '], - 'permutation_49' => ['0.02', '% 2 '], - 'permutation_50' => ['0.02', ' % 2 '], - 'permutation_51' => ['0.02', ' 2 % '], - 'permutation_52' => ['-0.02', '-2%'], - 'permutation_53' => ['-0.02', '- %2'], - 'permutation_54' => ['-0.02', ' -%2 '], - 'permutation_55' => ['2%2', '2%2'], - 'permutation_56' => [' 2% %', ' 2% %'], - 'permutation_57' => [' % 2 -', ' % 2 -'], - 'permutation_58' => ['-0.02', '%-2'], - 'permutation_59' => ['-0.02', ' % - 2'], - 'permutation_60' => ['-0.0275', '%-2.75 '], - 'permutation_61' => ['-0.0275', ' % - 2.75 '], - 'permutation_62' => ['-0.0275', ' % - 2.75 '], - 'permutation_63' => ['-0.0275', ' % - 2.75 '], - 'permutation_64' => ['0.0275', ' % + 2.75 '], - 'permutation_65' => ['0.0275', ' % + 2.75 '], - 'permutation_66' => ['0.0275', ' % + 2.75 '], - 'permutation_67' => ['0.02', '+2%'], - 'permutation_68' => ['0.02', ' +2% '], - 'permutation_69' => ['0.02', '+ 2% '], - 'permutation_70' => ['0.02', '+2 % '], - 'permutation_71' => ['0.0275', '+2.75% '], - 'permutation_72' => ['0.0275', ' +2.75% '], - 'permutation_73' => ['0.0275', '+2.75 % '], - 'permutation_74' => ['0.0275', ' + 2.75 % '], - 'permutation_75' => ['-2.5E-6', '-2.5E-4%'], - 'permutation_76' => ['200', '2E4%'], - 'permutation_77' => ['-2.5E-8', '-%2.50E-06'], - 'permutation_78' => [' - % 2.50 E -06 ', ' - % 2.50 E -06 '], - 'permutation_79' => ['-2.5E-8', ' - % 2.50E-06 '], - 'permutation_80' => [' - % 2.50E- 06 ', ' - % 2.50E- 06 '], - 'permutation_81' => [' - % 2.50E - 06 ', ' - % 2.50E - 06 '], - 'permutation_82' => ['-2.5E-6', '-2.5e-4%'], - 'permutation_83' => ['200', '2e4%'], - 'permutation_84' => ['-2.5E-8', '-%2.50e-06'], - 'permutation_85' => [' - % 2.50 e -06 ', ' - % 2.50 e -06 '], - 'permutation_86' => ['-2.5E-8', ' - % 2.50e-06 '], - 'permutation_87' => [' - % 2.50e- 06 ', ' - % 2.50e- 06 '], - 'permutation_88' => [' - % 2.50e - 06 ', ' - % 2.50e - 06 '], + 'permutation_1' => ['0.02', '2%'], + 'permutation_2' => ['0.02', ' 2%'], + 'permutation_3' => ['0.02', '2% '], + 'permutation_4' => ['0.02', ' 2 % '], + 'permutation_5' => ['0.0275', '2.75% '], + 'permutation_6' => ['0.0275', ' 2.75% '], + 'permutation_7' => ['0.0275', ' 2.75 % '], + 'permutation_8' => [' 2 . 75 %', ' 2 . 75 %'], + 'permutation_9' => [' 2.7 5 % ', ' 2.7 5 % '], + 'permutation_10' => ['-0.02', '-2%'], + 'permutation_11' => ['-0.02', ' -2% '], + 'permutation_12' => ['-0.02', '- 2% '], + 'permutation_13' => ['-0.02', '-2 % '], + 'permutation_14' => ['-0.0275', '-2.75% '], + 'permutation_15' => ['-0.0275', ' -2.75% '], + 'permutation_16' => ['-0.0275', '-2.75 % '], + 'permutation_17' => ['-0.0275', ' - 2.75 % '], + 'permutation_18' => ['0.02', '2%'], + 'permutation_19' => ['0.02', '% 2 '], + 'permutation_20' => ['0.02', ' %2 '], + 'permutation_21' => ['0.02', ' % 2 '], + 'permutation_22' => ['0.0275', '%2.75 '], + 'permutation_23' => ['0.0275', ' %2.75 '], + 'permutation_24' => ['0.0275', ' % 2.75 '], + 'permutation_25' => [' %2 . 75 ', ' %2 . 75 '], + 'permutation_26' => [' %2.7 5 ', ' %2.7 5 '], + 'permutation_27' => [' % 2 . 75 ', ' % 2 . 75 '], + 'permutation_28' => [' % 2.7 5 ', ' % 2.7 5 '], + 'permutation_29' => ['-0.0275', '-%2.75 '], + 'permutation_30' => ['-0.0275', ' - %2.75 '], + 'permutation_31' => ['-0.0275', '- % 2.75 '], + 'permutation_32' => ['-0.0275', ' - % 2.75 '], + 'permutation_33' => ['0.02', '2%'], + 'permutation_34' => ['0.02', '2 %'], + 'permutation_35' => ['0.02', ' 2%'], + 'permutation_36' => ['0.02', ' 2 % '], + 'permutation_37' => ['0.0275', '2.75%'], + 'permutation_38' => ['0.0275', ' 2.75 % '], + 'permutation_39' => ['2 . 75 % ', '2 . 75 % '], + 'permutation_40' => ['-0.0275', '-2.75% '], + 'permutation_41' => ['-0.0275', '- 2.75% '], + 'permutation_42' => ['-0.0275', ' - 2.75% '], + 'permutation_43' => ['-0.0275', ' -2.75 % '], + 'permutation_44' => ['-2. 75 % ', '-2. 75 % '], + 'permutation_45' => ['%', '%'], + 'permutation_46' => ['0.02', '%2 '], + 'permutation_47' => ['0.02', '% 2 '], + 'permutation_48' => ['0.02', ' %2 '], + 'permutation_49' => ['0.02', '% 2 '], + 'permutation_50' => ['0.02', ' % 2 '], + 'permutation_51' => ['0.02', ' 2 % '], + 'permutation_52' => ['-0.02', '-2%'], + 'permutation_53' => ['-0.02', '- %2'], + 'permutation_54' => ['-0.02', ' -%2 '], + 'permutation_55' => ['2%2', '2%2'], + 'permutation_56' => [' 2% %', ' 2% %'], + 'permutation_57' => [' % 2 -', ' % 2 -'], + 'permutation_58' => ['-0.02', '%-2'], + 'permutation_59' => ['-0.02', ' % - 2'], + 'permutation_60' => ['-0.0275', '%-2.75 '], + 'permutation_61' => ['-0.0275', ' % - 2.75 '], + 'permutation_62' => ['-0.0275', ' % - 2.75 '], + 'permutation_63' => ['-0.0275', ' % - 2.75 '], + 'permutation_64' => ['0.0275', ' % + 2.75 '], + 'permutation_65' => ['0.0275', ' % + 2.75 '], + 'permutation_66' => ['0.0275', ' % + 2.75 '], + 'permutation_67' => ['0.02', '+2%'], + 'permutation_68' => ['0.02', ' +2% '], + 'permutation_69' => ['0.02', '+ 2% '], + 'permutation_70' => ['0.02', '+2 % '], + 'permutation_71' => ['0.0275', '+2.75% '], + 'permutation_72' => ['0.0275', ' +2.75% '], + 'permutation_73' => ['0.0275', '+2.75 % '], + 'permutation_74' => ['0.0275', ' + 2.75 % '], + 'permutation_75' => ['-2.5E-6', '-2.5E-4%'], + 'permutation_76' => ['200', '2E4%'], + 'permutation_77' => ['-2.5E-8', '-%2.50E-06'], + 'permutation_78' => [' - % 2.50 E -06 ', ' - % 2.50 E -06 '], + 'permutation_79' => ['-2.5E-8', ' - % 2.50E-06 '], + 'permutation_80' => [' - % 2.50E- 06 ', ' - % 2.50E- 06 '], + 'permutation_81' => [' - % 2.50E - 06 ', ' - % 2.50E - 06 '], + 'permutation_82' => ['-2.5E-6', '-2.5e-4%'], + 'permutation_83' => ['200', '2e4%'], + 'permutation_84' => ['-2.5E-8', '-%2.50e-06'], + 'permutation_85' => [' - % 2.50 e -06 ', ' - % 2.50 e -06 '], + 'permutation_86' => ['-2.5E-8', ' - % 2.50e-06 '], + 'permutation_87' => [' - % 2.50e- 06 ', ' - % 2.50e- 06 '], + 'permutation_88' => [' - % 2.50e - 06 ', ' - % 2.50e - 06 '], ]; } } From 6f3f4501010d42a666afaeb571be6cb5a6e3fc5d Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 10 Nov 2022 10:11:07 +0100 Subject: [PATCH 9/9] Improve readability --- src/PhpSpreadsheet/Shared/StringHelper.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index ab124907ba..06edf07221 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -11,7 +11,7 @@ class StringHelper // Fraction const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~'; - const STRING_REGEXP_PERCENT = '~^(?:(?: *(?[-+])? *\% *(?[-+])? *(?[0-9]+\.?[0-9*]*(E[-+]?[0-9]*)?) *)|(?: *(?[-+])? *(?[0-9]+\.?[0-9]*(E[-+]?[0-9]*)?) *\% *))$~i'; + const STRING_REGEXP_PERCENT = '~^(?:(?: *(?[-+])? *\% *(?[-+])? *(?[0-9]+\.?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?[-+])? *(?[0-9]+\.?[0-9]*(?:E[-+]?[0-9]*)?) *\% *))$~i'; /** * Control characters array. @@ -560,8 +560,6 @@ public static function convertToNumberIfFraction(string &$operand): bool return false; } - // function convertToNumberIfFraction() - /** * Identify whether a string contains a percentage, and if so, * convert it to a numeric. @@ -573,7 +571,8 @@ public static function convertToNumberIfPercent(string &$operand): bool $match = []; if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) { //Calculate the percentage - $operand = (float) ((($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '') . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100; + $sign = ($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? ''; + $operand = (float) ($sign . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100; return true; }