Add iterateOnlyExistingCells to Constructors (#3727)

* Add iterateOnlyExistingCells to Constructors

Fix #3721. That issue can already be handled, but requires several statements when one ought to suffice. Adding an extra parameter to the RowCellIterator and ColumnCellIterator constructors is useful, easy, and becomes especially practical now that our supported Php releases all support named parameters (see new test Issue3721Test).

* Update Column.php

* Update .php-cs-fixer.dist.php

* Update Row.php

* Update Column.php

* Update ByColumnAndRowTest.php

.php-cs-fixer.dist.php

@@ -132,7 +132,7 @@ $config
         'no_unneeded_final_method' => true,
         'no_unreachable_default_argument_value' => true,
         'no_unset_cast' => true,
-        'no_unset_on_property' => true,
+        'no_unset_on_property' => false,
         'no_unused_imports' => true,
         'no_useless_else' => true,
         'no_useless_return' => true,

src/PhpSpreadsheet/Worksheet/Column.php

@@ -4,7 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Worksheet;
 
 class Column
 {
-    private ?Worksheet $worksheet;
+    private Worksheet $worksheet;
 
     /**
      * Column index.
@@ -28,7 +28,7 @@ class Column
      */
     public function __destruct()
     {
-        $this->worksheet = null;
+        unset($this->worksheet);
     }
 
     /**
@@ -45,9 +45,9 @@ class Column
      * @param int $startRow The row number at which to start iterating
      * @param int $endRow Optionally, the row number at which to stop iterating
      */
-    public function getCellIterator($startRow = 1, $endRow = null): ColumnCellIterator
+    public function getCellIterator($startRow = 1, $endRow = null, bool $iterateOnlyExistingCells = false): ColumnCellIterator
     {
-        return new ColumnCellIterator($this->getWorksheet(), $this->columnIndex, $startRow, $endRow);
+        return new ColumnCellIterator($this->worksheet, $this->columnIndex, $startRow, $endRow, $iterateOnlyExistingCells);
     }
 
     /**
@@ -56,9 +56,9 @@ class Column
      * @param int $startRow The row number at which to start iterating
      * @param int $endRow Optionally, the row number at which to stop iterating
      */
-    public function getRowIterator($startRow = 1, $endRow = null): ColumnCellIterator
+    public function getRowIterator($startRow = 1, $endRow = null, bool $iterateOnlyExistingCells = false): ColumnCellIterator
     {
-        return $this->getCellIterator($startRow, $endRow);
+        return $this->getCellIterator($startRow, $endRow, $iterateOnlyExistingCells);
     }
 
     /**
@@ -108,7 +108,6 @@ class Column
      */
     public function getWorksheet(): Worksheet
     {
-        // @phpstan-ignore-next-line
         return $this->worksheet;
     }
 }

src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php

@@ -43,7 +43,7 @@ class ColumnCellIterator extends CellIterator
      * @param int $startRow The row number at which to start iterating
      * @param int $endRow Optionally, the row number at which to stop iterating
      */
-    public function __construct(Worksheet $worksheet, $columnIndex = 'A', int $startRow = 1, $endRow = null)
+    public function __construct(Worksheet $worksheet, $columnIndex = 'A', $startRow = 1, $endRow = null, bool $iterateOnlyExistingCells = false)
     {
         // Set subject
         $this->worksheet = $worksheet;
@@ -51,6 +51,7 @@ class ColumnCellIterator extends CellIterator
         $this->columnIndex = Coordinate::columnIndexFromString($columnIndex);
         $this->resetEnd($endRow);
         $this->resetStart($startRow);
+        $this->setIterateOnlyExistingCells($iterateOnlyExistingCells);
     }
 
     /**

src/PhpSpreadsheet/Worksheet/Row.php

@@ -4,7 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Worksheet;
 
 class Row
 {
-    private ?Worksheet $worksheet;
+    private Worksheet $worksheet;
 
     /**
      * Row index.
@@ -30,7 +30,7 @@ class Row
      */
     public function __destruct()
     {
-        $this->worksheet = null;
+        unset($this->worksheet);
     }
 
     /**
@@ -47,9 +47,9 @@ class Row
      * @param string $startColumn The column address at which to start iterating
      * @param string $endColumn Optionally, the column address at which to stop iterating
      */
-    public function getCellIterator($startColumn = 'A', $endColumn = null): RowCellIterator
+    public function getCellIterator($startColumn = 'A', $endColumn = null, bool $iterateOnlyExistingCells = false): RowCellIterator
     {
-        return new RowCellIterator($this->getWorksheet(), $this->rowIndex, $startColumn, $endColumn);
+        return new RowCellIterator($this->worksheet, $this->rowIndex, $startColumn, $endColumn, $iterateOnlyExistingCells);
     }
 
     /**
@@ -58,9 +58,9 @@ class Row
      * @param string $startColumn The column address at which to start iterating
      * @param string $endColumn Optionally, the column address at which to stop iterating
      */
-    public function getColumnIterator($startColumn = 'A', $endColumn = null): RowCellIterator
+    public function getColumnIterator($startColumn = 'A', $endColumn = null, bool $iterateOnlyExistingCells = false): RowCellIterator
     {
-        return $this->getCellIterator($startColumn, $endColumn);
+        return $this->getCellIterator($startColumn, $endColumn, $iterateOnlyExistingCells);
     }
 
     /**
@@ -110,7 +110,6 @@ class Row
      */
     public function getWorksheet(): Worksheet
     {
-        // @phpstan-ignore-next-line
         return $this->worksheet;
     }
 }

