From 8c1f0cc447fd8db57308552c2413fc5d5ac9e5f6 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Tue, 4 May 2021 19:33:24 +0200 Subject: [PATCH] Floats must not be stored as locale dependent Floats are currently stored formatted per the locale setting. This leads to different values being written whether the locale uses "." or "," for the decimal point for instance. This poses a problem as floats must be stored using "." as the decimal point to be valid. This commit ensures that the floats are stored correctly by forcing the formatting of the value. --- src/Spout/Common/Helper/StringHelper.php | 34 ++++++++++++++ .../Writer/ODS/Manager/WorksheetManager.php | 5 +- .../Writer/XLSX/Manager/WorksheetManager.php | 2 +- tests/Spout/Writer/ODS/WriterTest.php | 46 +++++++++++++++++++ tests/Spout/Writer/XLSX/WriterTest.php | 46 +++++++++++++++++++ 5 files changed, 130 insertions(+), 3 deletions(-) diff --git a/src/Spout/Common/Helper/StringHelper.php b/src/Spout/Common/Helper/StringHelper.php index 0906132..6256b1e 100644 --- a/src/Spout/Common/Helper/StringHelper.php +++ b/src/Spout/Common/Helper/StringHelper.php @@ -13,12 +13,20 @@ class StringHelper /** @var bool Whether the mbstring extension is loaded */ protected $hasMbstringSupport; + /** @var bool Whether the code is running with PHP7 or older versions */ + private $isRunningPhp7OrOlder; + + /** @var array Locale info, used for number formatting */ + private $localeInfo; + /** * */ public function __construct() { $this->hasMbstringSupport = \extension_loaded('mbstring'); + $this->isRunningPhp7OrOlder = \version_compare(PHP_VERSION, '8.0.0') < 0; + $this->localeInfo = \localeconv(); } /** @@ -68,4 +76,30 @@ class StringHelper return ($position !== false) ? $position : -1; } + + /** + * Formats a numeric value (int or float) in a way that's compatible with the expected spreadsheet format. + * + * Formatting of float values is locale dependent in PHP < 8. + * Thousands separators and decimal points vary from locale to locale (en_US: 12.34 vs pl_PL: 12,34). + * However, float values must be formatted with no thousands separator and a "." as decimal point + * to work properly. This method can be used to convert the value to the correct format before storing it. + * + * @see https://wiki.php.net/rfc/locale_independent_float_to_string for the changed behavior in PHP8. + * + * @param int|float $numericValue + * @return string + */ + public function formatNumericValue($numericValue) + { + if ($this->isRunningPhp7OrOlder && is_float($numericValue)) { + return str_replace( + [$this->localeInfo['thousands_sep'], $this->localeInfo['decimal_point']], + ['', '.'], + $numericValue + ); + } + + return $numericValue; + } } diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index e0366d1..7d7cb0e 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -231,8 +231,9 @@ class WorksheetManager implements WorksheetManagerInterface $data .= '' . $cell->getValue() . ''; $data .= ''; } elseif ($cell->isNumeric()) { - $data .= ' office:value-type="float" calcext:value-type="float" office:value="' . $cell->getValue() . '">'; - $data .= '' . $cell->getValue() . ''; + $cellValue = $this->stringHelper->formatNumericValue($cell->getValue()); + $data .= ' office:value-type="float" calcext:value-type="float" office:value="' . $cellValue . '">'; + $data .= '' . $cellValue . ''; $data .= ''; } elseif ($cell->isError() && is_string($cell->getValueEvenIfError())) { // only writes the error value if it's a string diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index 028bdef..61b93a1 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -240,7 +240,7 @@ EOD; } elseif ($cell->isBoolean()) { $cellXML .= ' t="b">' . (int) ($cell->getValue()) . ''; } elseif ($cell->isNumeric()) { - $cellXML .= '>' . $cell->getValue() . ''; + $cellXML .= '>' . $this->stringHelper->formatNumericValue($cell->getValue()) . ''; } elseif ($cell->isError() && is_string($cell->getValueEvenIfError())) { // only writes the error value if it's a string $cellXML .= ' t="e">' . $cell->getValueEvenIfError() . ''; diff --git a/tests/Spout/Writer/ODS/WriterTest.php b/tests/Spout/Writer/ODS/WriterTest.php index 31dcccf..61c1b9e 100644 --- a/tests/Spout/Writer/ODS/WriterTest.php +++ b/tests/Spout/Writer/ODS/WriterTest.php @@ -279,6 +279,52 @@ class WriterTest extends TestCase $this->assertValueWasWrittenToSheet($fileName, 1, 10.2); } + /** + * @return void + */ + public function testAddRowShouldSupportFloatValuesInDifferentLocale() + { + $previousLocale = \setlocale(LC_ALL, 0); + + try { + // Pick a supported locale whose decimal point is a comma. + // Installed locales differ from one system to another, so we can't pick + // a given locale. + $supportedLocales = explode("\n", shell_exec('locale -a')); + foreach ($supportedLocales as $supportedLocale) { + \setlocale(LC_ALL, $supportedLocale); + if (\localeconv()['decimal_point'] === ',') { + break; + } + } + $this->assertEquals(',', \localeconv()['decimal_point']); + + $cellValue = 1234.5; + var_dump("Cell value before: " . $cellValue); + $cellValue = str_replace( + [\localeconv()['thousands_sep'], \localeconv()['decimal_point']], + ['', '.'], + $cellValue + ); + var_dump("Cell value after: " . $cellValue); + var_dump("Thousands sep: " . \localeconv()['thousands_sep']); + var_dump("Decimal point: " . \localeconv()['decimal_point']); + + $fileName = 'test_add_row_should_support_float_values_in_different_locale.xlsx'; + $dataRows = $this->createRowsFromValues([ + [1234.5], + ]); + + $this->writeToODSFile($dataRows, $fileName); + + $this->assertValueWasNotWrittenToSheet($fileName, 1, "1234,5"); + $this->assertValueWasWrittenToSheet($fileName, 1, "1234.5"); + } finally { + // reset locale + \setlocale(LC_ALL, $previousLocale); + } + } + /** * @return array */ diff --git a/tests/Spout/Writer/XLSX/WriterTest.php b/tests/Spout/Writer/XLSX/WriterTest.php index 9cc157a..e2a8e4e 100644 --- a/tests/Spout/Writer/XLSX/WriterTest.php +++ b/tests/Spout/Writer/XLSX/WriterTest.php @@ -393,6 +393,52 @@ class WriterTest extends TestCase $this->assertInlineDataWasWrittenToSheet($fileName, 1, 't="e">#DIV/0'); } + /** + * @return void + */ + public function testAddRowShouldSupportFloatValuesInDifferentLocale() + { + $previousLocale = \setlocale(LC_ALL, 0); + $valueToWrite = 1234.5; // needs to be defined before changing the locale as PHP8 would expect 1234,5 + + try { + // Pick a supported locale whose decimal point is a comma. + // Installed locales differ from one system to another, so we can't pick + // a given locale. + $supportedLocales = explode("\n", shell_exec('locale -a')); + foreach ($supportedLocales as $supportedLocale) { + \setlocale(LC_ALL, $supportedLocale); + if (\localeconv()['decimal_point'] === ',') { + break; + } + } + $this->assertEquals(',', \localeconv()['decimal_point']); + + var_dump("Cell value before: " . $valueToWrite); + $cellValue = str_replace( + [\localeconv()['thousands_sep'], \localeconv()['decimal_point']], + ['', '.'], + $valueToWrite + ); + var_dump("Cell value after: " . $cellValue); + var_dump("Thousands sep: " . \localeconv()['thousands_sep']); + var_dump("Decimal point: " . \localeconv()['decimal_point']); + + $fileName = 'test_add_row_should_support_float_values_in_different_locale.xlsx'; + $dataRows = $this->createRowsFromValues([ + [$valueToWrite], + ]); + + $this->writeToXLSXFile($dataRows, $fileName, $shouldUseInlineStrings = false); + + $this->assertInlineDataWasNotWrittenToSheet($fileName, 1, "1234,5"); + $this->assertInlineDataWasWrittenToSheet($fileName, 1, "1234.5"); + } finally { + // reset locale + \setlocale(LC_ALL, $previousLocale); + } + } + /** * @return void */