Skip to content

Commit

Permalink
Merge pull request #3164 from PHPOffice/CalcEngine-Refactor_Formatted…
Browse files Browse the repository at this point in the history
…-Numbers

Refactoring for checks on strings containing formatted numeric values
  • Loading branch information
MarkBaker authored Nov 11, 2022
2 parents 6f71efc + 6a50973 commit c928bfd
Show file tree
Hide file tree
Showing 8 changed files with 311 additions and 211 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).

### Fixed

- Treat strings containing percentage values as floats in Calculation Engine operations [Issue #3155](https://github.com/PHPOffice/PhpSpreadsheet/issues/3155) [PR #3156](https://github.com/PHPOffice/PhpSpreadsheet/pull/3156) and [PR #3164](https://github.com/PHPOffice/PhpSpreadsheet/pull/3164)
- Xlsx Reader Accept Palette of Fewer than 64 Colors [Issue #3093](https://github.com/PHPOffice/PhpSpreadsheet/issues/3093) [PR #3096](https://github.com/PHPOffice/PhpSpreadsheet/pull/3096)
- Use Locale-Independent Float Conversion for Xlsx Writer Custom Property [Issue #3095](https://github.com/PHPOffice/PhpSpreadsheet/issues/3095) [PR #3099](https://github.com/PHPOffice/PhpSpreadsheet/pull/3099)
- Allow setting AutoFilter range on a single cell or row [Issue #3102](https://github.com/PHPOffice/PhpSpreadsheet/issues/3102) [PR #3111](https://github.com/PHPOffice/PhpSpreadsheet/pull/3111)
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -5110,7 +5110,7 @@ private function validateBinaryOperand(&$operand, &$stack)
$this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($operand));

return false;
} elseif (!Shared\StringHelper::convertToNumberIfFraction($operand) && !Shared\StringHelper::convertToNumberIfPercent($operand)) {
} elseif (Engine\FormattedNumber::convertToNumberIfFormatted($operand) === false) {
// 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!'));
Expand All @@ -5120,7 +5120,7 @@ private function validateBinaryOperand(&$operand, &$stack)
}
}

// return a true if the value of the operand is one that we can use in normal binary operations
// return a true if the value of the operand is one that we can use in normal binary mathematical operations
return true;
}

Expand Down
95 changes: 95 additions & 0 deletions src/PhpSpreadsheet/Calculation/Engine/FormattedNumber.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<?php

namespace PhpOffice\PhpSpreadsheet\Calculation\Engine;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;

