Skip to content

Commit ef81f19

Browse files
oleibmanoleibman
committed
More Precision for Float to String Casts
Fix PHPOffice#3899. Supersedes PR PHPOffice#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); ```
1 parent 85a9a39 commit ef81f19

File tree

9 files changed

+212
-27
lines changed

9 files changed

+212
-27
lines changed

‎src/PhpSpreadsheet/Shared/Date.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ public static function stringToExcel(string $dateValue): bool|float
471471
if (strlen($dateValue) < 2) {
472472
return false;
473473
}
474-
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)) {
474+
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)) {
475475
return false;
476476
}
477477

‎src/PhpSpreadsheet/Shared/StringHelper.php‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Shared;
44

5+
use Composer\Pcre\Preg;
56
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
67
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
78
use Stringable;
@@ -494,9 +495,9 @@ public static function mbStrSplit(string $string): array
494495
{
495496
// Split at all position not after the start: ^
496497
// and not before the end: $
497-
$split = preg_split('/(?<!^)(?!$)/u', $string);
498+
$split = Preg::split('/(?<!^)(?!$)/u', $string);
498499

499-
return ($split === false) ? [] : $split;
500+
return $split;
500501
}
501502

502503
/**
@@ -530,6 +531,7 @@ private static function getLocaleValue(string $key, string $altKey, string $defa
530531
$localeconv = localeconv();
531532
$rslt = $localeconv[$key];
532533
// win-1252 implements Euro as 0x80 plus other symbols
534+
// Not suitable for Composer\Pcre\Preg
533535
if (preg_match('//u', $rslt) !== 1) {
534536
$rslt = '';
535537
}
@@ -659,6 +661,22 @@ public static function convertToString(mixed $value, bool $throw = true, string
659661
if ($convertBool && is_bool($value)) {
660662
return $value ? Calculation::getTRUE() : Calculation::getFALSE();
661663
}
664+
if (is_float($value)) {
665+
$string = (string) $value;
666+
// look out for scientific notation
667+
if (!Preg::isMatch('/[^-+0-9.]/', $string)) {
668+
$minus = $value < 0 ? '-' : '';
669+
$positive = abs($value);
670+
$floor = floor($positive);
671+
$oldFrac = (string) ($positive - $floor);
672+
$frac = Preg::replace('/^0[.](\d+)$/', '$1', $oldFrac);
673+
if ($frac !== $oldFrac) {
674+
return "$minus$floor.$frac";
675+
}
676+
}
677+
678+
return $string;
679+
}
662680
if ($value === null || is_scalar($value) || $value instanceof Stringable) {
663681
return (string) $value;
664682
}

‎src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,15 +1463,11 @@ private function writeCellString(XMLWriter $objWriter, string $mappedType, RichT
14631463

14641464
private function writeCellNumeric(XMLWriter $objWriter, float|int $cellValue): void
14651465
{
1466-
//force a decimal to be written if the type is float
1467-
if (is_float($cellValue)) {
1468-
// force point as decimal separator in case current locale uses comma
1469-
$cellValue = str_replace(',', '.', (string) $cellValue);
1470-
if (!str_contains($cellValue, '.')) {
1471-
$cellValue = $cellValue . '.0';
1472-
}
1466+
$result = StringHelper::convertToString($cellValue);
1467+
if (is_float($cellValue) && !str_contains($result, '.')) {
1468+
$result .= '.0';
14731469
}
1474-
$objWriter->writeElement('v', "$cellValue");
1470+
$objWriter->writeElement('v', $result);
14751471
}
14761472

14771473
private function writeCellBoolean(XMLWriter $objWriter, string $mappedType, bool $cellValue): void

‎tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/TruncTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ public function testTooMuchPrecision(mixed $expectedResult, float|int|string $va
5454
$sheet = $this->getSheet();
5555
$sheet->getCell('E1')->setValue($value);
5656
$sheet->getCell('E2')->setValue("=TRUNC(E1,$digits)");
57-
$result = $sheet->getCell('E2')->getCalculatedValueString();
58-
self::assertSame($expectedResult, $result);
57+
/** @var float|string */
58+
$result = $sheet->getCell('E2')->getCalculatedValue();
59+
self::assertSame($expectedResult, (string) $result);
5960
}
6061

6162
public static function providerTooMuchPrecision(): array

