Skip to content
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.

Sign up for GitHub

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

Php 8.3 Problem - RLM Added to NumberFormatter Currency - Minor Break #3640

Merged
merged 3 commits into from
Jul 24, 2023
Merged

Php 8.3 Problem - RLM Added to NumberFormatter Currency - Minor Break #3640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0753ad5
Php 8.3 Problem - RLM Added to NumberFormatter Currency - Minor Break
oleibman Jul 9, 2023
0d8561d
Eliminate 2 Dead Statements
oleibman Jul 9, 2023
7e7a1da
Merge branch 'master' into issue3571
oleibman Jul 24, 2023
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
14 changes: 10 additions & 4 deletions src/PhpSpreadsheet/Style/NumberFormat/Wizard/Accounting.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,16 @@ public function __construct(
bool $thousandsSeparator = true,
bool $currencySymbolPosition = self::LEADING_SYMBOL,
bool $currencySymbolSpacing = self::SYMBOL_WITHOUT_SPACING,
?string $locale = null
?string $locale = null,
bool $stripLeadingRLM = self::DEFAULT_STRIP_LEADING_RLM
) {
$this->setCurrencyCode($currencyCode);
$this->setThousandsSeparator($thousandsSeparator);
$this->setDecimals($decimals);
$this->setCurrencySymbolPosition($currencySymbolPosition);
$this->setCurrencySymbolSpacing($currencySymbolSpacing);
$this->setLocale($locale);
$this->stripLeadingRLM = $stripLeadingRLM;
}

/**
Expand All @@ -44,24 +46,28 @@ public function __construct(
protected function getLocaleFormat(): string
{
if (version_compare(PHP_VERSION, '7.4.1', '<')) {
// @codeCoverageIgnoreStart
throw new Exception('The Intl extension does not support Accounting Formats below PHP 7.4.1');
// @codeCoverageIgnoreEnd
}

if ($this->icuVersion() < 53.0) {
if (self::icuVersion() < 53.0) {
// @codeCoverageIgnoreStart
throw new Exception('The Intl extension does not support Accounting Formats without ICU 53');
// @codeCoverageIgnoreEnd
}

// Scrutinizer does not recognize CURRENCY_ACCOUNTING
$formatter = new Locale($this->fullLocale, NumberFormatter::CURRENCY_ACCOUNTING);
$mask = $formatter->format();
$mask = $formatter->format($this->stripLeadingRLM);
if ($this->decimals === 0) {
$mask = (string) preg_replace('/\.0+/miu', '', $mask);
}

return str_replace('¤', $this->formatCurrencyCode(), $mask);
}

private function icuVersion(): float
public static function icuVersion(): float
{
[$major, $minor] = explode('.', INTL_ICU_VERSION);

Expand Down
17 changes: 15 additions & 2 deletions src/PhpSpreadsheet/Style/NumberFormat/Wizard/Currency.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ class Currency extends Number

protected bool $currencySymbolSpacing = self::SYMBOL_WITHOUT_SPACING;

protected const DEFAULT_STRIP_LEADING_RLM = false;

protected bool $stripLeadingRLM = self::DEFAULT_STRIP_LEADING_RLM;

/**
* @param string $currencyCode the currency symbol or code to display for this mask
* @param int $decimals number of decimal places to display, in the range 0-30
Expand All @@ -33,6 +37,8 @@ class Currency extends Number
* If provided, Locale values must be a valid formatted locale string (e.g. 'en-GB', 'fr', uz-Arab-AF).
* Note that setting a locale will override any other settings defined in this class
* other than the currency code; or decimals (unless the decimals value is set to 0).
* @param bool $stripLeadingRLM remove leading RLM added with
* ICU 72.1+.
*
* @throws Exception If a provided locale code is not a valid format
*/
Expand All @@ -42,14 +48,16 @@ public function __construct(
bool $thousandsSeparator = true,
bool $currencySymbolPosition = self::LEADING_SYMBOL,
bool $currencySymbolSpacing = self::SYMBOL_WITHOUT_SPACING,
?string $locale = null
?string $locale = null,
bool $stripLeadingRLM = self::DEFAULT_STRIP_LEADING_RLM
) {
$this->setCurrencyCode($currencyCode);
$this->setThousandsSeparator($thousandsSeparator);
$this->setDecimals($decimals);
$this->setCurrencySymbolPosition($currencySymbolPosition);
$this->setCurrencySymbolSpacing($currencySymbolSpacing);
$this->setLocale($locale);
$this->stripLeadingRLM = $stripLeadingRLM;
}

public function setCurrencyCode(string $currencyCode): void
Expand All @@ -67,10 +75,15 @@ public function setCurrencySymbolSpacing(bool $currencySymbolSpacing = self::SYM
$this->currencySymbolSpacing = $currencySymbolSpacing;
}

public function setStripLeadingRLM(bool $stripLeadingRLM): void
{
$this->stripLeadingRLM = $stripLeadingRLM;
}

