From ef81f199966286b949fd1a81fab6cef845204550 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 20 May 2025 23:35:50 -0700 Subject: [PATCH 1/2] More Precision for Float to String Casts Fix #3899. Supersedes PR #4476, which will be changed to draft status and closed if this PR is merged. A standard cast from float to string in PHP can drop trailing decimal positions. This can lead to problems above and beyond the usual problems associated with floating point. See the superseded PR for a more complete explanation. `StringHelper::convertToString` is changed for how it handles floats. It will now do separate casts for the whole and decimal parts, and then combine the results. This affects `Cell::getValueString` and `Cell::getCalculatedValueString`. Xlsx Writer will now invoke `convertToString` before writing a float to Xml. Ods Writer already uses `getValueString`, so no change is needed there. Xls Writer writes its float values in binary, so no change is needed there. Tests are added for all 3 writers. Aside from fixing some problems, it might appear that this change introduces some new problems. For instance, setting a cell to `12345.6789` will now result in `12345.67890000000079` in the Xml. This difference is an illusion, merely a consequence of floating point rounding. If you run the following check under PhpUnit, it will pass: ```php self::assertSame(12345.6789, 12345.67890000000079); ``` --- src/PhpSpreadsheet/Shared/Date.php | 2 +- src/PhpSpreadsheet/Shared/StringHelper.php | 22 +++++++- src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 12 ++--- .../Functions/MathTrig/TruncTest.php | 5 +- .../Functions/TextData/ArrayToTextTest.php | 4 ++ .../Writer/Ods/ContentTest.php | 50 +++++++++++++------ .../Writer/Ods/MicrosecondsTest.php | 50 +++++++++++++++++++ .../Writer/Xls/MicrosecondsTest.php | 47 +++++++++++++++++ .../Writer/Xlsx/MicrosecondsTest.php | 47 +++++++++++++++++ 9 files changed, 212 insertions(+), 27 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Ods/MicrosecondsTest.php create mode 100644 tests/PhpSpreadsheetTests/Writer/Xls/MicrosecondsTest.php create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/MicrosecondsTest.php diff --git a/src/PhpSpreadsheet/Shared/Date.php b/src/PhpSpreadsheet/Shared/Date.php index e8f23164f3..3767a60118 100644 --- a/src/PhpSpreadsheet/Shared/Date.php +++ b/src/PhpSpreadsheet/Shared/Date.php @@ -471,7 +471,7 @@ public static function stringToExcel(string $dateValue): bool|float if (strlen($dateValue) < 2) { return false; } - if (!preg_match('/^(\d{1,4}[ \.\/\-][A-Z]{3,9}([ \.\/\-]\d{1,4})?|[A-Z]{3,9}[ \.\/\-]\d{1,4}([ \.\/\-]\d{1,4})?|\d{1,4}[ \.\/\-]\d{1,4}([ \.\/\-]\d{1,4})?)( \d{1,2}:\d{1,2}(:\d{1,2})?)?$/iu', $dateValue)) { + if (!preg_match('/^(\d{1,4}[ \.\/\-][A-Z]{3,9}([ \.\/\-]\d{1,4})?|[A-Z]{3,9}[ \.\/\-]\d{1,4}([ \.\/\-]\d{1,4})?|\d{1,4}[ \.\/\-]\d{1,4}([ \.\/\-]\d{1,4})?)( \d{1,2}:\d{1,2}(:\d{1,2}([.]\d+))?)?$/iu', $dateValue)) { return false; } diff --git a/src/PhpSpreadsheet/Shared/StringHelper.php b/src/PhpSpreadsheet/Shared/StringHelper.php index 40adfd8c96..3e4311832c 100644 --- a/src/PhpSpreadsheet/Shared/StringHelper.php +++ b/src/PhpSpreadsheet/Shared/StringHelper.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Shared; +use Composer\Pcre\Preg; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; use Stringable; @@ -494,9 +495,9 @@ public static function mbStrSplit(string $string): array { // Split at all position not after the start: ^ // and not before the end: $ - $split = preg_split('/(?writeElement('v', "$cellValue"); + $objWriter->writeElement('v', $result); } private function writeCellBoolean(XMLWriter $objWriter, string $mappedType, bool $cellValue): void diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/TruncTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/TruncTest.php index 0f6bd0b977..31a1607945 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/TruncTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/TruncTest.php @@ -54,8 +54,9 @@ public function testTooMuchPrecision(mixed $expectedResult, float|int|string $va $sheet = $this->getSheet(); $sheet->getCell('E1')->setValue($value); $sheet->getCell('E2')->setValue("=TRUNC(E1,$digits)"); - $result = $sheet->getCell('E2')->getCalculatedValueString(); - self::assertSame($expectedResult, $result); + /** @var float|string */ + $result = $sheet->getCell('E2')->getCalculatedValue(); + self::assertSame($expectedResult, (string) $result); } public static function providerTooMuchPrecision(): array diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ArrayToTextTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ArrayToTextTest.php index 3ec22ddda4..2979463327 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ArrayToTextTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ArrayToTextTest.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\TextData; +use PhpOffice\PhpSpreadsheet\Shared\StringHelper; use PHPUnit\Framework\Attributes\DataProvider; class ArrayToTextTest extends AllSetupTeardown @@ -17,6 +18,9 @@ public function testArrayToText(string $expectedResult, array $testData, int $mo $worksheet->getCell('H1')->setValue("=ARRAYTOTEXT(A1:C5, {$mode})"); $result = $worksheet->getCell('H1')->getCalculatedValue(); + $b1SimpleCast = '12345.6789'; + $b1AccurateCast = StringHelper::convertToString(12345.6789); + $expectedResult = str_replace($b1SimpleCast, $b1AccurateCast, $expectedResult); self::assertSame($expectedResult, $result); } diff --git a/tests/PhpSpreadsheetTests/Writer/Ods/ContentTest.php b/tests/PhpSpreadsheetTests/Writer/Ods/ContentTest.php index 1bc54c3f62..9ec2138c89 100644 --- a/tests/PhpSpreadsheetTests/Writer/Ods/ContentTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Ods/ContentTest.php @@ -32,7 +32,9 @@ protected function setUp(): void parent::setUp(); $this->compatibilityMode = Functions::getCompatibilityMode(); - Functions::setCompatibilityMode(Functions::COMPATIBILITY_OPENOFFICE); + Functions::setCompatibilityMode( + Functions::COMPATIBILITY_OPENOFFICE + ); } protected function tearDown(): void @@ -57,6 +59,8 @@ public function testWriteSpreadsheet(): void $worksheet1 = $workbook->getActiveSheet(); $worksheet1->setCellValue('A1', 1); // Number $worksheet1->setCellValue('B1', 12345.6789); // Number + $b1SimpleCast = '12345.6789'; + $b1AccurateCast = StringHelper::convertToString(12345.6789); $worksheet1->setCellValue('C1', '1'); // Number without cast $worksheet1->setCellValueExplicit('D1', '01234', DataType::TYPE_STRING); // Number casted to string $worksheet1->setCellValue('E1', 'Lorem ipsum'); // String @@ -74,6 +78,11 @@ public function testWriteSpreadsheet(): void $worksheet1->getStyle('D2') ->getNumberFormat() ->setFormatCode(NumberFormat::FORMAT_DATE_DATETIME); + /** @var float */ + $d2SimpleCast = $worksheet1->getCell('D2')->getValue(); + $d2SimpleCast = (string) $d2SimpleCast; + $d2AccurateCast = $worksheet1 + ->getCell('D2')->getValueString(); $worksheet1->setCellValueExplicit('F1', null, DataType::TYPE_ERROR); $worksheet1->setCellValueExplicit('G1', 'Lorem ipsum', DataType::TYPE_INLINE); @@ -85,12 +94,17 @@ public function testWriteSpreadsheet(): void $worksheet1->getStyle('C1')->getFont()->setSize(14); $worksheet1->getStyle('C1')->getFont()->setColor(new Color(Color::COLOR_BLUE)); - $worksheet1->getStyle('C1')->getFill()->setFillType(Fill::FILL_SOLID); - $worksheet1->getStyle('C1')->getFill()->setStartColor(new Color(Color::COLOR_RED)); + $worksheet1->getStyle('C1') + ->getFill()->setFillType(Fill::FILL_SOLID); + $worksheet1->getStyle('C1') + ->getFill()->setStartColor(new Color(Color::COLOR_RED)); - $worksheet1->getStyle('C1')->getFont()->setUnderline(Font::UNDERLINE_SINGLE); - $worksheet1->getStyle('C2')->getFont()->setUnderline(Font::UNDERLINE_DOUBLE); - $worksheet1->getStyle('D2')->getFont()->setUnderline(Font::UNDERLINE_NONE); + $worksheet1->getStyle('C1')->getFont() + ->setUnderline(Font::UNDERLINE_SINGLE); + $worksheet1->getStyle('C2')->getFont() + ->setUnderline(Font::UNDERLINE_DOUBLE); + $worksheet1->getStyle('D2')->getFont() + ->setUnderline(Font::UNDERLINE_NONE); // Worksheet 2 $worksheet2 = $workbook->createSheet(); @@ -101,7 +115,12 @@ public function testWriteSpreadsheet(): void $content = new Content(new Ods($workbook)); $xml = $content->write(); - self::assertXmlStringEqualsXmlFile($this->samplesPath . '/content-with-data.xml', $xml); + $xmlFile = $this->samplesPath . '/content-with-data.xml'; + $xmlContents = (string) file_get_contents($xmlFile); + $xmlContents = str_replace($b1SimpleCast, $b1AccurateCast, $xmlContents); + $xmlContents = str_replace($d2SimpleCast, $d2AccurateCast, $xmlContents); + self::assertXmlStringEqualsXmlString($xmlContents, $xml); + $workbook->disconnectWorksheets(); } public function testWriteWithHiddenWorksheet(): void @@ -124,19 +143,21 @@ public function testWriteWithHiddenWorksheet(): void $xml = $content->write(); self::assertXmlStringEqualsXmlFile($this->samplesPath . '/content-hidden-worksheet.xml', $xml); + $workbook->disconnectWorksheets(); } public function testWriteBorderStyle(): void { $spreadsheet = new Spreadsheet(); - $spreadsheet->getActiveSheet()->getStyle('A1:B2')->applyFromArray([ - 'borders' => [ - 'outline' => [ - 'borderStyle' => Border::BORDER_THICK, - 'color' => ['argb' => 'AA22DD00'], + $spreadsheet->getActiveSheet() + ->getStyle('A1:B2')->applyFromArray([ + 'borders' => [ + 'outline' => [ + 'borderStyle' => Border::BORDER_THICK, + 'color' => ['argb' => 'AA22DD00'], + ], ], - ], - ]); + ]); $content = new Content(new Ods($spreadsheet)); $xml = $content->write(); @@ -161,5 +182,6 @@ public function testWriteBorderStyle(): void } } } + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Writer/Ods/MicrosecondsTest.php b/tests/PhpSpreadsheetTests/Writer/Ods/MicrosecondsTest.php new file mode 100644 index 0000000000..70db6fd806 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Ods/MicrosecondsTest.php @@ -0,0 +1,50 @@ +getActiveSheet(); + $sheet->setCellValue('A1', Date::dateTimeToExcel($originalDateTime)); + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Ods'); + $spreadsheet->disconnectWorksheets(); + + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + $rsheet->getStyle('A1') + ->getNumberFormat() + ->setFormatCode('yyyy-mm-dd hh:mm:ss.000'); + /** @var float */ + $reread = $rsheet->getCell('A1')->getValue(); + $temp = Date::excelToDateTimeObject($reread) + ->format('Y-m-d H:i:s.u'); + self::assertSame("{$date} {$time}.000000", $temp, 'round trip works with float value'); + $formatted = $rsheet->getCell('A1')->getFormattedValue(); + self::assertSame("{$date} {$time}.000", $formatted, 'round trip works with formatted value'); + /** @var float */ + $temp = Date::stringToExcel($formatted); + $temp = Date::excelToDateTimeObject($temp) + ->format('Y-m-d H:i:s.u'); + self::assertSame("{$date} {$time}.000000", $temp, 'round trip works using stringToExcel on formatted value'); + + $reloadedSpreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/PhpSpreadsheetTests/Writer/Xls/MicrosecondsTest.php b/tests/PhpSpreadsheetTests/Writer/Xls/MicrosecondsTest.php new file mode 100644 index 0000000000..130d6a401e --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xls/MicrosecondsTest.php @@ -0,0 +1,47 @@ +getActiveSheet(); + $sheet->setCellValue('A1', Date::dateTimeToExcel($originalDateTime)); + $sheet->getStyle('A1') + ->getNumberFormat() + ->setFormatCode('yyyy-mm-dd hh:mm:ss.000'); + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xls'); + $spreadsheet->disconnectWorksheets(); + + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + /** @var float */ + $reread = $rsheet->getCell('A1')->getValue(); + $temp = Date::excelToDateTimeObject($reread) + ->format('Y-m-d H:i:s.u'); + self::assertSame("{$date} {$time}.000000", $temp, 'round trip works with float value'); + $formatted = $rsheet->getCell('A1')->getFormattedValue(); + self::assertSame("{$date} {$time}.000", $formatted, 'round trip works with formatted value'); + /** @var float */ + $temp = Date::stringToExcel($formatted); + $temp = Date::excelToDateTimeObject($temp) + ->format('Y-m-d H:i:s.u'); + self::assertSame("{$date} {$time}.000000", $temp, 'round trip works using stringToExcel on formatted value'); + + $reloadedSpreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/MicrosecondsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/MicrosecondsTest.php new file mode 100644 index 0000000000..bdc50bf8cd --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/MicrosecondsTest.php @@ -0,0 +1,47 @@ +getActiveSheet(); + $sheet->setCellValue('A1', Date::dateTimeToExcel($originalDateTime)); + $sheet->getStyle('A1') + ->getNumberFormat() + ->setFormatCode('yyyy-mm-dd hh:mm:ss.000'); + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + /** @var float */ + $reread = $rsheet->getCell('A1')->getValue(); + $temp = Date::excelToDateTimeObject($reread) + ->format('Y-m-d H:i:s.u'); + self::assertSame("{$date} {$time}.000000", $temp, 'round trip works with float value'); + $formatted = $rsheet->getCell('A1')->getFormattedValue(); + self::assertSame("{$date} {$time}.000", $formatted, 'round trip works with formatted value'); + /** @var float */ + $temp = Date::stringToExcel($formatted); + $temp = Date::excelToDateTimeObject($temp) + ->format('Y-m-d H:i:s.u'); + self::assertSame("{$date} {$time}.000000", $temp, 'round trip works using stringToExcel on formatted value'); + + $reloadedSpreadsheet->disconnectWorksheets(); + } +} From dbca68c1ab763c4f6bb11653f115040a2603f00d Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 24 May 2025 22:28:35 -0700 Subject: [PATCH 2/2] Additional Test --- CHANGELOG.md | 1 + .../FloatImprecisionTest.php | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/PhpSpreadsheetTests/FloatImprecisionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 3420e60a45..76f1bc1f68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Copy Styles after insertNewColumnBefore. [Issue #1425](https://github.com/PHPOffice/PhpSpreadsheet/issues/1425) [PR #4468](https://github.com/PHPOffice/PhpSpreadsheet/pull/4468) - Xls Writer Treat Hyperlink Starting with # as Internal. [Issue #56](https://github.com/PHPOffice/PhpSpreadsheet/issues/56) [PR #4453](https://github.com/PHPOffice/PhpSpreadsheet/pull/4453) - ODS Handling of Ceiling and Floor. [Issue #407](https://github.com/PHPOffice/PhpSpreadsheet/issues/407) [PR #4466](https://github.com/PHPOffice/PhpSpreadsheet/pull/4466) +- More Precision for Float to String Casts. [Issue #3899](https://github.com/PHPOffice/PhpSpreadsheet/issues/3899) [PR #4479](https://github.com/PHPOffice/PhpSpreadsheet/pull/4479) ## 2025-04-16 - 4.2.0 diff --git a/tests/PhpSpreadsheetTests/FloatImprecisionTest.php b/tests/PhpSpreadsheetTests/FloatImprecisionTest.php new file mode 100644 index 0000000000..aa960486b3 --- /dev/null +++ b/tests/PhpSpreadsheetTests/FloatImprecisionTest.php @@ -0,0 +1,24 @@ +