geckon01/PhpSpreadsheet
1
0
Fork 0
mirror of https://github.com/PHPOffice/PhpSpreadsheet.git synced 2026-09-15 04:26:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
* Php 8.3 Problem - RLM Added to NumberFormatter Currency - Minor Break

Fix #3571. This isn't truly a Php8.3 problem - it all depends on the version of ICU with which Php was linked. ICU 72.1 adds an RLM (right-to-left mark) character in some circumstances when creating a currency format. This broke some tests for the Currency and Accounting wizards, and can result in a difference in the appearance of some spreadsheet cells.

This PR changes code to strip out the RLM or not depending on a new property. The new property could default to true (so end-users will not see any change no matter what release of ICU is used), or false. For the latter, users might see a break, but my assumption is that the ICU developers have good reasons for their change, and it's probably best to go along with it. If users wish to retain the existing behavior, they can do so by adding the following code before setting the wizard's locale:
```php
$wizard->setStripLeadingRLM(true);
```

* Eliminate 2 Dead Statements

Correctly flagged by Scrutinizer.
This commit is contained in:
oleibman
2023-07-23 20:51:23 -07:00
committed by GitHub
parent 0bc8330b77
commit fb7b2ed219
5 changed files with 110 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/PhpSpreadsheet/Style/NumberFormat/Wizard/Accounting.php
+10 -4
View File
@@ -28,7 +28,8 @@ class Accounting extends Currency
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);
@@ -36,6 +37,7 @@ class Accounting extends Currency
$this->setCurrencySymbolPosition($currencySymbolPosition);
$this->setCurrencySymbolSpacing($currencySymbolSpacing);
$this->setLocale($locale);
$this->stripLeadingRLM = $stripLeadingRLM;
}
/**
@@ -44,16 +46,20 @@ class Accounting extends Currency
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);
}
@@ -61,7 +67,7 @@ class Accounting extends Currency
return str_replace('¤', $this->formatCurrencyCode(), $mask);
}
private function icuVersion(): float
public static function icuVersion(): float
{
[$major, $minor] = explode('.', INTL_ICU_VERSION);
src/PhpSpreadsheet/Style/NumberFormat/Wizard/Currency.php
+15 -2
View File
@@ -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
@@ -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
*/
@@ -42,7 +48,8 @@ class Currency extends Number
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);
@@ -50,6 +57,7 @@ class Currency extends Number
$this->setCurrencySymbolPosition($currencySymbolPosition);
$this->setCurrencySymbolSpacing($currencySymbolSpacing);
$this->setLocale($locale);
$this->stripLeadingRLM = $stripLeadingRLM;
}
public function setCurrencyCode(string $currencyCode): void
@@ -67,10 +75,15 @@ class Currency extends Number
$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);
}
src/PhpSpreadsheet/Style/NumberFormat/Wizard/Locale.php
+4 -2
View File
@@ -30,8 +30,10 @@ final class Locale
}
}
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;
}
}
tests/PhpSpreadsheetTests/Style/NumberFormat/Wizard/AccountingTest.php
+40 -4
View File
@@ -44,19 +44,25 @@ class AccountingTest extends TestCase
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'],
@@ -66,29 +72,59 @@ class AccountingTest extends TestCase
['[$$-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'],
@@ -98,7 +134,7 @@ class AccountingTest extends TestCase
['[$$-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
];
}
tests/PhpSpreadsheetTests/Style/NumberFormat/Wizard/CurrencyTest.php
+41 -4
View File
@@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Style\NumberFormat\Wizard;
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;
@@ -43,19 +44,25 @@ class CurrencyTest extends TestCase
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
@@ -65,29 +72,59 @@ class CurrencyTest extends TestCase
['[$$-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
@@ -97,7 +134,7 @@ class CurrencyTest extends TestCase
['[$$-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
];
}