Repository: box/spout (php)

Issue:
Floats are stored locale-aware
Hi,

I just got bitten by the problem described in #168, but mine is quite different. I was trying to store floats in the generated spreadsheet and kept getting errors. I started digging and discovered that floats in XML are stored in a locale-aware way. They are written as-is, but PHP can sometimes format them with a different decimal separator, other than a dot (eg. in Poland we are using a comma), if you start messing with `setlocale(LC_ALL, 'pl')`.

This results in: `<c r="C7" s="0"><v>3,5</v></c>`

If you manually change it to `<c r="C7" s="0"><v>3.5</v></c>` then Excel is happy and formats the value as a number.

The solution is quite simple, instead of doing:

```
} elseif ($cell->isNumeric()) {
        $cellXML .= '><v>' . $cell->getValue() . '</v></c>';
```

do

```
} elseif ($cell->isNumeric()) {
            $cellXML .= '><v>' . sprintf('%F', $cell->getValue()) . '</v></c>';
```

or even better

```
} elseif ($cell->isNumeric()) {
            $cellXML .= '><v>' . sprintf(\is_float($cell->getValue()) ? '%F' : '%d', $cell->getValue()) . '</v></c>';
```

or temporarily reset LC_ALL / LC_NUMERIC during the export.

I'm changing my code to use LC_TIME, because that's what I actually need right now, but you need to be aware of the unusual side-effects of locale for Spout.

Pull request:
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.

Fixes #601 

Patch:
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 .= '<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
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"><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>';


Test patch:
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"><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
      */