WIP Minor Breaking Change to DefaultValueBinder

Fix #4522. Although this is technically a breaking change, it is expected that very few, if any, existing programs will be affected. Nevertheless, DefaultValueBinder is implicitly used by the vast majority of programs out there, and I am reluctant to install a breaking change for something so widespread. So I will save this change for the next breaking release, which should be PhpSpreadsheet 5.0.0. There is no current schedule for that release.

For strings consisting entirely of digits, DefaultValueBinder treats the value as a string if greater than PHP_INT_MAX, or an int otherwise. This prevents the loss of precision in large integers. This treatment dates back to PHPExcel, and has been in place since 2014.

There are several problems with this approach. Excel itself maintains
[15 digits of precision](https://support.microsoft.com/en-us/office/excel-specifications-and-limits-1672b34d-7043-467e-8e27-269d656771c3).
So, string-vs-int should be decided at 999_999_999_999_999. This is much lower than PHP_INT_MAX for 64-bit, and much higher than for 32-bit. DefaultValueBinder is changed to use that new limit.

A second problem is that DefaultValueBinder is only making this adjustment for positive integers. It is changed to test absolute value.

A third problem is that DefaultValueBinder is making this adjustment only for strings, so that if you pass the number in as an int, it will not be adjusted (and will lose precision). It is changed to apply the same test for int.

Author: oleibman

src/PhpSpreadsheet/Cell/DefaultValueBinder.php

@@ -2,6 +2,7 @@
 
 namespace PhpOffice\PhpSpreadsheet\Cell;
 
+use Composer\Pcre\Preg;
 use DateTimeInterface;
 use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
@@ -12,6 +13,9 @@
 
 class DefaultValueBinder implements IValueBinder
 {
+    //                            123 456 789 012 345
+    private const FIFTEEN_NINES = 999_999_999_999_999;
+
     /**
      * Bind value to a cell.
      *
@@ -49,6 +53,9 @@ public static function dataTypeForValue(mixed $value): string
         if ($value === null) {
             return DataType::TYPE_NULL;
         }
+        if (is_int($value) && abs($value) > self::FIFTEEN_NINES) {
+            return DataType::TYPE_STRING;
+        }
         if (is_float($value) || is_int($value)) {
             return DataType::TYPE_NUMERIC;
         }
@@ -89,13 +96,18 @@ public static function dataTypeForValue(mixed $value): string
 
             return DataType::TYPE_FORMULA;
         }
-        if (preg_match('/^[\+\-]?(\d+\.?\d*|\d*\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) {
+        if (Preg::isMatch('/^[\+\-]?(\d+\.?\d*|\d*\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) {
             $tValue = ltrim($value, '+-');
             if (strlen($tValue) > 1 && $tValue[0] === '0' && $tValue[1] !== '.') {
                 return DataType::TYPE_STRING;
-            } elseif ((!str_contains($value, '.')) && ($value > PHP_INT_MAX)) {
-                return DataType::TYPE_STRING;
-            } elseif (!is_numeric($value)) {
+            }
+            if (!Preg::isMatch('/[eE.]/', $value)) {
+                $aValue = abs((float) $value);
+                if ($aValue > self::FIFTEEN_NINES) {
+                    return DataType::TYPE_STRING;
+                }
+            }
+            if (!is_numeric($value)) {
                 return DataType::TYPE_STRING;
             }
 

tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php

@@ -8,16 +8,21 @@
 use PhpOffice\PhpSpreadsheet\Shared\File;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 class FloatsRetainedTest extends TestCase
 {
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerIntyFloatsRetainedByWriter')]
-    public function testIntyFloatsRetainedByWriter(float|int $value): void
+    #[DataProvider('providerIntyFloatsRetainedByWriter')]
+    public function testIntyFloatsRetainedByWriter(float|int $value, mixed $expected = null): void
     {
+        if ($expected === null) {
+            $expected = $value;
+        }
         $outputFilename = File::temporaryFilename();
         $spreadsheet = new Spreadsheet();
-        $spreadsheet->getActiveSheet()->getCell('A1')->setValue($value);
+        $spreadsheet->getActiveSheet()
+            ->getCell('A1')->setValue($value);
 
         $writer = new Writer($spreadsheet);
         $writer->save($outputFilename);
@@ -27,7 +32,11 @@ public function testIntyFloatsRetainedByWriter(float|int $value): void
         $spreadsheet2 = $reader->load($outputFilename);
         unlink($outputFilename);
 
-        self::assertSame($value, $spreadsheet2->getActiveSheet()->getCell('A1')->getValue());
+        self::assertSame(
+            $expected,
+            $spreadsheet2->getActiveSheet()
+                ->getCell('A1')->getValue()
+        );
         $spreadsheet2->disconnectWorksheets();
     }
 
@@ -44,10 +53,10 @@ public static function providerIntyFloatsRetainedByWriter(): array
             [1.3e-10],
             [1e10],
             [3.00000000000000000001],
-            [99999999999999999],
-            [99999999999999999.0],
-            [999999999999999999999999999999999999999999],
-            [999999999999999999999999999999999999999999.0],
+            'int but too much precision for Excel' => [99_999_999_999_999_999, '99999999999999999'],
+            [99_999_999_999_999_999.0],
+            'int > PHP_INT_MAX so stored as float' => [999_999_999_999_999_999_999_999_999_999_999_999_999_999],
+            [999_999_999_999_999_999_999_999_999_999_999_999_999_999.0],
         ];
     }
 }