Eliminate Some Phpstan Annotations (#3746)

* Eliminate Some Phpstan Annotations Some changes are possible due to Php8+ features like null-safe operators and Stringable. * Possible Uninitialized Variable Scrutinizer might be technically correct. * CellAddress Static -> Self I don't understand the use case for extending it, which would be the reason for using static rather than self. * Update Worksheet.php
PHPOffice · Sep 30, 2023 · d0fae3f · d0fae3f
1 parent e53e44d
commit d0fae3f
Show file tree

Hide file tree

Showing 20 changed files with 129 additions and 57 deletions.
diff --git a/src/PhpSpreadsheet/Cell/AdvancedValueBinder.php b/src/PhpSpreadsheet/Cell/AdvancedValueBinder.php
@@ -67,7 +67,7 @@ public function bindValue(Cell $cell, $value = null)
                 $currencyCode = $matches['PrefixedCurrency'] ?? $matches['PostfixedCurrency'];
                 $value = (float) ($sign . trim(str_replace([$decimalSeparatorNoPreg, $currencyCode, ' ', '-'], ['.', '', '', ''], preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', $value)))); // @phpstan-ignore-line
 
-                return $this->setCurrency($value, $cell, $currencyCode); // @phpstan-ignore-line
+                return $this->setCurrency($value, $cell, $currencyCode ?? '');
             }
 
             // Check for time without seconds e.g. '9:45', '09:45'

diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php
@@ -296,9 +296,9 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
 
         $this->updateInCollection();
         $cellCoordinate = $this->getCoordinate();
-        self::updateIfCellIsTableHeader($this->getParent()->getParent(), $this, $oldValue, $value); // @phpstan-ignore-line
+        self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value);
 
-        return $this->getParent()->get($cellCoordinate); // @phpstan-ignore-line
+        return $this->getParent()?->get($cellCoordinate) ?? $this;
     }
 
     public const CALCULATE_DATE_TIME_ASIS = 0;

diff --git a/src/PhpSpreadsheet/Cell/CellAddress.php b/src/PhpSpreadsheet/Cell/CellAddress.php
@@ -27,7 +27,7 @@ public function __construct(string $cellAddress, ?Worksheet $worksheet = null)
 
     public function __destruct()
     {
-        $this->worksheet = null;
+        unset($this->worksheet);
     }
 
     /**
@@ -45,21 +45,19 @@ public static function fromColumnAndRow(mixed $columnId, mixed $rowId, ?Workshee
     {
         self::validateColumnAndRow($columnId, $rowId);
 
-        /** @phpstan-ignore-next-line */
-        return new static(Coordinate::stringFromColumnIndex($columnId) . ((string) $rowId), $worksheet);
+        return new self(Coordinate::stringFromColumnIndex($columnId) . ((string) $rowId), $worksheet);
     }
 
     public static function fromColumnRowArray(array $array, ?Worksheet $worksheet = null): self
     {
         [$columnId, $rowId] = $array;
 
-        return static::fromColumnAndRow($columnId, $rowId, $worksheet);
+        return self::fromColumnAndRow($columnId, $rowId, $worksheet);
     }
 
     public static function fromCellAddress(mixed $cellAddress, ?Worksheet $worksheet = null): self
     {
-        /** @phpstan-ignore-next-line */
-        return new static($cellAddress, $worksheet);
+        return new self($cellAddress, $worksheet);
     }
 
     /**
@@ -115,7 +113,7 @@ public function nextRow(int $offset = 1): self
             $newRowId = 1;
         }
 
-        return static::fromColumnAndRow($this->columnId, $newRowId);
+        return self::fromColumnAndRow($this->columnId, $newRowId);
     }
 
     public function previousRow(int $offset = 1): self
@@ -130,7 +128,7 @@ public function nextColumn(int $offset = 1): self
             $newColumnId = 1;
         }
 
-        return static::fromColumnAndRow($newColumnId, $this->rowId);
+        return self::fromColumnAndRow($newColumnId, $this->rowId);
     }
 
     public function previousColumn(int $offset = 1): self

diff --git a/src/PhpSpreadsheet/Cell/Coordinate.php b/src/PhpSpreadsheet/Cell/Coordinate.php
@@ -124,7 +124,7 @@ public static function absoluteCoordinate(string $cellAddress)
         }
 
         // Create absolute coordinate
-        [$column, $row] = self::coordinateFromString($cellAddress); // @phpstan-ignore-line
+        [$column, $row] = self::coordinateFromString($cellAddress ?? 'A1');
         $column = ltrim($column, '$');
         $row = ltrim($row, '$');
 
@@ -148,13 +148,12 @@ public static function splitRange($range)
         }
 
         $exploded = explode(',', $range);
-        $counter = count($exploded);
-        for ($i = 0; $i < $counter; ++$i) {
-            // @phpstan-ignore-next-line
-            $exploded[$i] = explode(':', $exploded[$i]);
+        $outArray = [];
+        foreach ($exploded as $value) {
+            $outArray[] = explode(':', $value);
         }
 
-        return $exploded;
+        return $outArray;
     }
 
     /**
@@ -361,7 +360,7 @@ public static function extractAllCellReferencesInRange($cellRange): array
             }
             $worksheet = str_replace("'", "''", $worksheet);
         }
-        [$ranges, $operators] = self::getCellBlocksFromRangeString($cellRange); // @phpstan-ignore-line
+        [$ranges, $operators] = self::getCellBlocksFromRangeString($cellRange ?? 'A1');
 
         $cells = [];
         foreach ($ranges as $range) {
@@ -571,8 +570,7 @@ private static function getCellBlocksFromRangeString($rangeString)
         $rangeString = str_replace('$', '', strtoupper($rangeString));
 
         // split range sets on intersection (space) or union (,) operators
-        $tokens = preg_split('/([ ,])/', $rangeString, -1, PREG_SPLIT_DELIM_CAPTURE);
-        /** @phpstan-ignore-next-line */
+        $tokens = preg_split('/([ ,])/', $rangeString, -1, PREG_SPLIT_DELIM_CAPTURE) ?: [];
         $split = array_chunk($tokens, 2);
         $ranges = array_column($split, 0);
         $operators = array_column($split, 1);

