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

Author: oleibman

.php-cs-fixer.dist.php

@@ -132,7 +132,7 @@
         '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 @@
 
 class Column
 {
-    private ?Worksheet $worksheet;
+    private Worksheet $worksheet;
 
     /**
      * Column index.
@@ -28,7 +28,7 @@ public function __construct(Worksheet $worksheet, $columnIndex = 'A')
      */
     public function __destruct()
     {
-        $this->worksheet = null;
+        unset($this->worksheet);
     }
 
     /**
@@ -45,9 +45,9 @@ public function getColumnIndex(): string
      * @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 @@ public function getCellIterator($startRow = 1, $endRow = null): ColumnCellIterat
      * @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 @@ public function isEmpty(int $definitionOfEmptyFlags = 0, $startRow = 1, $endRow
      */
     public function getWorksheet(): Worksheet
     {
-        // @phpstan-ignore-next-line
         return $this->worksheet;
     }
 }

src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php

@@ -43,14 +43,15 @@ 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;
         $this->cellCollection = $worksheet->getCellCollection();
         $this->columnIndex = Coordinate::columnIndexFromString($columnIndex);
         $this->resetEnd($endRow);
         $this->resetStart($startRow);
+        $this->setIterateOnlyExistingCells($iterateOnlyExistingCells);
     }
 
     /**

src/PhpSpreadsheet/Worksheet/Row.php

@@ -4,7 +4,7 @@
 
 class Row
 {
-    private ?Worksheet $worksheet;
+    private Worksheet $worksheet;
 
     /**
      * Row index.
@@ -30,7 +30,7 @@ public function __construct(Worksheet $worksheet, $rowIndex = 1)
      */
     public function __destruct()
     {
-        $this->worksheet = null;
+        unset($this->worksheet);
     }
 
     /**
@@ -47,9 +47,9 @@ public function getRowIndex(): int
      * @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 @@ public function getCellIterator($startColumn = 'A', $endColumn = null): RowCellI
      * @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 @@ public function isEmpty(int $definitionOfEmptyFlags = 0, $startColumn = 'A', $en
      */
     public function getWorksheet(): Worksheet
     {
-        // @phpstan-ignore-next-line
         return $this->worksheet;
     }
 }

src/PhpSpreadsheet/Worksheet/RowCellIterator.php

@@ -45,14 +45,15 @@ 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;
         $this->cellCollection = $worksheet->getCellCollection();
         $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 @@ public function testGetCommentByColumnAndRow(): void
             ->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();