Inconsistency Between Actual and Declared Type - Minor Break (#3715)

* Inconsistency Between Actual and Declared Type - Minor Break

Fix #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.

* Strict Types for New Tests

Consistent with work being done in PR #3718.

* Improve Test

Better match to original issue.

* Typo

Author: oleibman

src/PhpSpreadsheet/Cell/Cell.php

@@ -457,12 +457,9 @@ public function getDataType(): string
      */
     public function setDataType($dataType): self
     {
-        if ($dataType == DataType::TYPE_STRING2) {
-            $dataType = DataType::TYPE_STRING;
-        }
-        $this->dataType = $dataType;
+        $this->setValueExplicit($this->value, $dataType);
 
-        return $this->updateInCollection();
+        return $this;
     }
 
     /**

tests/PhpSpreadsheetTests/Cell/DataType2Test.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Cell;
+
+use PhpOffice\PhpSpreadsheet\Cell\DataType;
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PHPUnit\Framework\TestCase;
+
+class DataType2Test extends TestCase
+{
+    public function testSetDataType(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->getCell('A1')->setValue(28.1);
+        self::assertSame(28.1, $sheet->getCell('A1')->getValue());
+        self::assertSame('28.1', (string) $sheet->getCell('A1'));
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_STRING);
+        self::assertSame('28.1', $sheet->getCell('A1')->getValue());
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_NUMERIC);
+        self::assertSame(28.1, $sheet->getCell('A1')->getValue());
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_STRING2);
+        self::assertSame('28.1', $sheet->getCell('A1')->getValue());
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_INLINE);
+        self::assertSame('28.1', $sheet->getCell('A1')->getValue());
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_BOOL);
+        self::assertTrue($sheet->getCell('A1')->getValue());
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_NUMERIC);
+        self::assertSame(1, $sheet->getCell('A1')->getValue());
+
+        $sheet->getCell('A2')->setValue('X');
+
+        try {
+            $sheet->getCell('A2')->setDataType(DataType::TYPE_NUMERIC);
+        } catch (PhpSpreadsheetException $e) {
+            self::assertSame('Invalid numeric value for datatype Numeric', $e->getMessage());
+        }
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3711Test.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Cell\DataType;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+class Issue3711Test extends AbstractFunctional
+{
+    public function testIssue3711(): void
+    {
+        // Issue 3711 - float being used as index in StringTable.
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->getCell('A1')->setValue('21.5');
+        self::assertSame(21.5, $sheet->getCell('A1')->getValue());
+        $sheet->getCell('A1')->setDataType(DataType::TYPE_STRING);
+        $sheet->getCell('A2')->setValue('21');
+        self::assertSame(21, $sheet->getCell('A2')->getValue());
+        $sheet->getCell('A2')->setDataType(DataType::TYPE_STRING);
+
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
+        $spreadsheet->disconnectWorksheets();
+        $rsheet = $reloadedSpreadsheet->getActiveSheet();
+        self::assertSame('21.5', $rsheet->getCell('A1')->getValue());
+        self::assertSame('21', $rsheet->getCell('A2')->getValue());
+        $reloadedSpreadsheet->disconnectWorksheets();
+    }
+}