diff --git a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php
@@ -3,8 +3,10 @@
 namespace PhpOffice\PhpSpreadsheet\Cell;
 
 use DateTimeInterface;
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\RichText\RichText;
 use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
+use Stringable;
 
 class DefaultValueBinder implements IValueBinder
 {
@@ -21,14 +23,14 @@ public function bindValue(Cell $cell, $value)
         // sanitize UTF-8 strings
         if (is_string($value)) {
             $value = StringHelper::sanitizeUTF8($value);
-        } elseif (is_object($value)) {
-            // Handle any objects that might be injected
-            if ($value instanceof DateTimeInterface) {
-                $value = $value->format('Y-m-d H:i:s');
-            } elseif (!($value instanceof RichText)) {
-                // Attempt to cast any unexpected objects to string
-                $value = (string) $value; // @phpstan-ignore-line
-            }
+        } elseif ($value === null || is_scalar($value) || $value instanceof RichText) {
+            // No need to do anything
+        } elseif ($value instanceof DateTimeInterface) {
+            $value = $value->format('Y-m-d H:i:s');
+        } elseif ($value instanceof Stringable) {
+            $value = (string) $value;
+        } else {
+            throw new SpreadsheetException('Unable to bind unstringable ' . gettype($value));
         }
 
         // Set value explicit

diff --git a/src/PhpSpreadsheet/Cell/StringValueBinder.php b/src/PhpSpreadsheet/Cell/StringValueBinder.php
@@ -3,8 +3,10 @@
 namespace PhpOffice\PhpSpreadsheet\Cell;
 
 use DateTimeInterface;
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\RichText\RichText;
 use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
+use Stringable;
 
 class StringValueBinder implements IValueBinder
 {
@@ -82,6 +84,9 @@ public function bindValue(Cell $cell, $value): bool
         if (is_object($value)) {
             return $this->bindObjectValue($cell, $value);
         }
+        if ($value !== null && !is_scalar($value)) {
+            throw new SpreadsheetException('Unable to bind unstringable ' . gettype($value));
+        }
 
         // sanitize UTF-8 strings
         if (is_string($value)) {
@@ -111,14 +116,15 @@ protected function bindObjectValue(Cell $cell, object $value): bool
         // Handle any objects that might be injected
         if ($value instanceof DateTimeInterface) {
             $value = $value->format('Y-m-d H:i:s');
+            $cell->setValueExplicit($value, DataType::TYPE_STRING);
         } elseif ($value instanceof RichText) {
             $cell->setValueExplicit($value, DataType::TYPE_INLINE);
-
-            return true;
+        } elseif ($value instanceof Stringable) {
+            $cell->setValueExplicit((string) $value, DataType::TYPE_STRING);
+        } else {
+            throw new SpreadsheetException('Unable to bind unstringable object of type ' . get_class($value));
         }
 
-        $cell->setValueExplicit((string) $value, DataType::TYPE_STRING); // @phpstan-ignore-line
-
         return true;
     }
 }
diff --git a/src/PhpSpreadsheet/Chart/Chart.php b/src/PhpSpreadsheet/Chart/Chart.php
@@ -671,9 +671,10 @@ public function render($outputDestination = null)
         // Ensure that data series values are up-to-date before we render
         $this->refresh();
 
+        /** @var Renderer\IRenderer */
         $renderer = new $libraryName($this);
 
-        return $renderer->render($outputDestination); // @phpstan-ignore-line
+        return $renderer->render($outputDestination);
     }
 
     public function getRotX(): ?int

