Merge pull request #2625 from PHPOffice/Iso-Date-Datatype-Additional-Unit-Tests

Minor refactoring of ISO Date checking
2022-02-26 16:10:24 +01:00 · 2022-02-26 16:10:24 +01:00 · d88e8f85ba
parent 9cf526a920 138638ad57
commit d88e8f85ba
8 changed files with 139 additions and 70 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org).

 ### Changed

- Nothing
+- Gnumeric Reader now loads number formatting for cells.
+- Gnumeric Reader now correctly identifies selected worksheet.

 ### Deprecated

--- a/src/PhpSpreadsheet/Cell/Cell.php
+++ b/src/PhpSpreadsheet/Cell/Cell.php
@ -2,7 +2,6 @@

 namespace PhpOffice\PhpSpreadsheet\Cell;

-use DateTime;
 use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
 use PhpOffice\PhpSpreadsheet\Collection\Cells;
@ -240,18 +239,7 @@ class Cell

                break;
            case DataType::TYPE_ISO_DATE:
-                if (!is_string($value)) {
-                    throw new Exception('Non-string supplied for datatype Date');
-                }
-                $date = new DateTime($value);
-                $newValue = SharedDate::PHPToExcel($date);
-                if ($newValue === false) {
-                    throw new Exception("Invalid string $value supplied for datatype Date");
-                }
-                if (preg_match('/^\\d\\d:\\d\\d:\\d\\d/', $value) == 1) {
-                    $newValue = fmod($newValue, 1.0);
-                }
-                $this->value = $newValue;
+                $this->value = SharedDate::convertIsoDate($value);
                $dataType = DataType::TYPE_NUMERIC;

                break;
--- a/src/PhpSpreadsheet/Reader/Gnumeric.php
+++ b/src/PhpSpreadsheet/Reader/Gnumeric.php
@ -284,12 +284,8 @@ class Gnumeric extends BaseReader
                $row = (int) $cellAttributes->Row + 1;
                $column = (int) $cellAttributes->Col;

-                if ($row > $maxRow) {
-                    $maxRow = $row;
-                }
-                if ($column > $maxCol) {
-                    $maxCol = $column;
-                }
+                $maxRow = max($maxRow, $row);
+                $maxCol = max($maxCol, $column);

                $column = Coordinate::stringFromColumnIndex($column + 1);

@ -300,38 +296,7 @@ class Gnumeric extends BaseReader
                    }
                }

-                $ValueType = $cellAttributes->ValueType;
-                $ExprID = (string) $cellAttributes->ExprID;
-                $type = DataType::TYPE_FORMULA;
-                if ($ExprID > '') {
-                    if (((string) $cell) > '') {
-                        $this->expressions[$ExprID] = [
-                            'column' => $cellAttributes->Col,
-                            'row' => $cellAttributes->Row,
-                            'formula' => (string) $cell,
-                        ];
-                    } else {
-                        $expression = $this->expressions[$ExprID];
-
-                        $cell = $this->referenceHelper->updateFormulaReferences(
-                            $expression['formula'],
-                            'A1',
-                            $cellAttributes->Col - $expression['column'],
-                            $cellAttributes->Row - $expression['row'],
-                            $worksheetName
-                        );
-                    }
-                    $type = DataType::TYPE_FORMULA;
-                } else {
-                    $vtype = (string) $ValueType;
-                    if (array_key_exists($vtype, self::$mappings['dataType'])) {
-                        $type = self::$mappings['dataType'][$vtype];
-                    }
-                    if ($vtype === '20') {        //    Boolean
-                        $cell = $cell == 'TRUE';
-                    }
-                }
-                $this->spreadsheet->getActiveSheet()->getCell($column . $row)->setValueExplicit((string) $cell, $type);
+                $this->loadCell($cell, $worksheetName, $cellAttributes, $column, $row);
            }

            if ($sheet->Styles !== null) {
@ -349,10 +314,21 @@ class Gnumeric extends BaseReader

        $this->processDefinedNames($gnmXML);

+        $this->setSelectedSheet($gnmXML);
+
        // Return
        return $this->spreadsheet;
    }

