thephpleague/csv

TabularDataReader::getRecords()->current() can return NULL

josefsabl opened this issue · 3 comments

Bug Report

Information Description
Version 9.12.0
PHP version 8.3.1
OS Platform Alpine Linux

Summary

In 9.12.0 the \League\Csv\Statement::create()->process('...')->getRecords()->current() started to return NULL even if the records is not empty.

Iterating over iterator works as expected.

Standalone code, or other way to reproduce the problem

var_dump(
    $records = \League\Csv\Statement::create()
        ->process(
            \League\Csv\Reader::createFromString(
                'hello;world'
            )
                ->setDelimiter(';')
        )
        ->getRecords()
        ->current()
);

Expected result

Version <9.11.0 prints:

array(2) {
  [0]=>
  string(5) "hello"
  [1]=>
  string(5) "world"
}

Actual result

Version >=9.12.0 (including master) prints:

NULL

Checks before submitting

  • Be sure that there isn't already an issue about this. See: Issues list
  • Be sure that there isn't already a pull request about this. See: Pull requests
  • I have added every step to reproduce the bug.
  • If possible I added relevant code examples.
  • This issue is about 1 bug and nothing more.
  • The issue has a descriptive title. For example: "JSON rendering failed on Windows for filenames with space".

@josefsabl thanks for using the library.

To me what you are experiencing/describing is not a bug but rather an undocumented and unsupported way of using the package. The method is expected to be used either with a foreach construct or via the iterator_to_array function.

To me the documented way of doing what you were expecting is as follow:

<?php

use League\Csv\Reader;
use League\Csv\Statement;

var_dump(
    $records = Statement::create()
        ->process(
            Reader::createFromString('hello;world')
                ->setDelimiter(';')
        )
-        ->getRecords()
-        ->current()
+        ->first()
);

You may use nth instead of first if you want another specific record see the documentation page for more information on those methods.

Using TabularDataReader::first or TabularDataReader::nth, the return value is always consistent regardless of the version you are using. Hence why those methods are present and in the public API

@josefsabl after consideration a quick fix was added to correct the behaviour. What you are experiencing is due to how PHP handles iterator.
The changes made during 9.12.0 development did not broke the Iterator contract but you are currently getting an un-rewinded Iterator.
To fix your current code you can either wait for 9.15.0 to be release or do the following:

<?php

$records = Statement::create()
    ->process(
        Reader::createFromString('hello;world')
            ->setDelimiter(';')
    )
    ->getRecords();
+ $records->rewind();
$records->current();

When used via foreach or iterator_to_array PHP automatically does the rewind for you otherwise you are responsable for making sure the rewind was performed. Hence why I suggested that the correct way to use getRecords is via those two method/structures.

@josefsabl After much consideration I've decided to go along the POLA principle aka Principle Of Least Astonishment.

This means that I will not consider the current ticket as it being a bug but rather the expected behaviour in PHP language. So there's no need to wait for the next minor version for fixing this behaviour.

Currently to align with PHP behaviour you only have 2 possibilities. Those I gave you in response to this ticket:

  • Explicitly call the rewind method on the returned Iterator;
  • Use the TabularDataReader::nth or TabularDataReader::first methods from the public API.