diff --git a/src/PhpSpreadsheet/Chart/Renderer/IRenderer.php b/src/PhpSpreadsheet/Chart/Renderer/IRenderer.php
@@ -14,7 +14,7 @@ public function __construct(Chart $chart);
     /**
      * Render the chart to given file (or stream).
      *
-     * @param string $filename Name of the file render to
+     * @param ?string $filename Name of the file render to
      *
      * @return bool true on success
      */

diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -496,7 +496,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
         $rels = $this->loadZip(self::INITIAL_FILE, Namespaces::RELATIONSHIPS);
 
         $propertyReader = new PropertyReader($this->getSecurityScannerOrThrow(), $excel->getProperties());
-        $chartDetails = [];
+        $charts = $chartDetails = [];
         foreach ($rels->Relationship as $relx) {
             $rel = self::getAttributes($relx);
             $relTarget = (string) $rel['Target'];
@@ -1743,12 +1743,14 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                         // Modify range, and extract the first worksheet reference
                                         // Need to split on a comma or a space if not in quotes, and extract the first part.
                                         $definedNameValueParts = preg_split("/[ ,](?=([^']*'[^']*')*[^']*$)/miuU", $extractedRange);
-                                        // Extract sheet name
-                                        [$extractedSheetName] = Worksheet::extractSheetTitle((string) $definedNameValueParts[0], true); // @phpstan-ignore-line
-                                        $extractedSheetName = trim($extractedSheetName, "'"); // @phpstan-ignore-line
+                                        if (is_array($definedNameValueParts)) {
+                                            // Extract sheet name
+                                            [$extractedSheetName] = Worksheet::extractSheetTitle((string) $definedNameValueParts[0], true);
+                                            $extractedSheetName = trim((string) $extractedSheetName, "'");
 
-                                        // Locate sheet
-                                        $locatedSheet = $excel->getSheetByName($extractedSheetName);
+                                            // Locate sheet
+                                            $locatedSheet = $excel->getSheetByName($extractedSheetName);
+                                        }
                                     }
 
                                     if ($locatedSheet === null && !DefinedName::testIfFormula($extractedRange)) {
@@ -1790,8 +1792,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                             $objChart = $chartReader->readChart($chartElements, basename($chartEntryRef, '.xml'));
                             if (isset($charts[$chartEntryRef])) {
                                 $chartPositionRef = $charts[$chartEntryRef]['sheet'] . '!' . $charts[$chartEntryRef]['id'];
-                                if (isset($chartDetails[$chartPositionRef])) {
-                                    $excel->getSheetByName($charts[$chartEntryRef]['sheet'])->addChart($objChart); // @phpstan-ignore-line
+                                if (isset($chartDetails[$chartPositionRef]) && $excel->getSheetByName($charts[$chartEntryRef]['sheet']) !== null) {
+                                    $excel->getSheetByName($charts[$chartEntryRef]['sheet'])->addChart($objChart);
                                     $objChart->setWorksheet($excel->getSheetByName($charts[$chartEntryRef]['sheet']));
                                     // For oneCellAnchor or absoluteAnchor positioned charts,
                                     //     toCoordinate is not in the data. Does it need to be calculated?

diff --git a/src/PhpSpreadsheet/Reader/Xlsx/BaseParserClass.php b/src/PhpSpreadsheet/Reader/Xlsx/BaseParserClass.php
@@ -2,12 +2,14 @@
 
 namespace PhpOffice\PhpSpreadsheet\Reader\Xlsx;
 
+use Stringable;
+
 class BaseParserClass
 {
     protected static function boolean(mixed $value): bool
     {
         if (is_object($value)) {
-            $value = (string) $value; // @phpstan-ignore-line
+            $value = ($value instanceof Stringable) ? ((string) $value) : 'true';
         }
 
         if (is_numeric($value)) {

diff --git a/src/PhpSpreadsheet/Worksheet/CellIterator.php b/src/PhpSpreadsheet/Worksheet/CellIterator.php
@@ -55,8 +55,7 @@ abstract class CellIterator implements NativeIterator
      */
     public function __destruct()
     {
-        // @phpstan-ignore-next-line
-        $this->worksheet = $this->cellCollection = null;
+        unset($this->worksheet, $this->cellCollection);
     }
 
     public function getIfNotExists(): bool

diff --git a/src/PhpSpreadsheet/Worksheet/ColumnIterator.php b/src/PhpSpreadsheet/Worksheet/ColumnIterator.php
@@ -60,8 +60,7 @@ public function __construct(Worksheet $worksheet, string $startColumn = 'A', $en
      */
     public function __destruct()
     {
-        // @phpstan-ignore-next-line
-        $this->worksheet = null;
+        unset($this->worksheet);
     }
 
     /**

diff --git a/src/PhpSpreadsheet/Worksheet/RowIterator.php b/src/PhpSpreadsheet/Worksheet/RowIterator.php
@@ -55,7 +55,7 @@ public function __construct(Worksheet $subject, int $startRow = 1, $endRow = nul
 
     public function __destruct()
     {
-        $this->subject = null; // @phpstan-ignore-line
+        unset($this->subject);
     }
 
     /**

diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
@@ -2841,8 +2841,8 @@ private function writeCFRule(
             $bFormatAlign = 0;
         }
         // Protection
-        $bProtLocked = ($conditional->getStyle()->getProtection()->getLocked() == null ? 1 : 0);
-        $bProtHidden = ($conditional->getStyle()->getProtection()->getHidden() == null ? 1 : 0);
+        $bProtLocked = ($conditional->getStyle()->getProtection()->getLocked() === null ? 1 : 0);
+        $bProtHidden = ($conditional->getStyle()->getProtection()->getHidden() === null ? 1 : 0);
         if ($bProtLocked == 0 || $bProtHidden == 0) {
             $bFormatProt = 1;
         } else {

diff --git a/tests/PhpSpreadsheetTests/Calculation/StructuredReferenceFormulaTest.php b/tests/PhpSpreadsheetTests/Calculation/StructuredReferenceFormulaTest.php
@@ -41,7 +41,7 @@ public function testStructuredReferenceHiddenHeaders(): void
         $result = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();
         self::assertSame('Region', $result);
 
-        $spreadsheet->getCalculationEngine()->flushInstance(); // @phpstan-ignore-line
+        $spreadsheet->getCalculationEngine()?->flushInstance();
         $table->setShowHeaderRow(false);
 
         $result = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();

diff --git a/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php b/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php
@@ -8,6 +8,7 @@
 use DateTimeImmutable;
 use PhpOffice\PhpSpreadsheet\Cell\DataType;
 use PhpOffice\PhpSpreadsheet\Cell\DefaultValueBinder;
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\RichText\RichText;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
@@ -48,6 +49,29 @@ public static function binderProvider(): array
         ];
     }
 
+    public function testNonStringableBindValue(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+
+        try {
+            $sheet->getCell('A1')->setValue($this);
+            self::fail('Did not receive expected Exception');
+        } catch (SpreadsheetException $e) {
+            self::assertStringContainsString('Unable to bind unstringable', $e->getMessage());
+        }
+        $sheet->getCell('A2')->setValue(new StringableObject());
+        self::assertSame('abc', $sheet->getCell('A2')->getValue());
+
+        try {
+            $sheet->getCell('A3')->setValue(fopen(__FILE__, 'rb'));
+            self::fail('Did not receive expected Exception');
+        } catch (SpreadsheetException $e) {
+            self::assertStringContainsString('Unable to bind unstringable', $e->getMessage());
+        }
+        $spreadsheet->disconnectWorksheets();
+    }
+
     /**
      * @dataProvider providerDataTypeForValue
      */

diff --git a/tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php b/tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php
@@ -9,6 +9,7 @@
 use PhpOffice\PhpSpreadsheet\Cell\Cell;
 use PhpOffice\PhpSpreadsheet\Cell\DataType;
 use PhpOffice\PhpSpreadsheet\Cell\StringValueBinder;
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\RichText\RichText;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
@@ -73,6 +74,30 @@ public static function providerDataValuesDefault(): array
         ];
     }
 
+    public function testNonStringableBindValue(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        Cell::setValueBinder(new StringValueBinder());
+
+        try {
+            $sheet->getCell('A1')->setValue($this);
+            self::fail('Did not receive expected Exception');
+        } catch (SpreadsheetException $e) {
+            self::assertStringContainsString('Unable to bind unstringable', $e->getMessage());
+        }
+        $sheet->getCell('A2')->setValue(new StringableObject());
+        self::assertSame('abc', $sheet->getCell('A2')->getValue());
+
+        try {
+            $sheet->getCell('A3')->setValue([1, 2, 3]);
+            self::fail('Did not receive expected Exception');
+        } catch (SpreadsheetException $e) {
+            self::assertStringContainsString('Unable to bind unstringable', $e->getMessage());
+        }
+        $spreadsheet->disconnectWorksheets();
+    }
+
     /**
      * @dataProvider providerDataValuesSuppressNullConversion
      */
-Original file line number
+Diff line change
@@ Expand Up @@
         public function __destruct()
         {
-            $this->subject = null; // @phpstan-ignore-line
+            unset($this->subject);
         }
         /**
@@ Expand Down @@