+    private function setSelectedSheet(SimpleXMLElement $gnmXML): void
+    {
+        if (isset($gnmXML->UIData)) {
+            $attributes = self::testSimpleXml($gnmXML->UIData->attributes());
+            $selectedSheet = (int) $attributes['SelectedTab'];
+            $this->spreadsheet->setActiveSheetIndex($selectedSheet);
+        }
+    }
+
    private function processMergedCells(?SimpleXMLElement $sheet): void
    {
        //    Handle Merged Cells in this worksheet
@ -530,4 +506,51 @@ class Gnumeric extends BaseReader

        return $value;
    }
+
+    private function loadCell(
+        SimpleXMLElement $cell,
+        string $worksheetName,
+        SimpleXMLElement $cellAttributes,
+        string $column,
+        int $row
+    ): void {
+        $ValueType = $cellAttributes->ValueType;
+        $ExprID = (string) $cellAttributes->ExprID;
+        $type = DataType::TYPE_FORMULA;
+        if ($ExprID > '') {
+            if (((string) $cell) > '') {
+                $this->expressions[$ExprID] = [
+                    'column' => $cellAttributes->Col,
+                    'row' => $cellAttributes->Row,
+                    'formula' => (string) $cell,
+                ];
+            } else {
+                $expression = $this->expressions[$ExprID];
+
+                $cell = $this->referenceHelper->updateFormulaReferences(
+                    $expression['formula'],
+                    'A1',
+                    $cellAttributes->Col - $expression['column'],
+                    $cellAttributes->Row - $expression['row'],
+                    $worksheetName
+                );
+            }
+            $type = DataType::TYPE_FORMULA;
+        } else {
+            $vtype = (string) $ValueType;
+            if (array_key_exists($vtype, self::$mappings['dataType'])) {
+                $type = self::$mappings['dataType'][$vtype];
+            }
+            if ($vtype === '20') {        //    Boolean
+                $cell = $cell == 'TRUE';
+            }
+        }
+
+        $this->spreadsheet->getActiveSheet()->getCell($column . $row)->setValueExplicit((string) $cell, $type);
+        if (isset($cellAttributes->ValueFormat)) {
+            $this->spreadsheet->getActiveSheet()->getCell($column . $row)
+                ->getStyle()->getNumberFormat()
+                ->setFormatCode((string) $cellAttributes->ValueFormat);
+        }
+    }
 }
--- a/src/PhpSpreadsheet/Reader/Ods.php
+++ b/src/PhpSpreadsheet/Reader/Ods.php
@ -2,7 +2,6 @@

 namespace PhpOffice\PhpSpreadsheet\Reader;

-use DateTime;
 use DOMAttr;
 use DOMDocument;
 use DOMElement;
@ -478,21 +477,7 @@ class Ods extends BaseReader
                                        case 'date':
                                            $type = DataType::TYPE_NUMERIC;
                                            $value = $cellData->getAttributeNS($officeNs, 'date-value');
-
-                                            $dateObj = new DateTime($value);
-                                            [$year, $month, $day, $hour, $minute, $second] = explode(
-                                                ' ',
-                                                $dateObj->format('Y m d H i s')
-                                            );
-
-                                            $dataValue = Date::formattedPHPToExcel(
-                                                (int) $year,
-                                                (int) $month,
-                                                (int) $day,
-                                                (int) $hour,
-                                                (int) $minute,
-                                                (int) $second
-                                            );
+                                            $dataValue = Date::convertIsoDate($value);

                                            if ($dataValue != floor($dataValue)) {
                                                $formatting = NumberFormat::FORMAT_DATE_XLSX15
--- a/src/PhpSpreadsheet/Shared/Date.php
+++ b/src/PhpSpreadsheet/Shared/Date.php
@ -9,7 +9,9 @@ use PhpOffice\PhpSpreadsheet\Calculation\DateTimeExcel;
 use PhpOffice\PhpSpreadsheet\Calculation\Functions;
 use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
 use PhpOffice\PhpSpreadsheet\Cell\Cell;
+use PhpOffice\PhpSpreadsheet\Exception;
 use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
+use PhpOffice\PhpSpreadsheet\Shared\Date as SharedDate;
 use PhpOffice\PhpSpreadsheet\Style\NumberFormat;

 class Date
@ -158,6 +160,36 @@ class Date
        throw new PhpSpreadsheetException('Invalid timezone');
    }

