Shared/Date::isDateTime Handle Cells Which Calculate as Arrays

In issue #4557, the user complains, with some justification, about the way Excel handles certain calculations. We are not able to help with that problem. However, the user also notes a problem in Shared/Date when `isDateTime` has to evaluate a cell whose calculated value is an array. This is solved by flattening the calculated result to a single value.

It became obvious while working on this change that the code to set `instanceArrayReturnType` was kind of awkward. Simpler methods `returnArrayAsArray` and `returnArrayAsValue` are added to `Spreadsheet`. Even these started out a bit awkward because `Spreadsheet::calculationEngine` was defined as nullable, which really isn't true. It is allocated by the constructor, and never freed except in the destructor. It is no longer nullable.

Author: oleibman

src/PhpSpreadsheet/Calculation/Calculation.php

@@ -207,10 +207,7 @@ public function __construct(?Spreadsheet $spreadsheet = null)
     public static function getInstance(?Spreadsheet $spreadsheet = null): self
     {
         if ($spreadsheet !== null) {
-            $instance = $spreadsheet->getCalculationEngine();
-            if (isset($instance)) {
-                return $instance;
-            }
+            return $spreadsheet->getCalculationEngine();
         }
 
         if (!self::$instance) {
@@ -220,6 +217,20 @@ public static function getInstance(?Spreadsheet $spreadsheet = null): self
         return self::$instance;
     }
 
+    /**
+     * Intended for use only via a destructor.
+     *
+     * @internal
+     */
+    public static function getInstanceOrNull(?Spreadsheet $spreadsheet = null): ?self
+    {
+        if ($spreadsheet !== null) {
+            return $spreadsheet->getCalculationEngineOrNull();
+        }
+
+        return null;
+    }
+
     /**
      * Flush the calculation cache for any existing instance of this class
      *        but only if a Calculation instance exists.

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -2,7 +2,6 @@
 
 namespace PhpOffice\PhpSpreadsheet\Reader;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
 use PhpOffice\PhpSpreadsheet\Cell\DataType;
@@ -452,10 +451,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
             switch ($rel['Type']) {
                 case "$xmlNamespaceBase/sheetMetadata":
                     if ($this->fileExistsInArchive($zip, "xl/{$relTarget}")) {
-                        $excel->getCalculationEngine()
-                            ?->setInstanceArrayReturnType(
-                                Calculation::RETURN_ARRAY_AS_ARRAY
-                            );
+                        $excel->returnArrayAsArray();
                     }
 
                     break;

src/PhpSpreadsheet/Shared/Date.php

@@ -374,7 +374,12 @@ public static function isDateTime(Cell $cell, mixed $value = null, bool $dateWit
             $selected = $worksheet->getSelectedCells();
 
             try {
-                $result = is_numeric($value ?? $cell->getCalculatedValue())
+                if ($value === null) {
+                    $value = Functions::flattenSingleValue(
+                        $cell->getCalculatedValue()
+                    );
+                }
+                $result = is_numeric($value)
                     && self::isDateTimeFormat(
                         $worksheet->getStyle(
                             $cell->getCoordinate()

src/PhpSpreadsheet/Spreadsheet.php

@@ -58,7 +58,7 @@ class Spreadsheet implements JsonSerializable
     /**
      * Calculation Engine.
      */
-    private ?Calculation $calculationEngine;
+    private Calculation $calculationEngine;
 
     /**
      * Active sheet index.
@@ -477,7 +477,7 @@ public function __construct()
     public function __destruct()
     {
         $this->disconnectWorksheets();
-        $this->calculationEngine = null;
+        unset($this->calculationEngine);
         $this->cellXfCollection = [];
         $this->cellStyleXfCollection = [];
         $this->definedNames = [];
@@ -499,11 +499,25 @@ public function disconnectWorksheets(): void
     /**
      * Return the calculation engine for this worksheet.
      */
-    public function getCalculationEngine(): ?Calculation
+    public function getCalculationEngine(): Calculation
     {
         return $this->calculationEngine;
     }
 
+    /**
+     * Intended for use only via a destructor.
+     *
+     * @internal
+     */
+    public function getCalculationEngineOrNull(): ?Calculation
+    {
+        if (!isset($this->calculationEngine)) { //* @phpstan-ignore-line
+            return null;
+        }
+
+        return $this->calculationEngine;
+    }
+
     /**
      * Get properties.
      */
@@ -1120,21 +1134,19 @@ public function __clone()
 
         $oldCalc = $this->calculationEngine;
         $this->calculationEngine = new Calculation($this);
-        if ($oldCalc !== null) {
-            $this->calculationEngine
-                ->setSuppressFormulaErrors(
-                    $oldCalc->getSuppressFormulaErrors()
-                )
-                ->setCalculationCacheEnabled(
-                    $oldCalc->getCalculationCacheEnabled()
-                )
-                ->setBranchPruningEnabled(
-                    $oldCalc->getBranchPruningEnabled()
-                )
-                ->setInstanceArrayReturnType(
-                    $oldCalc->getInstanceArrayReturnType()
-                );
-        }
+        $this->calculationEngine
+            ->setSuppressFormulaErrors(
+                $oldCalc->getSuppressFormulaErrors()
+            )
+            ->setCalculationCacheEnabled(
+                $oldCalc->getCalculationCacheEnabled()
+            )
+            ->setBranchPruningEnabled(
+                $oldCalc->getBranchPruningEnabled()
+            )
+            ->setInstanceArrayReturnType(
+                $oldCalc->getInstanceArrayReturnType()
+            );
         $usedKeys['calculationEngine'] = true;
 
         $currentCollection = $this->cellStyleXfCollection;
@@ -1801,4 +1813,18 @@ public function replaceBuiltinNumberFormat(int $builtinFormatIndex, string $form
             }
         }
     }