src/PhpSpreadsheet/Worksheet/RowCellIterator.php

@@ -45,7 +45,7 @@ class RowCellIterator extends CellIterator
      * @param string $startColumn The column address at which to start iterating
      * @param string $endColumn Optionally, the column address at which to stop iterating
      */
-    public function __construct(Worksheet $worksheet, $rowIndex = 1, string $startColumn = 'A', $endColumn = null)
+    public function __construct(Worksheet $worksheet, $rowIndex = 1, $startColumn = 'A', $endColumn = null, bool $iterateOnlyExistingCells = false)
     {
         // Set subject and row index
         $this->worksheet = $worksheet;
@@ -53,6 +53,7 @@ class RowCellIterator extends CellIterator
         $this->rowIndex = $rowIndex;
         $this->resetEnd($endColumn);
         $this->resetStart($startColumn);
+        $this->setIterateOnlyExistingCells($iterateOnlyExistingCells);
     }
 
     /**

tests/PhpSpreadsheetTests/Reader/Ods/Issue3721Test.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;
+
+use PhpOffice\PhpSpreadsheet\Reader\Ods;
+use PHPUnit\Framework\TestCase;
+
+class Issue3721Test extends TestCase
+{
+    public function testIssue2810(): void
+    {
+        // Problems with getHighestDataColumn
+        $filename = 'tests/data/Reader/Ods/issue.3721.ods';
+        $reader = new Ods();
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getSheetByNameOrThrow('sheet with data');
+        $origHigh = $sheet->getHighestDataColumn();
+        self::assertSame('C', $origHigh);
+        $cells = [];
+        foreach ($sheet->getRowIterator() as $row) {
+            foreach ($row->getCellIterator(iterateOnlyExistingCells: true) as $cell) {
+                $cells[] = $cell->getCoordinate();
+            }
+        }
+        self::assertSame(['A1', 'B1', 'C1', 'A2', 'B2', 'C2'], $cells);
+        self::assertSame('C', $sheet->getHighestDataColumn());
+        self::assertSame('BL', $sheet->getHighestColumn());
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Worksheet/ByColumnAndRowTest.php

@@ -204,8 +204,7 @@ class ByColumnAndRowTest extends TestCase
             ->getComment('B2')
             ->getText()->createTextRun('My Test Comment');
 
-        $comment /** @scrutinizer ignore-deprecated */
-        = $sheet->getCommentByColumnAndRow(2, 2);
+        $comment = /** @scrutinizer ignore-deprecated */ $sheet->getCommentByColumnAndRow(2, 2);
         self::assertInstanceOf(Comment::class, $comment);
         self::assertSame('My Test Comment', $comment->getText()->getPlainText());
         $spreadsheet->disconnectWorksheets();