Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/CsvReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -135,7 +138,7 @@ public function current(): ?array
$this->errors[$this->key()] = $line;
$this->next();
}
} while($this->valid());
}

return null;
}
Expand Down
91 changes: 91 additions & 0 deletions tests/CsvReaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down