+    /**
+     * @param mixed $value
+     *
+     * @return float|int
+     */
+    public static function convertIsoDate($value)
+    {
+        if (!is_string($value)) {
+            throw new Exception('Non-string value supplied for Iso Date conversion');
+        }
+
+        $date = new DateTime($value);
+        $dateErrors = DateTime::getLastErrors();
+
+        if (is_array($dateErrors) && ($dateErrors['warning_count'] > 0 || $dateErrors['error_count'] > 0)) {
+            throw new Exception("Invalid string $value supplied for datatype Date");
+        }
+
+        $newValue = SharedDate::PHPToExcel($date);
+        if ($newValue === false) {
+            throw new Exception("Invalid string $value supplied for datatype Date");
+        }
+
+        if (preg_match('/^\\d\\d:\\d\\d:\\d\\d/', $value) == 1) {
+            $newValue = fmod($newValue, 1.0);
+        }
+
+        return $newValue;
+    }
+
    /**
     * Convert a MS serialized datetime value from Excel to a PHP Date/Time object.
     *
--- a/tests/PhpSpreadsheetTests/Cell/CellTest.php
+++ b/tests/PhpSpreadsheetTests/Cell/CellTest.php
@ -2,6 +2,7 @@

 namespace PhpOffice\PhpSpreadsheetTests\Cell;

+use PhpOffice\PhpSpreadsheet\Cell\DataType;
 use PhpOffice\PhpSpreadsheet\Exception;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Style\Color;
@ -33,6 +34,19 @@ class CellTest extends TestCase
        return require 'tests/data/Cell/SetValueExplicit.php';
    }

+    public function testInvalidIsoDateSetValueExplicit(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $cell = $spreadsheet->getActiveSheet()->getCell('A1');
+
+        $dateValue = '2022-02-29'; // Invalid leap year
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage("Invalid string {$dateValue} supplied for datatype Date");
+        $cell->setValueExplicit($dateValue, DataType::TYPE_ISO_DATE);
+
+        $spreadsheet->disconnectWorksheets();
+    }
+
    /**
     * @dataProvider providerSetValueExplicitException
     *
--- a/tests/PhpSpreadsheetTests/Shared/DateTest.php
+++ b/tests/PhpSpreadsheetTests/Shared/DateTest.php
@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheetTests\Shared;

 use DateTimeZone;
+use PhpOffice\PhpSpreadsheet\Exception;
 use PhpOffice\PhpSpreadsheet\Shared\Date;
 use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
 use PHPUnit\Framework\TestCase;
@ -209,6 +210,15 @@ class DateTest extends TestCase
        return require 'tests/data/Shared/Date/ExcelToTimestamp1900Timezone.php';
    }

+    public function testConvertIsoDateError(): void
+    {
+        Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Non-string value supplied for Iso Date conversion');
+        Date::convertIsoDate(false);
+    }
+
    public function testVarious(): void
    {
        Date::setDefaultTimeZone('UTC');
--- a/tests/data/Cell/SetValueExplicit.php
+++ b/tests/data/Cell/SetValueExplicit.php
@ -1,5 +1,6 @@
 <?php

+use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
 use PhpOffice\PhpSpreadsheet\Cell\DataType;

 return [
@ -39,12 +40,27 @@ return [
        DataType::TYPE_NUMERIC,
    ],
    [
-        '#DIV/0!',
+        44613.43090277778,
+        '2022-02-21 10:20:30',
+        DataType::TYPE_ISO_DATE,
+    ],
+    [
+        44613.0,
+        '2022-02-21',
+        DataType::TYPE_ISO_DATE,
+    ],
+    [
+        -30879.0,
+        '1815-06-15',               // Dates outside the Excel Range should fail really
+        DataType::TYPE_ISO_DATE,
+    ],
+    [
+        ExcelError::DIV0(),
        '#DIV/0!',
        DataType::TYPE_ERROR,
    ],
    [
-        '#NULL!',
+        ExcelError::null(),
        'NOT A VALID ERROR TYPE VALUE',
        DataType::TYPE_ERROR,
    ],