‎tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ArrayToTextTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\TextData;
66

7+
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
78
use PHPUnit\Framework\Attributes\DataProvider;
89

910
class ArrayToTextTest extends AllSetupTeardown
@@ -17,6 +18,9 @@ public function testArrayToText(string $expectedResult, array $testData, int $mo
1718
$worksheet->getCell('H1')->setValue("=ARRAYTOTEXT(A1:C5, {$mode})");
1819

1920
$result = $worksheet->getCell('H1')->getCalculatedValue();
21+
$b1SimpleCast = '12345.6789';
22+
$b1AccurateCast = StringHelper::convertToString(12345.6789);
23+
$expectedResult = str_replace($b1SimpleCast, $b1AccurateCast, $expectedResult);
2024
self::assertSame($expectedResult, $result);
2125
}
2226

‎tests/PhpSpreadsheetTests/Writer/Ods/ContentTest.php‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ protected function setUp(): void
3232
parent::setUp();
3333

3434
$this->compatibilityMode = Functions::getCompatibilityMode();
35-
Functions::setCompatibilityMode(Functions::COMPATIBILITY_OPENOFFICE);
35+
Functions::setCompatibilityMode(
36+
Functions::COMPATIBILITY_OPENOFFICE
37+
);
3638
}
3739

3840
protected function tearDown(): void
@@ -57,6 +59,8 @@ public function testWriteSpreadsheet(): void
5759
$worksheet1 = $workbook->getActiveSheet();
5860
$worksheet1->setCellValue('A1', 1); // Number
5961
$worksheet1->setCellValue('B1', 12345.6789); // Number
62+
$b1SimpleCast = '12345.6789';
63+
$b1AccurateCast = StringHelper::convertToString(12345.6789);
6064
$worksheet1->setCellValue('C1', '1'); // Number without cast
6165
$worksheet1->setCellValueExplicit('D1', '01234', DataType::TYPE_STRING); // Number casted to string
6266
$worksheet1->setCellValue('E1', 'Lorem ipsum'); // String
@@ -74,6 +78,11 @@ public function testWriteSpreadsheet(): void
7478
$worksheet1->getStyle('D2')
7579
->getNumberFormat()
7680
->setFormatCode(NumberFormat::FORMAT_DATE_DATETIME);
81+
/** @var float */
82+
$d2SimpleCast = $worksheet1->getCell('D2')->getValue();
83+
$d2SimpleCast = (string) $d2SimpleCast;
84+
$d2AccurateCast = $worksheet1
85+
->getCell('D2')->getValueString();
7786

7887
$worksheet1->setCellValueExplicit('F1', null, DataType::TYPE_ERROR);
7988
$worksheet1->setCellValueExplicit('G1', 'Lorem ipsum', DataType::TYPE_INLINE);
@@ -85,12 +94,17 @@ public function testWriteSpreadsheet(): void
8594
$worksheet1->getStyle('C1')->getFont()->setSize(14);
8695
$worksheet1->getStyle('C1')->getFont()->setColor(new Color(Color::COLOR_BLUE));
8796

88-
$worksheet1->getStyle('C1')->getFill()->setFillType(Fill::FILL_SOLID);
89-
$worksheet1->getStyle('C1')->getFill()->setStartColor(new Color(Color::COLOR_RED));
97+
$worksheet1->getStyle('C1')
98+
->getFill()->setFillType(Fill::FILL_SOLID);
99+
$worksheet1->getStyle('C1')
100+
->getFill()->setStartColor(new Color(Color::COLOR_RED));
90101

91-
$worksheet1->getStyle('C1')->getFont()->setUnderline(Font::UNDERLINE_SINGLE);
92-
$worksheet1->getStyle('C2')->getFont()->setUnderline(Font::UNDERLINE_DOUBLE);
93-
$worksheet1->getStyle('D2')->getFont()->setUnderline(Font::UNDERLINE_NONE);
102+
$worksheet1->getStyle('C1')->getFont()
103+
->setUnderline(Font::UNDERLINE_SINGLE);
104+
$worksheet1->getStyle('C2')->getFont()
105+
->setUnderline(Font::UNDERLINE_DOUBLE);
106+
$worksheet1->getStyle('D2')->getFont()
107+
->setUnderline(Font::UNDERLINE_NONE);
94108