class FormattedNumber
{
/** Constants */
/** Regular Expressions */
// Fraction
private const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~';

private const STRING_REGEXP_PERCENT = '~^(?:(?: *(?<PrefixedSign>[-+])? *\% *(?<PrefixedSign2>[-+])? *(?<PrefixedValue>[0-9]+\.?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?<PostfixedSign>[-+])? *(?<PostfixedValue>[0-9]+\.?[0-9]*(?:E[-+]?[0-9]*)?) *\% *))$~i';

private const STRING_CONVERSION_LIST = [
[self::class, 'convertToNumberIfNumeric'],
[self::class, 'convertToNumberIfFraction'],
[self::class, 'convertToNumberIfPercent'],
];

/**
* Identify whether a string contains a formatted numeric value,
* and convert it to a numeric if it is.
*
* @param string $operand string value to test
*/
public static function convertToNumberIfFormatted(string &$operand): bool
{
foreach (self::STRING_CONVERSION_LIST as $conversionMethod) {
if ($conversionMethod($operand) === true) {
return true;
}
}

return false;
}

/**
* Identify whether a string contains a numeric value,
* and convert it to a numeric if it is.
*
* @param string $operand string value to test
*/
public static function convertToNumberIfNumeric(string &$operand): bool
{
if (is_numeric($operand)) {
$operand = (float) $operand;

return true;
}

return false;
}

/**
* Identify whether a string contains a fractional numeric value,
* and convert it to a numeric if it is.
*
* @param string $operand string value to test
*/
public static function convertToNumberIfFraction(string &$operand): bool
{
if (preg_match(self::STRING_REGEXP_FRACTION, $operand, $match)) {
$sign = ($match[1] === '-') ? '-' : '+';
$wholePart = ($match[3] === '') ? '' : ($sign . $match[3]);
$fractionFormula = '=' . $wholePart . $sign . $match[4];
$operand = Calculation::getInstance()->_calculateFormulaValue($fractionFormula);

return true;
}

return false;
}

/**
* 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
{
$match = [];
if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) {
//Calculate the percentage
$sign = ($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '';
$operand = (float) ($sign . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100;

return true;
}

return false;
}
}
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Shared/JAMA/Matrix.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

namespace PhpOffice\PhpSpreadsheet\Shared\JAMA;

use PhpOffice\PhpSpreadsheet\Calculation\Engine\FormattedNumber;
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;

/**
* Matrix class.
Expand Down Expand Up @@ -1169,7 +1169,7 @@ private function validateExtractedValue($value, bool $validValues): array
}
if ((is_string($value)) && (strlen($value) > 0) && (!is_numeric($value))) {
$value = trim($value, '"');
$validValues &= StringHelper::convertToNumberIfFraction($value);
$validValues &= FormattedNumber::convertToNumberIfFormatted($value);
}

return [$value, $validValues];
Expand Down
49 changes: 0 additions & 49 deletions src/PhpSpreadsheet/Shared/StringHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,8 @@

namespace PhpOffice\PhpSpreadsheet\Shared;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;

class StringHelper
{
/** Constants */
/** Regular Expressions */
// Fraction
const STRING_REGEXP_FRACTION = '~^\s*(-?)((\d*)\s+)?(\d+\/\d+)\s*$~';

const STRING_REGEXP_PERCENT = '~^(?:(?: *(?<PrefixedSign>[-+])? *\% *(?<PrefixedSign2>[-+])? *(?<PrefixedValue>[0-9]+\.?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?<PostfixedSign>[-+])? *(?<PostfixedValue>[0-9]+\.?[0-9]*(?:E[-+]?[0-9]*)?) *\% *))$~i';

/**
* Control characters array.
*
Expand Down Expand Up @@ -540,46 +531,6 @@ public static function strCaseReverse(string $textValue): string
return implode('', $characters);
}

/**
* Identify whether a string contains a fractional numeric value,
* and convert it to a numeric if it is.
*
* @param string $operand string value to test
*/
public static function convertToNumberIfFraction(string &$operand): bool
{
if (preg_match(self::STRING_REGEXP_FRACTION, $operand, $match)) {
$sign = ($match[1] == '-') ? '-' : '+';
$wholePart = ($match[3] === '') ? '' : ($sign . $match[3]);
$fractionFormula = '=' . $wholePart . $sign . $match[4];
$operand = Calculation::getInstance()->_calculateFormulaValue($fractionFormula);

return true;
}

return false;
}

/**
* 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
{
$match = [];
if (preg_match(self::STRING_REGEXP_PERCENT, $operand, $match, PREG_UNMATCHED_AS_NULL)) {
//Calculate the percentage
$sign = ($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '';
$operand = (float) ($sign . ($match['PostfixedValue'] ?? $match['PrefixedValue'])) / 100;

return true;
}

return false;
}

/**
* Get the decimal separator. If it has not yet been set explicitly, try to obtain number
* formatting information from locale.
Expand Down
26 changes: 25 additions & 1 deletion tests/PhpSpreadsheetTests/Calculation/CalculationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,30 @@ public function testCellWithFormulaTwoIndirect(): void
self::assertEquals('9', $cell3->getCalculatedValue());
}

public function testCellWithStringNumeric(): void
{
$spreadsheet = new Spreadsheet();
$workSheet = $spreadsheet->getActiveSheet();
$cell1 = $workSheet->getCell('A1');
$cell1->setValue('+2.5');
$cell2 = $workSheet->getCell('B1');
$cell2->setValue('=100*A1');

self::assertSame(250.0, $cell2->getCalculatedValue());
}

public function testCellWithStringFraction(): void
{
$spreadsheet = new Spreadsheet();
$workSheet = $spreadsheet->getActiveSheet();
$cell1 = $workSheet->getCell('A1');
$cell1->setValue('3/4');
$cell2 = $workSheet->getCell('B1');
$cell2->setValue('=100*A1');

self::assertSame(75.0, $cell2->getCalculatedValue());
}

public function testCellWithStringPercentage(): void
{
$spreadsheet = new Spreadsheet();
Expand All @@ -189,7 +213,7 @@ public function testCellWithStringPercentage(): void
$cell2 = $workSheet->getCell('B1');
$cell2->setValue('=100*A1');

self::assertEquals('2', $cell2->getCalculatedValue());
self::assertSame(2.0, $cell2->getCalculatedValue());
}

public function testBranchPruningFormulaParsingSimpleCase(): void
Expand Down
Loading

0 comments on commit c928bfd

Please sign in to comment.