From 5e090d1af08ec407af7245b22193daa97c6cd340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dalibor=20Karlovi=C4=87?= Date: Tue, 7 Aug 2018 09:38:18 +0200 Subject: [PATCH] Allow iterators to go out of bounds with prev() Iterators prev() behavior is now consistent with next(), meaning that it can go out of bounds and it must be validated with valid() before using it. Fixes #587 Fixes #627 --- CHANGELOG.md | 6 ++++ .../Worksheet/ColumnCellIterator.php | 6 +--- .../Worksheet/ColumnIterator.php | 7 +---- src/PhpSpreadsheet/Worksheet/Iterator.php | 4 +-- .../Worksheet/RowCellIterator.php | 5 +--- src/PhpSpreadsheet/Worksheet/RowIterator.php | 8 +----- .../Worksheet/ColumnCellIteratorTest.php | 3 +- .../Worksheet/ColumnIteratorTest.php | 3 +- .../Worksheet/IteratorTest.php | 28 +++++++++++++++++++ .../Worksheet/RowCellIteratorTest.php | 3 +- .../Worksheet/RowIteratorTest.php | 3 +- 11 files changed, 44 insertions(+), 32 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Worksheet/IteratorTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 42e6fc20..9b8518d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] + +### Fixed + +- Allow iterators to go out of bounds with prev - [#587](https://github.com/PHPOffice/PhpSpreadsheet/issues/587) + ## [1.4.0] - 2018-08-06 ### Added diff --git a/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php b/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php index a4cdb6f4..7e8f040d 100644 --- a/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php @@ -153,10 +153,6 @@ class ColumnCellIterator extends CellIterator */ public function prev() { - if ($this->currentRow <= $this->startRow) { - throw new PhpSpreadsheetException("Row is already at the beginning of range ({$this->startRow} - {$this->endRow})"); - } - do { --$this->currentRow; } while (($this->onlyExistingCells) && @@ -171,7 +167,7 @@ class ColumnCellIterator extends CellIterator */ public function valid() { - return $this->currentRow <= $this->endRow; + return $this->currentRow <= $this->endRow && $this->currentRow >= $this->startRow; } /** diff --git a/src/PhpSpreadsheet/Worksheet/ColumnIterator.php b/src/PhpSpreadsheet/Worksheet/ColumnIterator.php index 6fdfe3ba..d2b57aad 100644 --- a/src/PhpSpreadsheet/Worksheet/ColumnIterator.php +++ b/src/PhpSpreadsheet/Worksheet/ColumnIterator.php @@ -157,14 +157,9 @@ class ColumnIterator implements \Iterator /** * Set the iterator to its previous value. - * - * @throws PhpSpreadsheetException */ public function prev() { - if ($this->currentColumnIndex <= $this->startColumnIndex) { - throw new PhpSpreadsheetException('Column is already at the beginning of range (' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ' - ' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ')'); - } --$this->currentColumnIndex; } @@ -175,6 +170,6 @@ class ColumnIterator implements \Iterator */ public function valid() { - return $this->currentColumnIndex <= $this->endColumnIndex; + return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex; } } diff --git a/src/PhpSpreadsheet/Worksheet/Iterator.php b/src/PhpSpreadsheet/Worksheet/Iterator.php index 311808c0..d8797a34 100644 --- a/src/PhpSpreadsheet/Worksheet/Iterator.php +++ b/src/PhpSpreadsheet/Worksheet/Iterator.php @@ -25,7 +25,7 @@ class Iterator implements \Iterator * * @param Spreadsheet $subject */ - public function __construct(Spreadsheet $subject = null) + public function __construct(Spreadsheet $subject) { // Set subject $this->subject = $subject; @@ -82,6 +82,6 @@ class Iterator implements \Iterator */ public function valid() { - return $this->position < $this->subject->getSheetCount(); + return $this->position < $this->subject->getSheetCount() && $this->position >= 0; } } diff --git a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php index 23d9b9fd..59ef329c 100644 --- a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php @@ -155,9 +155,6 @@ class RowCellIterator extends CellIterator */ public function prev() { - if ($this->currentColumnIndex <= $this->startColumnIndex) { - throw new PhpSpreadsheetException('Column is already at the beginning of range (' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ' - ' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ')'); - } do { --$this->currentColumnIndex; } while (($this->onlyExistingCells) && (!$this->worksheet->cellExistsByColumnAndRow($this->currentColumnIndex, $this->rowIndex)) && ($this->currentColumnIndex >= $this->startColumnIndex)); @@ -170,7 +167,7 @@ class RowCellIterator extends CellIterator */ public function valid() { - return $this->currentColumnIndex <= $this->endColumnIndex; + return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex; } /** diff --git a/src/PhpSpreadsheet/Worksheet/RowIterator.php b/src/PhpSpreadsheet/Worksheet/RowIterator.php index 70b7b8f7..433cea6a 100644 --- a/src/PhpSpreadsheet/Worksheet/RowIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowIterator.php @@ -152,15 +152,9 @@ class RowIterator implements \Iterator /** * Set the iterator to its previous value. - * - * @throws PhpSpreadsheetException */ public function prev() { - if ($this->position <= $this->startRow) { - throw new PhpSpreadsheetException("Row is already at the beginning of range ({$this->startRow} - {$this->endRow})"); - } - --$this->position; } @@ -171,6 +165,6 @@ class RowIterator implements \Iterator */ public function valid() { - return $this->position <= $this->endRow; + return $this->position <= $this->endRow && $this->position >= $this->startRow; } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php index 46880e2e..39756740 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php @@ -78,9 +78,8 @@ class ColumnCellIteratorTest extends TestCase public function testPrevOutOfRange() { - $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); - $iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4); $iterator->prev(); + self::assertFalse($iterator->valid()); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/ColumnIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/ColumnIteratorTest.php index 7285961d..04c626d1 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/ColumnIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/ColumnIteratorTest.php @@ -77,9 +77,8 @@ class ColumnIteratorTest extends TestCase public function testPrevOutOfRange() { - $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); - $iterator = new ColumnIterator($this->mockWorksheet, 'B', 'D'); $iterator->prev(); + self::assertFalse($iterator->valid()); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/IteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/IteratorTest.php new file mode 100644 index 00000000..80a2d783 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/IteratorTest.php @@ -0,0 +1,28 @@ +createSheet(); + $spreadsheet->createSheet(); + + $iterator = new Iterator($spreadsheet); + $columnIndexResult = 0; + self::assertEquals($columnIndexResult, $iterator->key()); + + foreach ($iterator as $key => $column) { + self::assertEquals($columnIndexResult++, $key); + self::assertInstanceOf(Worksheet::class, $column); + } + self::assertSame(3, $columnIndexResult); + } +} diff --git a/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php index 9346d0a1..a10c7aa5 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php @@ -80,9 +80,8 @@ class RowCellIteratorTest extends TestCase public function testPrevOutOfRange() { - $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); - $iterator = new RowCellIterator($this->mockWorksheet, 2, 'B', 'D'); $iterator->prev(); + self::assertFalse($iterator->valid()); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/RowIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/RowIteratorTest.php index 63e26b34..cb0b12d1 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/RowIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/RowIteratorTest.php @@ -75,9 +75,8 @@ class RowIteratorTest extends TestCase public function testPrevOutOfRange() { - $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); - $iterator = new RowIterator($this->mockWorksheet, 2, 4); $iterator->prev(); + self::assertFalse($iterator->valid()); } }