95109
// Worksheet 2
96110
$worksheet2 = $workbook->createSheet();
@@ -101,7 +115,12 @@ public function testWriteSpreadsheet(): void
101115
$content = new Content(new Ods($workbook));
102116
$xml = $content->write();
103117

104-
self::assertXmlStringEqualsXmlFile($this->samplesPath . '/content-with-data.xml', $xml);
118+
$xmlFile = $this->samplesPath . '/content-with-data.xml';
119+
$xmlContents = (string) file_get_contents($xmlFile);
120+
$xmlContents = str_replace($b1SimpleCast, $b1AccurateCast, $xmlContents);
121+
$xmlContents = str_replace($d2SimpleCast, $d2AccurateCast, $xmlContents);
122+
self::assertXmlStringEqualsXmlString($xmlContents, $xml);
123+
$workbook->disconnectWorksheets();
105124
}
106125

107126
public function testWriteWithHiddenWorksheet(): void
@@ -124,19 +143,21 @@ public function testWriteWithHiddenWorksheet(): void
124143
$xml = $content->write();
125144

126145
self::assertXmlStringEqualsXmlFile($this->samplesPath . '/content-hidden-worksheet.xml', $xml);
146+
$workbook->disconnectWorksheets();
127147
}
128148

129149
public function testWriteBorderStyle(): void
130150
{
131151
$spreadsheet = new Spreadsheet();
132-
$spreadsheet->getActiveSheet()->getStyle('A1:B2')->applyFromArray([
133-
'borders' => [
134-
'outline' => [
135-
'borderStyle' => Border::BORDER_THICK,
136-
'color' => ['argb' => 'AA22DD00'],
152+
$spreadsheet->getActiveSheet()
153+
->getStyle('A1:B2')->applyFromArray([
154+
'borders' => [
155+
'outline' => [
156+
'borderStyle' => Border::BORDER_THICK,
157+
'color' => ['argb' => 'AA22DD00'],
158+
],
137159
],
138-
],
139-
]);
160+
]);
140161

141162
$content = new Content(new Ods($spreadsheet));
142163
$xml = $content->write();
@@ -161,5 +182,6 @@ public function testWriteBorderStyle(): void
161182
}
162183
}
163184
}
185+
$spreadsheet->disconnectWorksheets();
164186
}
165187
}

‎tests/PhpSpreadsheetTests/Writer/Ods/MicrosecondsTest.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Ods;
6+
7+
use DateTime;
8+
use PhpOffice\PhpSpreadsheet\Shared\Date;
9+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
10+
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
11+
12+
class MicrosecondsTest extends AbstractFunctional
13+
{
14+
/**
15+
* Test save and load XLSX file for round-trip DateTime.
16+
* Ods Writer does not support date formats,
17+
* and Reader does not support styles, so this
18+
* test is slightly different than its Xls/Xlsx counterparts.
19+
*/
20+
public function testIssue4476(): void
21+
{
22+
$date = '2020-10-21';
23+
$time = '14:55:31';
24+
$originalDateTime = new DateTime("{$date}T{$time}");
25+
$spreadsheet = new Spreadsheet();
26+
$sheet = $spreadsheet->getActiveSheet();
27+
$sheet->setCellValue('A1', Date::dateTimeToExcel($originalDateTime));
28+
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Ods');
29+
$spreadsheet->disconnectWorksheets();
30+
31+
$rsheet = $reloadedSpreadsheet->getActiveSheet();
32+
$rsheet->getStyle('A1')
33+
->getNumberFormat()
34+
->setFormatCode('yyyy-mm-dd hh:mm:ss.000');
35+
/** @var float */
36+
$reread = $rsheet->getCell('A1')->getValue();
37+
$temp = Date::excelToDateTimeObject($reread)
38+
->format('Y-m-d H:i:s.u');
39+
self::assertSame("{$date} {$time}.000000", $temp, 'round trip works with float value');
40+
$formatted = $rsheet->getCell('A1')->getFormattedValue();
41+
self::assertSame("{$date} {$time}.000", $formatted, 'round trip works with formatted value');
42+
/** @var float */
43+
$temp = Date::stringToExcel($formatted);
44+
$temp = Date::excelToDateTimeObject($temp)
45+
->format('Y-m-d H:i:s.u');
46+
self::assertSame("{$date} {$time}.000000", $temp, 'round trip works using stringToExcel on formatted value');
47+
48+
$reloadedSpreadsheet->disconnectWorksheets();
49+
}
50+
}

