From 170b058049797bd7cc623eb2fab59b7de32ec3ff Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 4 Jul 2025 20:58:03 -0700 Subject: [PATCH] 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. --- .../Cell/DefaultValueBinder.php | 20 ++++++++++++--- .../Writer/Xlsx/FloatsRetainedTest.php | 25 +++++++++++++------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php index 10c5c93c59..f636befcfb 100644 --- a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php +++ b/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; } diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php index c10a1b9b47..fa9205154c 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php +++ b/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], ]; } }