Skip to content

Accommodating Slash with preg_quote - Delimiters #3584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/PhpSpreadsheet/Calculation/Engine/FormattedNumber.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ public static function convertToNumberIfFormatted(string &$operand): bool
*/
public static function convertToNumberIfNumeric(string &$operand): bool
{
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');
$value = preg_replace(['/(\d)' . $thousandsSeparator . '(\d)/u', '/([+-])\s+(\d)/u'], ['$1$2', '$1$2'], trim($operand));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');
$value = preg_replace(['/(\d)' . $decimalSeparator . '(\d)/u', '/([+-])\s+(\d)/u'], ['$1.$2', '$1$2'], $value ?? '');

if (is_numeric($value)) {
Expand Down Expand Up @@ -90,9 +90,9 @@ public static function convertToNumberIfFraction(string &$operand): bool
*/
public static function convertToNumberIfPercent(string &$operand): bool
{
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');
$value = preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', trim($operand));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');
$value = preg_replace(['/(\d)' . $decimalSeparator . '(\d)/u', '/([+-])\s+(\d)/u'], ['$1.$2', '$1$2'], $value ?? '');

$match = [];
Expand All @@ -116,26 +116,31 @@ public static function convertToNumberIfPercent(string &$operand): bool
public static function convertToNumberIfCurrency(string &$operand): bool
{
$currencyRegexp = self::currencyMatcherRegexp();
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');
$value = preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', $operand);

$match = [];
if ($value !== null && preg_match($currencyRegexp, $value, $match, PREG_UNMATCHED_AS_NULL)) {
//Determine the sign
$sign = ($match['PrefixedSign'] ?? $match['PrefixedSign2'] ?? $match['PostfixedSign']) ?? '';
$decimalSeparator = StringHelper::getDecimalSeparator();
//Cast to a float
$operand = (float) ($sign . ($match['PostfixedValue'] ?? $match['PrefixedValue']));
$intermediate = (string) ($match['PostfixedValue'] ?? $match['PrefixedValue']);
$intermediate = str_replace($decimalSeparator, '.', $intermediate);
if (is_numeric($intermediate)) {
$operand = (float) ($sign . str_replace($decimalSeparator, '.', $intermediate));

return true;
return true;
}
}

return false;
}

public static function currencyMatcherRegexp(): string
{
$currencyCodes = sprintf(self::CURRENCY_CONVERSION_LIST, preg_quote(StringHelper::getCurrencyCode()));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$currencyCodes = sprintf(self::CURRENCY_CONVERSION_LIST, preg_quote(StringHelper::getCurrencyCode(), '/'));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');

return '~^(?:(?: *(?<PrefixedSign>[-+])? *(?<PrefixedCurrency>[' . $currencyCodes . ']) *(?<PrefixedSign2>[-+])? *(?<PrefixedValue>[0-9]+[' . $decimalSeparator . ']?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?<PostfixedSign>[-+])? *(?<PostfixedValue>[0-9]+' . $decimalSeparator . '?[0-9]*(?:E[-+]?[0-9]*)?) *(?<PostfixedCurrency>[' . $currencyCodes . ']) *))$~ui';
}
Expand Down
7 changes: 4 additions & 3 deletions src/PhpSpreadsheet/Cell/AdvancedValueBinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ public function bindValue(Cell $cell, $value = null)
return $this->setImproperFraction($matches, $cell);
}

$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$decimalSeparatorNoPreg = StringHelper::getDecimalSeparator();
$decimalSeparator = preg_quote($decimalSeparatorNoPreg, '/');
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');

// Check for percentage
if (preg_match('/^\-?\d*' . $decimalSeparator . '?\d*\s?\%$/', preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', $value))) {
Expand All @@ -64,7 +65,7 @@ public function bindValue(Cell $cell, $value = null)
// Convert value to number
$sign = ($matches['PrefixedSign'] ?? $matches['PrefixedSign2'] ?? $matches['PostfixedSign']) ?? null;
$currencyCode = $matches['PrefixedCurrency'] ?? $matches['PostfixedCurrency'];
$value = (float) ($sign . trim(str_replace([$decimalSeparator, $currencyCode, ' ', '-'], ['.', '', '', ''], preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', $value)))); // @phpstan-ignore-line
$value = (float) ($sign . trim(str_replace([$decimalSeparatorNoPreg, $currencyCode, ' ', '-'], ['.', '', '', ''], preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', $value)))); // @phpstan-ignore-line

return $this->setCurrency($value, $cell, $currencyCode); // @phpstan-ignore-line
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine;

use PhpOffice\PhpSpreadsheet\Calculation\Engine\FormattedNumber;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
use PHPUnit\Framework\TestCase;

class FormattedNumberSlashTest extends TestCase
{
private string $originalCurrencyCode;

private string $originalDecimalSeparator;

private string $originalThousandsSeparator;

protected function setUp(): void
{
$this->originalCurrencyCode = StringHelper::getCurrencyCode();
$this->originalDecimalSeparator = StringHelper::getDecimalSeparator();
$this->originalThousandsSeparator = StringHelper::getThousandsSeparator();
}

protected function tearDown(): void
{
StringHelper::setCurrencyCode($this->originalCurrencyCode);
StringHelper::setDecimalSeparator($this->originalDecimalSeparator);
StringHelper::setThousandsSeparator($this->originalThousandsSeparator);
}

/**
* @dataProvider providerNumbers
*
* @param mixed $expected
*/
public function testNumber($expected, string $value, string $thousandsSeparator = ',', string $decimalSeparator = '.'): void
{
StringHelper::setThousandsSeparator($thousandsSeparator);
StringHelper::setDecimalSeparator($decimalSeparator);
$result = FormattedNumber::convertToNumberIfFormatted($value);
self::assertTrue($result);
self::assertSame($expected, $value);
}

public static function providerNumbers(): array
{
return [
'normal' => [1234.5, '1,234.5'],
'slash as thousands separator' => [-1234.5, '- 1/234.5', '/', '.'],
'slash as decimal separator' => [-1234.5, '- 1,234/5', ',', '/'],
];
}

/**
* @dataProvider providerPercentages
*/
public function testPercentage(string $expected, string $value, string $thousandsSeparator = ',', string $decimalSeparator = '.'): void
{
$originalValue = $value;
StringHelper::setThousandsSeparator($thousandsSeparator);
StringHelper::setDecimalSeparator($decimalSeparator);
$result = FormattedNumber::convertToNumberIfPercent($value);
self::assertTrue($result);
self::assertSame($expected, (string) $value);
self::assertNotEquals($value, $originalValue);
}

public static function providerPercentages(): array
{
return [
'normal' => ['21.5034', '2,150.34%'],
'slash as thousands separator' => ['21.5034', '2/150.34%', '/', '.'],
'slash as decimal separator' => ['21.5034', '2,150/34%', ',', '/'],
];
}

/**
* @dataProvider providerCurrencies
*/
public function testCurrencies(string $expected, string $value, string $thousandsSeparator = ',', string $decimalSeparator = '.', ?string $currencyCode = null): void
{
$originalValue = $value;
StringHelper::setThousandsSeparator($thousandsSeparator);
StringHelper::setDecimalSeparator($decimalSeparator);
if ($currencyCode !== null) {
StringHelper::setCurrencyCode($currencyCode);
}
$result = FormattedNumber::convertToNumberIfCurrency($value);
self::assertTrue($result);
self::assertSame($expected, (string) $value);
self::assertNotEquals($value, $originalValue);
}

public static function providerCurrencies(): array
{
return [
'switched delimiters' => ['2134.56', '$2.134,56', '.', ','],
'normal' => ['2134.56', '$2,134.56'],
'slash as thousands separator' => ['2134.56', '$2/134.56', '/', '.'],
'slash as decimal separator' => ['2134.56', '$2,134/56', ',', '/'],
'slash as currency code' => ['2134.56', '/2,134.56', ',', '.', '/'],
];
}
}
40 changes: 17 additions & 23 deletions tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,41 +14,33 @@ class AdvancedValueBinderTest extends TestCase
{
const AVB_PRECISION = 1.0E-8;

/**
* @var string
*/
private $currencyCode;
private string $originalLocale;

/**
* @var string
*/
private $decimalSeparator;
private string $originalCurrencyCode;

/**
* @var string
*/
private $thousandsSeparator;
private string $originalDecimalSeparator;

/**
* @var IValueBinder
*/
private $valueBinder;
private string $originalThousandsSeparator;

private IValueBinder $valueBinder;

protected function setUp(): void
{
Settings::setLocale('en_US');
$this->currencyCode = StringHelper::getCurrencyCode();
$this->decimalSeparator = StringHelper::getDecimalSeparator();
$this->thousandsSeparator = StringHelper::getThousandsSeparator();
$this->originalLocale = Settings::getLocale();
$this->originalCurrencyCode = StringHelper::getCurrencyCode();
$this->originalDecimalSeparator = StringHelper::getDecimalSeparator();
$this->originalThousandsSeparator = StringHelper::getThousandsSeparator();

$this->valueBinder = Cell::getValueBinder();
Cell::setValueBinder(new AdvancedValueBinder());
}

protected function tearDown(): void
{
StringHelper::setCurrencyCode($this->currencyCode);
StringHelper::setDecimalSeparator($this->decimalSeparator);
StringHelper::setThousandsSeparator($this->thousandsSeparator);
StringHelper::setCurrencyCode($this->originalCurrencyCode);
StringHelper::setDecimalSeparator($this->originalDecimalSeparator);
StringHelper::setThousandsSeparator($this->originalThousandsSeparator);
Settings::setLocale($this->originalLocale);
Cell::setValueBinder($this->valueBinder);
}

Expand Down Expand Up @@ -134,6 +126,8 @@ public static function currencyProvider(): array
['€2,020.22', 2020.22, ',', '.', '€'],
['$10.11', 10.11, ',', '.', '€'],
['€2,020.20', 2020.2, ',', '.', '$'],
'slash as group separator' => ['€2/020.20', 2020.2, '/', '.', '$'],
'slash as decimal separator' => ['€2,020/20', 2020.2, ',', '/', '$'],
['-2,020.20€', -2020.2, ',', '.', '$'],
['- 2,020.20 € ', -2020.2, ',', '.', '$'],
];
Expand Down