+
+    public function returnArrayAsArray(): void
+    {
+        $this->calculationEngine->setInstanceArrayReturnType(
+            Calculation::RETURN_ARRAY_AS_ARRAY
+        );
+    }
+
+    public function returnArrayAsValue(): void
+    {
+        $this->calculationEngine->setInstanceArrayReturnType(
+            Calculation::RETURN_ARRAY_AS_VALUE
+        );
+    }
 }

src/PhpSpreadsheet/Worksheet/Worksheet.php

@@ -373,7 +373,8 @@ public function disconnectCells(): void
      */
     public function __destruct()
     {
-        Calculation::getInstance($this->parent)->clearCalculationCacheForWorksheet($this->title);
+        Calculation::getInstanceOrNull($this->parent)
+            ?->clearCalculationCacheForWorksheet($this->title);
 
         $this->disconnectCells();
         unset($this->rowDimensions, $this->columnDimensions, $this->tableCollection, $this->drawingCollection, $this->chartCollection, $this->autoFilter);
@@ -909,7 +910,7 @@ public function setTitle(string $title, bool $updateFormulaCellReferences = true
         // Set title
         $this->title = $title;
 
-        if ($this->parent && $this->parent->getIndex($this, true) >= 0 && $this->parent->getCalculationEngine()) {
+        if ($this->parent && $this->parent->getIndex($this, true) >= 0) {
             // New title
             $newTitle = $this->getTitle();
             $this->parent->getCalculationEngine()

tests/PhpSpreadsheetTests/Calculation/StructuredReferenceFormulaTest.php

@@ -39,7 +39,7 @@ public function testStructuredReferenceHiddenHeaders(): void
         $result = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();
         self::assertSame('Region', $result);
 
-        $spreadsheet->getCalculationEngine()?->flushInstance();
+        $spreadsheet->getCalculationEngine()->flushInstance();
         $table->setShowHeaderRow(false);
 
         $result = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();

tests/PhpSpreadsheetTests/Shared/DateTest.php

@@ -11,6 +11,7 @@
 use PhpOffice\PhpSpreadsheet\Shared\Date;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 class DateTest extends TestCase
@@ -51,7 +52,7 @@ public function testSetExcelCalendarWithInvalidValue(): void
         self::assertFalse($result);
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeExcelToTimestamp1900')]
+    #[DataProvider('providerDateTimeExcelToTimestamp1900')]
     public function testDateTimeExcelToTimestamp1900(float|int $expectedResult, float|int $excelDateTimeValue): void
     {
         if ($expectedResult > PHP_INT_MAX || $expectedResult < PHP_INT_MIN) {
@@ -68,7 +69,7 @@ public static function providerDateTimeExcelToTimestamp1900(): array
         return require 'tests/data/Shared/Date/ExcelToTimestamp1900.php';
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeTimestampToExcel1900')]
+    #[DataProvider('providerDateTimeTimestampToExcel1900')]
     public function testDateTimeTimestampToExcel1900(float|int $expectedResult, float|int|string $unixTimestamp): void
     {
         Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
@@ -82,7 +83,7 @@ public static function providerDateTimeTimestampToExcel1900(): array
         return require 'tests/data/Shared/Date/TimestampToExcel1900.php';
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeDateTimeToExcel')]
+    #[DataProvider('providerDateTimeDateTimeToExcel')]
     public function testDateTimeDateTimeToExcel(float|int $expectedResult, DateTimeInterface $dateTimeObject): void
     {
         Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
@@ -99,7 +100,7 @@ public static function providerDateTimeDateTimeToExcel(): array
     /**
      * @param array{0: int, 1: int, 2: int, 3: int, 4: int, 5: float|int} $args Array containing year/month/day/hours/minutes/seconds
      */
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeFormattedPHPToExcel1900')]
+    #[DataProvider('providerDateTimeFormattedPHPToExcel1900')]
     public function testDateTimeFormattedPHPToExcel1900(mixed $expectedResult, ...$args): void
     {
         Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
@@ -113,7 +114,7 @@ public static function providerDateTimeFormattedPHPToExcel1900(): array
         return require 'tests/data/Shared/Date/FormattedPHPToExcel1900.php';
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeExcelToTimestamp1904')]
+    #[DataProvider('providerDateTimeExcelToTimestamp1904')]
     public function testDateTimeExcelToTimestamp1904(float|int $expectedResult, float|int $excelDateTimeValue): void
     {
         if ($expectedResult > PHP_INT_MAX || $expectedResult < PHP_INT_MIN) {
@@ -130,7 +131,7 @@ public static function providerDateTimeExcelToTimestamp1904(): array
         return require 'tests/data/Shared/Date/ExcelToTimestamp1904.php';
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeTimestampToExcel1904')]
+    #[DataProvider('providerDateTimeTimestampToExcel1904')]
     public function testDateTimeTimestampToExcel1904(mixed $expectedResult, float|int|string $unixTimestamp): void
     {
         Date::setExcelCalendar(Date::CALENDAR_MAC_1904);
@@ -144,7 +145,7 @@ public static function providerDateTimeTimestampToExcel1904(): array
         return require 'tests/data/Shared/Date/TimestampToExcel1904.php';
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerIsDateTimeFormatCode')]
+    #[DataProvider('providerIsDateTimeFormatCode')]
     public function testIsDateTimeFormatCode(mixed $expectedResult, string $format): void
     {
         $result = Date::isDateTimeFormatCode($format);
@@ -156,7 +157,7 @@ public static function providerIsDateTimeFormatCode(): array
         return require 'tests/data/Shared/Date/FormatCodes.php';
     }
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerDateTimeExcelToTimestamp1900Timezone')]
+    #[DataProvider('providerDateTimeExcelToTimestamp1900Timezone')]
     public function testDateTimeExcelToTimestamp1900Timezone(float|int $expectedResult, float|int $excelDateTimeValue, string $timezone): void
     {
         if ($expectedResult > PHP_INT_MAX || $expectedResult < PHP_INT_MIN) {
@@ -240,6 +241,27 @@ public function testVarious(): void
         $spreadsheet->disconnectWorksheets();
     }
 
+    public function testArray(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $spreadsheet->returnArrayAsArray();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A1', 45000);
+        $sheet->setCellValue('A2', 44000);
+        $sheet->setCellValue('A3', 46000);
+        $sheet->setCellValue('C1', '=SORT(A1:A3)');
+        $sheet->setCellValue('D1', '=SORT(A1:A3)');
+        $sheet->getStyle('C1')
+            ->getNumberFormat()
+            ->setFormatCode('yyyy-mm-dd');
+        self::assertTrue(Date::isDateTime($sheet->getCell('C1')));
+        self::assertFalse(Date::isDateTime($sheet->getCell('D1')));
+        self::assertIsArray(
+            $sheet->getCell('C1')->getCalculatedValue()
+        );
+        $spreadsheet->disconnectWorksheets();
+    }
+
     public function testRoundMicroseconds(): void
     {
         $dti = new DateTime('2000-01-02 03:04:05.999999');

tests/PhpSpreadsheetTests/SpreadsheetCopyCloneTest.php

@@ -49,8 +49,8 @@ public function testCopyClone(string $type): void
         } else {
             $spreadsheet2 = $this->spreadsheet2 = clone $spreadsheet;
         }
-        self::assertSame($spreadsheet, $spreadsheet->getCalculationEngine()?->getSpreadsheet());
-        self::assertSame($spreadsheet2, $spreadsheet2->getCalculationEngine()?->getSpreadsheet());
+        self::assertSame($spreadsheet, $spreadsheet->getCalculationEngine()->getSpreadsheet());
+        self::assertSame($spreadsheet2, $spreadsheet2->getCalculationEngine()->getSpreadsheet());
         self::assertSame('A3', $sheet->getSelectedCells());
         $copysheet = $spreadsheet2->getActiveSheet();
         self::assertSame('A3', $copysheet->getSelectedCells());
@@ -132,7 +132,6 @@ public function testCopyClone2(string $type, bool $suppress, bool $cache, bool $
     {
         $spreadsheet = $this->spreadsheet = new Spreadsheet();
         $calc = $spreadsheet->getCalculationEngine();
-        self::assertNotNull($calc);
         $calc->setSuppressFormulaErrors($suppress);
         $calc->setCalculationCacheEnabled($cache);
         $calc->setBranchPruningEnabled($pruning);
@@ -143,7 +142,6 @@ public function testCopyClone2(string $type, bool $suppress, bool $cache, bool $
             $spreadsheet2 = $this->spreadsheet2 = clone $spreadsheet;
         }
         $calc2 = $spreadsheet2->getCalculationEngine();
-        self::assertNotNull($calc2);
         self::assertSame($suppress, $calc2->getSuppressFormulaErrors());
         self::assertSame($cache, $calc2->getCalculationCacheEnabled());
         self::assertSame($pruning, $calc2->getBranchPruningEnabled());