protected function getLocaleFormat(): string
{
$formatter = new Locale($this->fullLocale, NumberFormatter::CURRENCY);
$mask = $formatter->format();
$mask = $formatter->format($this->stripLeadingRLM);
if ($this->decimals === 0) {
$mask = (string) preg_replace('/\.0+/miu', '', $mask);
}
Expand Down
6 changes: 4 additions & 2 deletions src/PhpSpreadsheet/Style/NumberFormat/Wizard/Locale.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ public function __construct(?string $locale, int $style)
}
}

public function format(): string
public function format(bool $stripRlm = true): string
{
return $this->formatter->getPattern();
$str = $this->formatter->getPattern();

return ($stripRlm && substr($str, 0, 3) === "\xe2\x80\x8f") ? substr($str, 3) : $str;
}
}
44 changes: 40 additions & 4 deletions tests/PhpSpreadsheetTests/Style/NumberFormat/Wizard/AccountingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,25 @@ public static function providerAccounting(): array
public function testAccountingLocale(
string $expectedResult,
string $currencyCode,
string $locale
string $locale,
?bool $stripRLM = null
): void {
if (class_exists(NumberFormatter::class) === false) {
self::markTestSkipped('Intl extension is not available');
}

$wizard = new Accounting($currencyCode);
if ($stripRLM !== null) {
$wizard->setStripLeadingRLM($stripRLM);
}
$wizard->setLocale($locale);
self::assertSame($expectedResult, (string) $wizard);
}

public static function providerAccountingLocale(): array
{
// \u{a0} is non-breaking space
// \u{200e} is LRM (left-to-right mark)
return [
["[\$€-fy-NL]\u{a0}#,##0.00;([\$€-fy-NL]\u{a0}#,##0.00)", '€', 'fy-NL'],
["[\$€-nl-NL]\u{a0}#,##0.00;([\$€-nl-NL]\u{a0}#,##0.00)", '€', 'nl-NL'],
Expand All @@ -66,29 +72,59 @@ public static function providerAccountingLocale(): array
['[$$-en-CA]#,##0.00;([$$-en-CA]#,##0.00)', '$', 'en-ca'],
["#,##0.00\u{a0}[\$\$-fr-CA];(#,##0.00\u{a0}[\$\$-fr-CA])", '$', 'fr-ca'],
['[$¥-ja-JP]#,##0;([$¥-ja-JP]#,##0)', '¥', 'ja-JP'], // No decimals
["#,##0.000\u{a0}[\$د.ب‎-ar-BH]", 'د.ب‎', 'ar-BH'], // 3 decimals
["#,##0.000\u{a0}[\$د.ب\u{200e}-ar-BH]", "د.ب\u{200e}", 'ar-BH', true], // 3 decimals
];
}

public function testIcu721(): void
{
if (class_exists(NumberFormatter::class) === false) {
self::markTestSkipped('Intl extension is not available');
}

$currencyCode = "د.ب\u{200e}";
$locale = 'ar-BH';
$wizardFalse = new Accounting($currencyCode);
$wizardFalse->setStripLeadingRLM(false);
$wizardFalse->setLocale($locale);
$stringFalse = (string) $wizardFalse;
$wizardTrue = new Accounting($currencyCode);
$wizardTrue->setStripLeadingRLM(true);
$wizardTrue->setLocale($locale);
$stringTrue = (string) $wizardTrue;
$version = Accounting::icuVersion();
if ($version < 72.1) {
self::assertSame($stringFalse, $stringTrue);
} else {
self::assertSame("\u{200f}$stringTrue", $stringFalse);
}
}

/**
* @dataProvider providerAccountingLocaleNoDecimals
*/
public function testAccountingLocaleNoDecimals(
string $expectedResult,
string $currencyCode,
string $locale
string $locale,
?bool $stripRLM = null
): void {
if (class_exists(NumberFormatter::class) === false) {
self::markTestSkipped('Intl extension is not available');
}

$wizard = new Accounting($currencyCode, 0);
if ($stripRLM !== null) {
$wizard->setStripLeadingRLM($stripRLM);
}
$wizard->setLocale($locale);
self::assertSame($expectedResult, (string) $wizard);
}

public static function providerAccountingLocaleNoDecimals(): array
{
// \u{a0} is non-breaking space
// \u{200e} is LRM (left-to-right mark)
return [
["[\$€-fy-NL]\u{a0}#,##0;([\$€-fy-NL]\u{a0}#,##0)", '€', 'fy-NL'],
["[\$€-nl-NL]\u{a0}#,##0;([\$€-nl-NL]\u{a0}#,##0)", '€', 'nl-NL'],
Expand All @@ -98,7 +134,7 @@ public static function providerAccountingLocaleNoDecimals(): array
['[$$-en-CA]#,##0;([$$-en-CA]#,##0)', '$', 'en-ca'],
["#,##0\u{a0}[\$\$-fr-CA];(#,##0\u{a0}[\$\$-fr-CA])", '$', 'fr-ca'],
['[$¥-ja-JP]#,##0;([$¥-ja-JP]#,##0)', '¥', 'ja-JP'], // No decimals to truncate
["#,##0\u{a0}[\$د.ب‎-ar-BH]", 'د.ب‎', 'ar-BH'], // 3 decimals truncated to none
["#,##0\u{a0}[\$د.ب\u{200e}-ar-BH]", "د.ب\u{200e}", 'ar-BH', true], // 3 decimals truncated to none
];
}

