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
This commit is contained in:
Dalibor Karlović 2018-08-07 09:38:18 +02:00 committed by Adrien Crivelli
parent 125f462a71
commit 5e090d1af0
No known key found for this signature in database
GPG Key ID: B182FD79DC6DE92E
11 changed files with 44 additions and 32 deletions

View File

@ -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/) The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/). 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 ## [1.4.0] - 2018-08-06
### Added ### Added

View File

@ -153,10 +153,6 @@ class ColumnCellIterator extends CellIterator
*/ */
public function prev() public function prev()
{ {
if ($this->currentRow <= $this->startRow) {
throw new PhpSpreadsheetException("Row is already at the beginning of range ({$this->startRow} - {$this->endRow})");
}
do { do {
--$this->currentRow; --$this->currentRow;
} while (($this->onlyExistingCells) && } while (($this->onlyExistingCells) &&
@ -171,7 +167,7 @@ class ColumnCellIterator extends CellIterator
*/ */
public function valid() public function valid()
{ {
return $this->currentRow <= $this->endRow; return $this->currentRow <= $this->endRow && $this->currentRow >= $this->startRow;
} }
/** /**

View File

@ -157,14 +157,9 @@ class ColumnIterator implements \Iterator
/** /**
* Set the iterator to its previous value. * Set the iterator to its previous value.
*
* @throws PhpSpreadsheetException
*/ */
public function prev() 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; --$this->currentColumnIndex;
} }
@ -175,6 +170,6 @@ class ColumnIterator implements \Iterator
*/ */
public function valid() public function valid()
{ {
return $this->currentColumnIndex <= $this->endColumnIndex; return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex;
} }
} }

View File

@ -25,7 +25,7 @@ class Iterator implements \Iterator
* *
* @param Spreadsheet $subject * @param Spreadsheet $subject
*/ */
public function __construct(Spreadsheet $subject = null) public function __construct(Spreadsheet $subject)
{ {
// Set subject // Set subject
$this->subject = $subject; $this->subject = $subject;
@ -82,6 +82,6 @@ class Iterator implements \Iterator
*/ */
public function valid() public function valid()
{ {
return $this->position < $this->subject->getSheetCount(); return $this->position < $this->subject->getSheetCount() && $this->position >= 0;
} }
} }

View File

@ -155,9 +155,6 @@ class RowCellIterator extends CellIterator
*/ */
public function prev() 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 { do {
--$this->currentColumnIndex; --$this->currentColumnIndex;
} while (($this->onlyExistingCells) && (!$this->worksheet->cellExistsByColumnAndRow($this->currentColumnIndex, $this->rowIndex)) && ($this->currentColumnIndex >= $this->startColumnIndex)); } 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() public function valid()
{ {
return $this->currentColumnIndex <= $this->endColumnIndex; return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex;
} }
/** /**

View File

@ -152,15 +152,9 @@ class RowIterator implements \Iterator
/** /**
* Set the iterator to its previous value. * Set the iterator to its previous value.
*
* @throws PhpSpreadsheetException
*/ */
public function prev() 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; --$this->position;
} }
@ -171,6 +165,6 @@ class RowIterator implements \Iterator
*/ */
public function valid() public function valid()
{ {
return $this->position <= $this->endRow; return $this->position <= $this->endRow && $this->position >= $this->startRow;
} }
} }

View File

@ -78,9 +78,8 @@ class ColumnCellIteratorTest extends TestCase
public function testPrevOutOfRange() public function testPrevOutOfRange()
{ {
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
$iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4); $iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4);
$iterator->prev(); $iterator->prev();
self::assertFalse($iterator->valid());
} }
} }

View File

@ -77,9 +77,8 @@ class ColumnIteratorTest extends TestCase
public function testPrevOutOfRange() public function testPrevOutOfRange()
{ {
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
$iterator = new ColumnIterator($this->mockWorksheet, 'B', 'D'); $iterator = new ColumnIterator($this->mockWorksheet, 'B', 'D');
$iterator->prev(); $iterator->prev();
self::assertFalse($iterator->valid());
} }
} }

View File

@ -0,0 +1,28 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\Iterator;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PHPUnit\Framework\TestCase;
class IteratorTest extends TestCase
{
public function testIteratorFullRange()
{
$spreadsheet = new Spreadsheet();
$spreadsheet->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);
}
}

View File

@ -80,9 +80,8 @@ class RowCellIteratorTest extends TestCase
public function testPrevOutOfRange() public function testPrevOutOfRange()
{ {
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
$iterator = new RowCellIterator($this->mockWorksheet, 2, 'B', 'D'); $iterator = new RowCellIterator($this->mockWorksheet, 2, 'B', 'D');
$iterator->prev(); $iterator->prev();
self::assertFalse($iterator->valid());
} }
} }

View File

@ -75,9 +75,8 @@ class RowIteratorTest extends TestCase
public function testPrevOutOfRange() public function testPrevOutOfRange()
{ {
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
$iterator = new RowIterator($this->mockWorksheet, 2, 4); $iterator = new RowIterator($this->mockWorksheet, 2, 4);
$iterator->prev(); $iterator->prev();
self::assertFalse($iterator->valid());
} }
} }