Skip to content

Commit 5926ede

Browse files
committed
Inconsistency Between Actual and Declared Type - Minor Break
Fix PHPOffice#3711. User set a cell value to float (implicitly by default value binder), then used `setDataType` to change its type to string. This caused a problem for Xlsx Writer, which uses the string cell values as an index into a Shared String array. However, as the cell actually contained a floating point value, Php treated it as an integer index; such a treatment is both deprecated, and leads to invalid values in the spreadsheet. The use case for `setDataType` is not strong. The user always has the option to use `setValueExplicit` if the type is important. Setting a type afterwards, i.e. irrespective of the value, seems like a peculiar action. Indeed, there are no tests whatever for such use in the unit test suite. There are two possible approaches to fixing this problem. The first is to add casts to the 3 or 4 places in Writer Xlsx which might be affected by this problem (hoping that you've found them all and realizing that similar changes might be needed for other Writers). The second is to change `setDataType` to call `setValueExplicit` using the current value of the cell, thereby possibly changing the cell value. I have gone with the second option - it seems like a much more logical approach, and guarantees that the content of the cell will always be consistent with its declared type. It is, however, a breaking change; if, for example, you have a cell with a string or numeric value and specify `boolean` to `setDataType`, the cell's value will change to `true` or `false` with no way to get back to the original.
1 parent 7288b4d commit 5926ede

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

src/PhpSpreadsheet/Cell/Cell.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -457,12 +457,9 @@ public function getDataType(): string
457457
*/
458458
public function setDataType($dataType): self
459459
{
460-
if ($dataType == DataType::TYPE_STRING2) {
461-
$dataType = DataType::TYPE_STRING;
462-
}
463-
$this->dataType = $dataType;
460+
$this->setValueExplicit($this->value, $dataType);
464461

465-
return $this->updateInCollection();
462+
return $this;
466463
}
467464

468465
/**
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Cell;
4+
5+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
6+
use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
7+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
8+
use PHPUnit\Framework\TestCase;
9+
10+
class DataType2Test extends TestCase
11+
{
12+
public function testSetDataType(): void
13+
{
14+
$spreadsheet = new Spreadsheet();
15+
$sheet = $spreadsheet->getActiveSheet();
16+
$sheet->getCell('A1')->setValue(28.1);
17+
self::assertSame(28.1, $sheet->getCell('A1')->getValue());
18+
self::assertSame('28.1', (string) $sheet->getCell('A1'));
19+
$sheet->getCell('A1')->setDataType(DataType::TYPE_STRING);
20+
self::assertSame('28.1', $sheet->getCell('A1')->getValue());
21+
$sheet->getCell('A1')->setDataType(DataType::TYPE_NUMERIC);
22+
self::assertSame(28.1, $sheet->getCell('A1')->getValue());
23+
$sheet->getCell('A1')->setDataType(DataType::TYPE_STRING2);
24+
self::assertSame('28.1', $sheet->getCell('A1')->getValue());
25+
$sheet->getCell('A1')->setDataType(DataType::TYPE_INLINE);
26+
self::assertSame('28.1', $sheet->getCell('A1')->getValue());
27+
$sheet->getCell('A1')->setDataType(DataType::TYPE_BOOL);
28+
self::assertTrue($sheet->getCell('A1')->getValue());
29+
$sheet->getCell('A1')->setDataType(DataType::TYPE_NUMERIC);
30+
self::assertSame(1, $sheet->getCell('A1')->getValue());
31+
32+
$sheet->getCell('A2')->setValue('X');
33+
34+
try {
35+
$sheet->getCell('A2')->setDataType(DataType::TYPE_NUMERIC);
36+
} catch (PhpSpreadsheetException $e) {
37+
self::assertSame('Invalid numeric value for datatype Numeric', $e->getMessage());
38+
}
39+
40+
$spreadsheet->disconnectWorksheets();
41+
}
42+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
4+
5+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
6+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
7+
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
8+
9+
class Issue3711Test extends AbstractFunctional
10+
{
11+
public function testIssue3711(): void
12+
{
13+
// Issue 3443 - float being used as index in StringTable.
14+
$spreadsheet = new Spreadsheet();
15+
$sheet = $spreadsheet->getActiveSheet();
16+
$sheet->getCell('A1')->setValue('21.8');
17+
$sheet->getCell('A1')->setDataType(DataType::TYPE_STRING);
18+
19+
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
20+
$spreadsheet->disconnectWorksheets();
21+
$rsheet = $reloadedSpreadsheet->getActiveSheet();
22+
self::assertSame('21.8', $rsheet->getCell('A1')->getValue());
23+
$reloadedSpreadsheet->disconnectWorksheets();
24+
}
25+
}

0 commit comments

Comments
 (0)