Skip to content

[BC break/RFC] Fix default cell type treatment as numeric for XLSX #1321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[BC break/RFC] Fix default cell type treatment as numeric for XLSX #1321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/PhpSpreadsheet/Reader/Xlsx.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PhpOffice\PhpSpreadsheet\Reader;

use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Cell\Hyperlink;
use PhpOffice\PhpSpreadsheet\NamedRange;
use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
Expand Down Expand Up @@ -262,6 +263,13 @@ private static function castToString($c)
return isset($c->v) ? (string) $c->v : null;
}

private static function castToNumeric($c)
{
$v = self::castToString($c);

return $v === null ? null : 0 + $v;
}

private function castToFormula($c, $r, &$cellDataType, &$value, &$calculatedValue, &$sharedFormulas, $castBaseType)
{
$cellDataType = 'f';
Expand Down Expand Up @@ -674,7 +682,7 @@ public function load($pFilename)

// Read cell!
switch ($cellDataType) {
case 's':
case DataType::TYPE_STRING:
if ((string) $c->v != '') {
$value = $sharedStrings[(int) ($c->v)];

Expand All @@ -686,7 +694,7 @@ public function load($pFilename)
}

break;
case 'b':
case DataType::TYPE_BOOL:
if (!isset($c->f)) {
$value = self::castToBoolean($c);
} else {
Expand All @@ -699,15 +707,15 @@ public function load($pFilename)
}

break;
case 'inlineStr':
case DataType::TYPE_INLINE:
if (isset($c->f)) {
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, $sharedFormulas, 'castToError');
} else {
$value = $this->parseRichText($c->is);
}

break;
case 'e':
case DataType::TYPE_ERROR:
if (!isset($c->f)) {
$value = self::castToError($c);
} else {
Expand All @@ -716,12 +724,16 @@ public function load($pFilename)
}

break;


case DataType::TYPE_NUMERIC:
//numeric is the default cell type
default:
if (!isset($c->f)) {
$value = self::castToString($c);
$value = self::castToNumeric($c);
} else {
// Formula
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, $sharedFormulas, 'castToString');
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, $sharedFormulas, 'castToNumeric');
}

break;
Expand Down
30 changes: 17 additions & 13 deletions src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\RichText\RichText;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
use PhpOffice\PhpSpreadsheet\Shared\XMLWriter;
Expand Down Expand Up @@ -1068,14 +1069,17 @@ private function writeCell(XMLWriter $objWriter, PhpspreadsheetWorksheet $pSheet
$mappedType = $pCell->getDataType();

// Write data type depending on its type
switch (strtolower($mappedType)) {
case 'inlinestr': // Inline string
case 's': // String
case 'b': // Boolean
switch ($mappedType) {
case DataType::TYPE_NUMERIC:
//noop - type attribute is optional and defaults to 'n'
break;
case DataType::TYPE_INLINE:
case DataType::TYPE_STRING:
case DataType::TYPE_BOOL:
$objWriter->writeAttribute('t', $mappedType);

break;
case 'f': // Formula
case DataType::TYPE_FORMULA:
$calculatedValue = ($this->getParentWriter()->getPreCalculateFormulas()) ?
$pCell->getCalculatedValue() : $cellValue;
if (is_string($calculatedValue)) {
Expand All @@ -1085,13 +1089,13 @@ private function writeCell(XMLWriter $objWriter, PhpspreadsheetWorksheet $pSheet
}

break;
case 'e': // Error
case DataType::TYPE_ERROR:
$objWriter->writeAttribute('t', $mappedType);
}

// Write data depending on its type
switch (strtolower($mappedType)) {
case 'inlinestr': // Inline string
switch ($mappedType) {
case DataType::TYPE_INLINE:
if (!$cellValue instanceof RichText) {
$objWriter->writeElement('t', StringHelper::controlCharacterPHP2OOXML(htmlspecialchars($cellValue)));
} elseif ($cellValue instanceof RichText) {
Expand All @@ -1101,7 +1105,7 @@ private function writeCell(XMLWriter $objWriter, PhpspreadsheetWorksheet $pSheet
}

break;
case 's': // String
case DataType::TYPE_STRING:
if (!$cellValue instanceof RichText) {
if (isset($pFlippedStringTable[$cellValue])) {
$objWriter->writeElement('v', $pFlippedStringTable[$cellValue]);
Expand All @@ -1111,7 +1115,7 @@ private function writeCell(XMLWriter $objWriter, PhpspreadsheetWorksheet $pSheet
}

break;
case 'f': // Formula
case DataType::TYPE_FORMULA:
$attributes = $pCell->getFormulaAttributes();
if (($attributes['t'] ?? null) === 'array') {
$objWriter->startElement('f');
Expand All @@ -1137,7 +1141,7 @@ private function writeCell(XMLWriter $objWriter, PhpspreadsheetWorksheet $pSheet
}

break;
case 'n': // Numeric
case DataType::TYPE_NUMERIC:
//force a decimal to be written if the type is float
if (is_float($cellValue)) {
// force point as decimal separator in case current locale uses comma
Expand All @@ -1149,11 +1153,11 @@ private function writeCell(XMLWriter $objWriter, PhpspreadsheetWorksheet $pSheet
$objWriter->writeElement('v', $cellValue);

break;
case 'b': // Boolean
case DataType::TYPE_BOOL:
$objWriter->writeElement('v', ($cellValue ? '1' : '0'));

break;
case 'e': // Error
case DataType::TYPE_ERROR:
if (substr($cellValue, 0, 1) === '=') {
$objWriter->writeElement('f', substr($cellValue, 1));
$objWriter->writeElement('v', substr($cellValue, 1));
Expand Down
85 changes: 85 additions & 0 deletions tests/PhpSpreadsheetTests/Writer/Xlsx/NumericCellTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;

use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader;
use PhpOffice\PhpSpreadsheet\Settings;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer;
use PHPUnit\Framework\TestCase;

class NumericCellTypeTest extends TestCase
{
protected $oldBinder;

protected function setUp()
{
Settings::setLibXmlLoaderOptions(null);
$this->oldBinder = Cell::getValueBinder();

$binder = new class() implements IValueBinder {
public function bindValue(Cell $cell, $value)
{
if (is_float($value) || is_int($value)) {
$type = DataType::TYPE_NUMERIC;
} elseif (is_string($value)) {
$type = DataType::TYPE_STRING;
} else {
return false;
}

$cell->setValueExplicit($value, $type);

return true;
}
};

Cell::setValueBinder($binder);
}

protected function tearDown()
{
Cell::setValueBinder($this->oldBinder);
}

/**
* @dataProvider providerCellShouldHaveNumericTypeAttribute
*
* @param float|int|string $value
*/
public function testCellShouldHaveNumericTypeAttribute($value)
{
$outputFilename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test');

$sheet = new Spreadsheet();
$sheet->getActiveSheet()->getCell('A1')->setValue($value);

$writer = new Writer($sheet);
$writer->save($outputFilename);

$reader = new Reader();
$sheet = $reader->load($outputFilename);

$this->assertSame($value, $sheet->getActiveSheet()->getCell('A1')->getValue());
}

public function providerCellShouldHaveNumericTypeAttribute()
{
return [
['1.0'],
[1.0],
['-1.0'],
[-1.0],
['0'],
[0],
['0.0'],
[0.0],
['1e1'],
[1e1],
];
}
}