Expand Down
45 changes: 41 additions & 4 deletions tests/PhpSpreadsheetTests/Style/NumberFormat/Wizard/CurrencyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use NumberFormatter;
use PhpOffice\PhpSpreadsheet\Exception;
use PhpOffice\PhpSpreadsheet\Style\NumberFormat\Wizard\Accounting;
use PhpOffice\PhpSpreadsheet\Style\NumberFormat\Wizard\Currency;
use PhpOffice\PhpSpreadsheet\Style\NumberFormat\Wizard\Number;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -43,19 +44,25 @@ public static function providerCurrency(): array
public function testCurrencyLocale(
string $expectedResult,
string $currencyCode,
string $locale
string $locale,
?bool $stripRLM = null
): void {
if (class_exists(NumberFormatter::class) === false) {
self::markTestSkipped('Intl extension is not available');
}

$wizard = new Currency($currencyCode);
if ($stripRLM !== null) {
$wizard->setStripLeadingRLM($stripRLM);
}
$wizard->setLocale($locale);
self::assertSame($expectedResult, (string) $wizard);
}

public static function providerCurrencyLocale(): array
{
// \u{a0} is non-breaking space
// \u{200e} is LRM (left-to-right mark)
return [
["[\$€-fy-NL]\u{a0}#,##0.00;[\$€-fy-NL]\u{a0}#,##0.00-", '€', 'fy-NL'], // Trailing negative
["[\$€-nl-NL]\u{a0}#,##0.00;[\$€-nl-NL]\u{a0}-#,##0.00", '€', 'nl-NL'], // Sign between currency and value
Expand All @@ -65,29 +72,59 @@ public static function providerCurrencyLocale(): array
['[$$-en-CA]#,##0.00', '$', 'en-ca'],
["#,##0.00\u{a0}[\$\$-fr-CA]", '$', 'fr-ca'], // Trailing currency code
['[$¥-ja-JP]#,##0', '¥', 'ja-JP'], // No decimals
["#,##0.000\u{a0}[\$د.ب‎-ar-BH]", 'د.ب‎', 'ar-BH'], // 3 decimals
["#,##0.000\u{a0}[\$د.ب\u{200e}-ar-BH]", "د.ب\u{200e}", 'ar-BH', true], // 3 decimals
];
}

public function testIcu721(): void
{
if (class_exists(NumberFormatter::class) === false) {
self::markTestSkipped('Intl extension is not available');
}

$currencyCode = "د.ب\u{200e}";
$locale = 'ar-BH';
$wizardFalse = new Currency($currencyCode);
$wizardFalse->setStripLeadingRLM(false);
$wizardFalse->setLocale($locale);
$stringFalse = (string) $wizardFalse;
$wizardTrue = new Currency($currencyCode);
$wizardTrue->setStripLeadingRLM(true);
$wizardTrue->setLocale($locale);
$stringTrue = (string) $wizardTrue;
$version = Accounting::icuVersion();
if ($version < 72.1) {
self::assertSame($stringFalse, $stringTrue);
} else {
self::assertSame("\u{200f}$stringTrue", $stringFalse);
}
}

/**
* @dataProvider providerCurrencyLocaleNoDecimals
*/
public function testCurrencyLocaleNoDecimals(
string $expectedResult,
string $currencyCode,
string $locale
string $locale,
?bool $stripRLM = null
): void {
if (class_exists(NumberFormatter::class) === false) {
self::markTestSkipped('Intl extension is not available');
}

$wizard = new Currency($currencyCode, 0);
if ($stripRLM !== null) {
$wizard->setStripLeadingRLM($stripRLM);
}
$wizard->setLocale($locale);
self::assertSame($expectedResult, (string) $wizard);
}

public static function providerCurrencyLocaleNoDecimals(): array
{
// \u{a0} is non-breaking space
// \u{200e} is LRM (left-to-right mark)
return [
["[\$€-fy-NL]\u{a0}#,##0;[\$€-fy-NL]\u{a0}#,##0-", '€', 'fy-NL'], // Trailing negative
["[\$€-nl-NL]\u{a0}#,##0;[\$€-nl-NL]\u{a0}-#,##0", '€', 'nl-NL'], // Sign between currency and value
Expand All @@ -97,7 +134,7 @@ public static function providerCurrencyLocaleNoDecimals(): array
['[$$-en-CA]#,##0', '$', 'en-ca'],
["#,##0\u{a0}[\$\$-fr-CA]", '$', 'fr-ca'], // Trailing currency code
['[$¥-ja-JP]#,##0', '¥', 'ja-JP'], // No decimals to truncate
["#,##0\u{a0}[\$د.ب‎-ar-BH]", 'د.ب‎', 'ar-BH'], // 3 decimals truncated to none
["#,##0\u{a0}[\$د.ب\u{200e}-ar-BH]", "د.ب\u{200e}", 'ar-BH', true], // 3 decimals truncated to none
];
}

Expand Down