Skip to content

Commit

Permalink
Floats must not be stored as locale dependent
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
adrilo committed May 5, 2021
1 parent eb84ec9 commit 8c1f0cc
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 3 deletions.
34 changes: 34 additions & 0 deletions src/Spout/Common/Helper/StringHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -68,4 +76,30 @@ public function getCharLastOccurrencePosition($char, $string)

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;
}
}
5 changes: 3 additions & 2 deletions src/Spout/Writer/ODS/Manager/WorksheetManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,9 @@ private function getCellXML(Cell $cell, $styleIndex, $numTimesValueRepeated)
$data .= '<text:p>' . $cell->getValue() . '</text:p>';
$data .= '</table:table-cell>';
} elseif ($cell->isNumeric()) {
$data .= ' office:value-type="float" calcext:value-type="float" office:value="' . $cell->getValue() . '">';
$data .= '<text:p>' . $cell->getValue() . '</text:p>';
$cellValue = $this->stringHelper->formatNumericValue($cell->getValue());
$data .= ' office:value-type="float" calcext:value-type="float" office:value="' . $cellValue . '">';
$data .= '<text:p>' . $cellValue . '</text:p>';
$data .= '</table:table-cell>';
} elseif ($cell->isError() && is_string($cell->getValueEvenIfError())) {
// only writes the error value if it's a string
Expand Down
2 changes: 1 addition & 1 deletion src/Spout/Writer/XLSX/Manager/WorksheetManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private function getCellXML($rowIndexOneBased, $columnIndexZeroBased, Cell $cell
} elseif ($cell->isBoolean()) {
$cellXML .= ' t="b"><v>' . (int) ($cell->getValue()) . '</v></c>';
} elseif ($cell->isNumeric()) {
$cellXML .= '><v>' . $cell->getValue() . '</v></c>';
$cellXML .= '><v>' . $this->stringHelper->formatNumericValue($cell->getValue()) . '</v></c>';
} elseif ($cell->isError() && is_string($cell->getValueEvenIfError())) {
// only writes the error value if it's a string
$cellXML .= ' t="e"><v>' . $cell->getValueEvenIfError() . '</v></c>';
Expand Down
46 changes: 46 additions & 0 deletions tests/Spout/Writer/ODS/WriterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,52 @@ public function testAddRowShouldSupportMultipleTypesOfData()
$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
*/
Expand Down
46 changes: 46 additions & 0 deletions tests/Spout/Writer/XLSX/WriterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,52 @@ public function testAddRowShouldSupportCellInError()
$this->assertInlineDataWasWrittenToSheet($fileName, 1, 't="e"><v>#DIV/0</v>');
}

/**
* @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
*/
Expand Down

0 comments on commit 8c1f0cc

Please sign in to comment.