Skip to content

Commit a377f44

Browse files
oleibmanoleibman
committed
Xlsx Reader Default Cell Type
Suggested by PR #1321 from @rtek, which was marked stale in March 2020. If the xml for a cell does not specify a type, PhpSpreadsheet has been using string; however, Excel uses numeric. PhpSpreadsheet will be changed to do the same as Excel. In theory, this is a BC break; in practice, it is doubtful that anyone will be adversely affected. No changes in the existing unit tests were required. The new test from rtek does demonstrate the change in behavior; however, the new behavior is certainly what a user would expect to happen. Unlike the original PR, this one is restricted to Xlsx Reader; I do not believe that any logic change is needed for Writer. (N.B. - changing Writer to use class constants rather than literals is a good idea, but, with no other logic change in this version, it seems risky - I may do that as separate PR.)
1 parent f7c183b commit a377f44

File tree

2 files changed

+121
-18
lines changed

2 files changed

+121
-18
lines changed

‎src/PhpSpreadsheet/Reader/Xlsx.php

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
846846

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

@@ -858,7 +858,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
858858
}
859859

860860
break;
861-
case 'b':
861+
case DataType::TYPE_BOOL:
862862
if (!isset($c->f) || ((string) $c->f) === '') {
863863
if (isset($c->v)) {
864864
$value = self::castToBoolean($c);
@@ -869,22 +869,29 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
869869
} else {
870870
// Formula
871871
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToBoolean');
872-
if (isset($c->f['t'])) {
873-
$att = $c->f;
874-
$docSheet->getCell($r)->setFormulaAttributes($att);
875-
}
872+
self::storeFormulaAttributes($c->f, $docSheet, $r);
873+
}
874+
875+
break;
876+
case DataType::TYPE_STRING2:
877+
if (isset($c->f)) {
878+
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToString');
879+
self::storeFormulaAttributes($c->f, $docSheet, $r);
880+
} else {
881+
$value = self::castToString($c);
876882
}
877883

878884
break;
879-
case 'inlineStr':
885+
case DataType::TYPE_INLINE:
880886
if (isset($c->f)) {
881887
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToError');
888+
self::storeFormulaAttributes($c->f, $docSheet, $r);
882889
} else {
883890
$value = $this->parseRichText($c->is);
884891
}
885892

886893
break;
887-
case 'e':
894+
case DataType::TYPE_ERROR:
888895
if (!isset($c->f)) {
889896
$value = self::castToError($c);
890897
} else {
@@ -902,20 +909,16 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
902909
default:
903910
if (!isset($c->f)) {
904911
$value = self::castToString($c);
912+
if (is_numeric($value)) {
913+
$value += 0;
914+
}
905915
} else {
906916
// Formula
907917
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToString');
908-
$formulaAttributes = [];
909-
$attributes = $c->f->attributes();
910-
if (isset($attributes['t'])) {
911-
$formulaAttributes['t'] = (string) $attributes['t'];
912-
}
913-
if (isset($attributes['ref'])) {
914-
$formulaAttributes['ref'] = (string) $attributes['ref'];
915-
}
916-
if (!empty($formulaAttributes)) {
917-
$docSheet->getCell($r)->setFormulaAttributes($formulaAttributes);
918+
if (is_numeric($calculatedValue)) {
919+
$calculatedValue += 0;
918920
}
921+
self::storeFormulaAttributes($c->f, $docSheet, $r);
919922
}
920923

921924
break;
@@ -2373,4 +2376,19 @@ private function processIgnoredErrors(SimpleXMLElement $xml, Worksheet $sheet):
23732376
}
23742377
}
23752378
}
2379+
2380+
private static function storeFormulaAttributes(SimpleXMLElement $f, Worksheet $docSheet, string $r): void
2381+
{
2382+
$formulaAttributes = [];
2383+
$attributes = $f->attributes();
2384+
if (isset($attributes['t'])) {
2385+
$formulaAttributes['t'] = (string) $attributes['t'];
2386+
}
2387+
if (isset($attributes['ref'])) {
2388+
$formulaAttributes['ref'] = (string) $attributes['ref'];
2389+
}
2390+
if (!empty($formulaAttributes)) {
2391+
$docSheet->getCell($r)->setFormulaAttributes($formulaAttributes);
2392+
}
2393+
}
23762394
}

‎tests/PhpSpreadsheetTests/Reader/Xlsx/NumericCellTypeTest.php

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
6+
7+
use PhpOffice\PhpSpreadsheet\Cell\Cell;
8+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
9+
use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
10+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader;
11+
use PhpOffice\PhpSpreadsheet\Shared\File;
12+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
13+
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer;
14+
use PHPUnit\Framework\TestCase;
15+
16+
class NumericCellTypeTest extends TestCase
17+
{
18+
private IValueBinder $oldBinder;
19+
20+
private string $filename = '';
21+
22+
protected function setUp(): void
23+
{
24+
$this->oldBinder = Cell::getValueBinder();
25+
26+
$binder = new class () implements IValueBinder {
27+
public function bindValue(Cell $cell, mixed $value): bool
28+
{
29+
if (is_float($value) || is_int($value)) {
30+
$type = DataType::TYPE_NUMERIC;
31+
} elseif (is_string($value)) {
32+
$type = DataType::TYPE_STRING;
33+
} else {
34+
return false;
35+
}
36+
37+
$cell->setValueExplicit($value, $type);
38+
39+
return true;
40+
}
41+
};
42+
43+
Cell::setValueBinder($binder);
44+
}
45+
46+
protected function tearDown(): void
47+
{
48+
Cell::setValueBinder($this->oldBinder);
49+
if ($this->filename !== '') {
50+
unlink($this->filename);
51+
$this->filename = '';
52+
}
53+
}
54+
55+
public function testCellShouldHaveNumericTypeAttribute(): void
56+
{
57+
$spreadsheet = new Spreadsheet();
58+
$sheet = $spreadsheet->getActiveSheet();
59+
$array = [
60+
['1.0'],
61+
[1.0],
62+
['-1.0'],
63+
[-1.0],
64+
['0'],
65+
[0],
66+
['0.0'],
67+
[0.0],
68+
['1e1'],
69+
[1e1],
70+
];
71+
$sheet->fromArray($array, null, 'A1', true);
72+
73+
$this->filename = File::temporaryFilename();
74+
$writer = new Writer($spreadsheet);
75+
$writer->save($this->filename);
76+
$spreadsheet->disconnectWorksheets();
77+
78+
$reader = new Reader();
79+
$spreadsheet2 = $reader->load($this->filename);
80+
81+
$sheet2 = $spreadsheet2->getActiveSheet();
82+
self::assertSame($array, $sheet2->toArray(null, false, false));
83+
$spreadsheet2->disconnectWorksheets();
84+
}
85+
}

0 commit comments

Comments
 (0)