diff --git a/src/CsvReader.php b/src/CsvReader.php index 7423c30..72abfff 100644 --- a/src/CsvReader.php +++ b/src/CsvReader.php @@ -107,8 +107,11 @@ public function current(): ?array return $this->file->current(); } - // Since the CSV has column headers use them to construct an associative array for the columns in this line - do { + // Since the CSV has column headers use them to construct an associative array for the columns in this line. + // Check valid() before current(): SplFileObject::current() returns false at EOF, and a do-while would + // still enter the body once when the iterator is already invalid (e.g. OneToManyReader calls current() + // after next() past the last detail row), causing count(false) TypeError on PHP 8+. + while ($this->valid()) { $line = $this->file->current(); // In non-strict mode pad/slice the line to match the column headers @@ -135,7 +138,7 @@ public function current(): ?array $this->errors[$this->key()] = $line; $this->next(); } - } while($this->valid()); + } return null; } diff --git a/tests/CsvReaderTest.php b/tests/CsvReaderTest.php index acf04ae..156f56a 100644 --- a/tests/CsvReaderTest.php +++ b/tests/CsvReaderTest.php @@ -265,6 +265,97 @@ public function testMaximumNesting() } } + /** + * When the iterator is already at EOF, current() must return null rather than + * calling count() on SplFileObject's false (do-while entered once while invalid). + * + * @see https://github.com/portphp/csv/pull/3 + */ + public function testCurrentAtEndOfFileWithHeadersReturnsNull() + { + $file = new \SplTempFileObject(); + $file->fwrite("id,name\n1,Alice\n2,Bob\n"); + $file->rewind(); + + $reader = new CsvReader($file); + $reader->setHeaderRowNumber(0); + + // Exhaust the iterator + iterator_to_array($reader); + + $this->assertFalse($reader->valid()); + $this->assertNull($reader->current()); + } + + /** + * getRow() past the last line should not TypeError on count(false). + * + * @see https://github.com/portphp/csv/pull/3 + */ + public function testGetRowPastEndWithHeadersReturnsNull() + { + $file = new \SplTempFileObject(); + $file->fwrite("id,name\n1,Alice\n"); + $file->rewind(); + + $reader = new CsvReader($file); + $reader->setHeaderRowNumber(0); + + $this->assertNull($reader->getRow(99)); + } + + /** + * OneToManyReader calls rightReader->current() after next() past the last + * detail row. Without checking valid() first, CsvReader::current() hit + * count(false) and broke joins on the last master row. + * + * This is the real-world failure reported against PR #3. + * + * @see https://github.com/portphp/csv/pull/3 + * @see https://github.com/portphp/csv/pull/3#issuecomment-769855101 + */ + public function testOneToManyReaderConsumesLastDetailRowWithoutError() + { + $masterFile = new \SplTempFileObject(); + $masterFile->fwrite("id,name\n1,Alice\n2,Bob\n"); + $masterFile->rewind(); + $masterReader = new CsvReader($masterFile); + $masterReader->setHeaderRowNumber(0); + + $detailFile = new \SplTempFileObject(); + $detailFile->fwrite("id,item\n1,apple\n1,banana\n2,carrot\n"); + $detailFile->rewind(); + $detailReader = new CsvReader($detailFile); + $detailReader->setHeaderRowNumber(0); + + $reader = new \Port\Reader\OneToManyReader( + $masterReader, + $detailReader, + 'items', + 'id', + 'id' + ); + + $rows = iterator_to_array($reader); + + $this->assertCount(2, $rows); + $this->assertEquals('Alice', $rows[1]['name']); + $this->assertEquals( + array( + array('id' => '1', 'item' => 'apple'), + array('id' => '1', 'item' => 'banana'), + ), + $rows[1]['items'] + ); + $this->assertEquals('Bob', $rows[2]['name']); + $this->assertEquals( + array( + array('id' => '2', 'item' => 'carrot'), + ), + $rows[2]['items'] + ); + } + protected function getReader($filename) { $file = new \SplFileObject(__DIR__.'/fixtures/'.$filename);