‎tests/PhpSpreadsheetTests/Writer/Xls/MicrosecondsTest.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xls;
6+
7+
use DateTime;
8+
use PhpOffice\PhpSpreadsheet\Shared\Date;
9+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
10+
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
11+
12+
class MicrosecondsTest extends AbstractFunctional
13+
{
14+
/**
15+
* Test save and load XLSX file for round-trip DateTime.
16+
*/
17+
public function testIssue4476(): void
18+
{
19+
$date = '2020-10-21';
20+
$time = '14:55:31';
21+
$originalDateTime = new DateTime("{$date}T{$time}");
22+
$spreadsheet = new Spreadsheet();
23+
$sheet = $spreadsheet->getActiveSheet();
24+
$sheet->setCellValue('A1', Date::dateTimeToExcel($originalDateTime));
25+
$sheet->getStyle('A1')
26+
->getNumberFormat()
27+
->setFormatCode('yyyy-mm-dd hh:mm:ss.000');
28+
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xls');
29+
$spreadsheet->disconnectWorksheets();
30+
31+
$rsheet = $reloadedSpreadsheet->getActiveSheet();
32+
/** @var float */
33+
$reread = $rsheet->getCell('A1')->getValue();
34+
$temp = Date::excelToDateTimeObject($reread)
35+
->format('Y-m-d H:i:s.u');
36+
self::assertSame("{$date} {$time}.000000", $temp, 'round trip works with float value');
37+
$formatted = $rsheet->getCell('A1')->getFormattedValue();
38+
self::assertSame("{$date} {$time}.000", $formatted, 'round trip works with formatted value');
39+
/** @var float */
40+
$temp = Date::stringToExcel($formatted);
41+
$temp = Date::excelToDateTimeObject($temp)
42+
->format('Y-m-d H:i:s.u');
43+
self::assertSame("{$date} {$time}.000000", $temp, 'round trip works using stringToExcel on formatted value');
44+
45+
$reloadedSpreadsheet->disconnectWorksheets();
46+
}
47+
}

‎tests/PhpSpreadsheetTests/Writer/Xlsx/MicrosecondsTest.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
6+
7+
use DateTime;
8+
use PhpOffice\PhpSpreadsheet\Shared\Date;
9+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
10+
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
11+
12+
class MicrosecondsTest extends AbstractFunctional
13+
{
14+
/**
15+
* Test save and load XLSX file for round-trip DateTime.
16+
*/
17+
public function testIssue4476(): void
18+
{
19+
$date = '2020-10-21';
20+
$time = '14:55:31';
21+
$originalDateTime = new DateTime("{$date}T{$time}");
22+
$spreadsheet = new Spreadsheet();
23+
$sheet = $spreadsheet->getActiveSheet();
24+
$sheet->setCellValue('A1', Date::dateTimeToExcel($originalDateTime));
25+
$sheet->getStyle('A1')
26+
->getNumberFormat()
27+
->setFormatCode('yyyy-mm-dd hh:mm:ss.000');
28+
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
29+
$spreadsheet->disconnectWorksheets();
30+
31+
$rsheet = $reloadedSpreadsheet->getActiveSheet();
32+
/** @var float */
33+
$reread = $rsheet->getCell('A1')->getValue();
34+
$temp = Date::excelToDateTimeObject($reread)
35+
->format('Y-m-d H:i:s.u');
36+
self::assertSame("{$date} {$time}.000000", $temp, 'round trip works with float value');
37+
$formatted = $rsheet->getCell('A1')->getFormattedValue();
38+
self::assertSame("{$date} {$time}.000", $formatted, 'round trip works with formatted value');
39+
/** @var float */
40+
$temp = Date::stringToExcel($formatted);
41+
$temp = Date::excelToDateTimeObject($temp)
42+
->format('Y-m-d H:i:s.u');
43+
self::assertSame("{$date} {$time}.000000", $temp, 'round trip works using stringToExcel on formatted value');
44+
45+
$reloadedSpreadsheet->disconnectWorksheets();
46+
}
47+
}

0 commit comments

Comments
 (0)