From 8d421553fd29552c12378c8f52e519b87ed124b8 Mon Sep 17 00:00:00 2001
From: Jorge Colon <jorge@2upmedia.com>
Date: Thu, 16 Jul 2015 16:19:43 -0400
Subject: [PATCH] discovered SplFileObject::fgetcsv() bugs implemented a
 workaround in the meantime, removed superfluous $line, more unit tests,
 updated README with examples

---
 .travis.yml                        |   7 +
 README.markdown                    |  17 +++
 lib/EasyCSV/AbstractBase.php       |   2 +-
 lib/EasyCSV/Reader.php             |  45 ++++---
 tests/EasyCSV/Tests/ReaderTest.php | 197 +++++++++++++++++++++++------
 tests/bootstrap.php                |   2 +-
 6 files changed, 213 insertions(+), 57 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 264de44..ac5e889 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,5 +5,12 @@ php:
   - 5.5
   - 5.6
   - hhvm
+  - 7.0
 
 before_script: composer install
+
+matrix:
+  allow_failures:
+    - php: hhvm
+    - php: 7.0
+  fast_finish: true
diff --git a/README.markdown b/README.markdown
index 011ff7d..cbf7c7d 100644
--- a/README.markdown
+++ b/README.markdown
@@ -33,6 +33,23 @@ Or you can get everything all at once:
 print_r($reader->getAll());
 ```
 
+If you have a file with the header in a different line:
+
+```php
+// our headers aren't on the first line
+$reader = new \EasyCSV\Reader('read.csv', 'r+', false);
+// zero-based index, so this is line 4
+$reader->setHeaderLine(3);
+```
+
+Advance to a different line:
+
+```
+$reader->advanceTo(6);
+```
+
+More in the Reader unit test.
+
 ## Writer
 
 To write CSV files we need to instantiate the EasyCSV writer class:
diff --git a/lib/EasyCSV/AbstractBase.php b/lib/EasyCSV/AbstractBase.php
index 8d9e7a5..d82c51d 100644
--- a/lib/EasyCSV/AbstractBase.php
+++ b/lib/EasyCSV/AbstractBase.php
@@ -10,7 +10,7 @@ abstract class AbstractBase
 
     public function __construct($path, $mode = 'r+')
     {
-        if ( ! file_exists($path)) {
+        if (! file_exists($path)) {
             touch($path);
         }
         $this->handle = new \SplFileObject($path, $mode);
diff --git a/lib/EasyCSV/Reader.php b/lib/EasyCSV/Reader.php
index 98e3e34..c741015 100644
--- a/lib/EasyCSV/Reader.php
+++ b/lib/EasyCSV/Reader.php
@@ -14,23 +14,18 @@ class Reader extends AbstractBase
      */
     private $headers = false;
 
-    /**
-     * @var int
-     */
-    private $line;
-
     /**
      * @var
      */
     private $init;
 
     /**
-     * @var bool
+     * @var bool|int
      */
     private $headerLine = false;
-    
+
     /**
-     * @var bool
+     * @var bool|int
      */
     private $lastLine = false;
 
@@ -43,7 +38,6 @@ public function __construct($path, $mode = 'r+', $headersInFirstRow = true)
     {
         parent::__construct($path, $mode);
         $this->headersInFirstRow = $headersInFirstRow;
-        $this->line = 0;
     }
 
     /**
@@ -62,17 +56,19 @@ public function getHeaders()
     public function getRow()
     {
         $this->init();
-        if ($this->handle->eof()) {
+        if ($this->isEof()) {
             return false;
         }
 
-        $row = $this->handle->fgetcsv($this->delimiter, $this->enclosure);
+        $row = $this->getCurrentRow();
         $isEmpty = $this->rowIsEmpty($row);
 
-        if ($row !== false && $row != null && $isEmpty === false) {
-            $this->line++;
+        if ($this->isEof() === false) {
+            $this->handle->next();
+        }
 
-            return $this->headers ? array_combine($this->headers, $row) : $row;
+        if ($isEmpty === false) {
+            return ($this->headers && is_array($this->headers)) ? array_combine($this->headers, $row) : $row;
         } elseif ($isEmpty) {
             // empty row, transparently try the next row
             return $this->getRow();
@@ -81,6 +77,14 @@ public function getRow()
         }
     }
 
+    /**
+     * @return bool
+     */
+    public function isEof()
+    {
+        return $this->handle->eof();
+    }
+
     /**
      * @return array
      */
@@ -138,7 +142,13 @@ public function advanceTo($lineNumber)
             throw new \LogicException("Line Number $lineNumber is equal to the header line that was set");
         }
 
-        $this->line = $lineNumber;
+        if ($lineNumber > 0) {
+            $this->handle->seek($lineNumber - 1);
+        } // check the line before
+
+        if ($this->isEof()) {
+            throw new \LogicException("Line Number $lineNumber is past the end of the file");
+        }
 
         $this->handle->seek($lineNumber);
     }
@@ -156,11 +166,10 @@ public function setHeaderLine($lineNumber)
 
         $this->headerLine = $lineNumber;
 
-        // seek to line before headers
         $this->handle->seek($lineNumber);
 
         // get headers
-        $this->headers = $this->getCurrentRow();
+        $this->headers = $this->getRow();
     }
 
     protected function init()
@@ -173,6 +182,8 @@ protected function init()
         if ($this->headersInFirstRow === true) {
             $this->handle->rewind();
 
+            $this->headerLine = 0;
+
             $this->headers = $this->getRow();
         }
     }
diff --git a/tests/EasyCSV/Tests/ReaderTest.php b/tests/EasyCSV/Tests/ReaderTest.php
index 1ec08bf..8ebe165 100644
--- a/tests/EasyCSV/Tests/ReaderTest.php
+++ b/tests/EasyCSV/Tests/ReaderTest.php
@@ -6,40 +6,59 @@
 
 class ReaderTest extends \PHPUnit_Framework_TestCase
 {
-    protected $headerValues = array( "column1", "column2", "column3" );
-    protected $expectedRows = array (
+    protected $headers = array("column1", "column2", "column3");
+
+    protected $expectedRows = array(
         0 =>
-            array (
+            array(
                 'column1' => '1column2value',
                 'column2' => '1column3value',
                 'column3' => '1column4value',
             ),
         1 =>
-            array (
+            array(
                 'column1' => '2column2value',
                 'column2' => '2column3value',
                 'column3' => '2column4value',
             ),
         2 =>
-            array (
+            array(
                 'column1' => '3column2value',
                 'column2' => '3column3value',
                 'column3' => '3column4value',
             ),
         3 =>
-            array (
+            array(
                 'column1' => '4column2value',
                 'column2' => '4column3value',
                 'column3' => '4column4value',
             ),
         4 =>
-            array (
+            array(
                 'column1' => '5column2value',
                 'column2' => '5column3value',
                 'column3' => '5column4value',
             ),
     );
 
+    protected $dataRow1 = array(
+        'column1' => '1column2value',
+        'column2' => '1column3value',
+        'column3' => '1column4value',
+    );
+
+    protected $dataRow2 = array(
+        'column1' => '2column2value',
+        'column2' => '2column3value',
+        'column3' => '2column4value',
+    );
+
+    protected $dataRow5 = array(
+        'column1' => '5column2value',
+        'column2' => '5column3value',
+        'column3' => '5column4value',
+    );
+
     /**
      * @dataProvider getReaders
      */
@@ -51,6 +70,38 @@ public function testOneAtAtime(Reader $reader)
         }
     }
 
+    /**
+     * @dataProvider getReaders
+     */
+    public function testLastRowIsCorrect(Reader $reader)
+    {
+        $reader->getRow();
+        $reader->getRow();
+        $reader->getRow();
+        $reader->getRow();
+        $row = $reader->getRow();
+
+        $expected = $this->dataRow5;
+
+        $this->assertEquals($expected, $row);
+
+        // line number should have not advanced
+        $this->assertEquals(5, $reader->getLineNumber());
+    }
+
+    /**
+     * @dataProvider getReaders
+     */
+    public function testIsEofReturnsCorrectly(Reader $reader)
+    {
+        $this->assertFalse($reader->isEof());
+
+        $reader->getAll();
+
+        $this->assertTrue($reader->isEof());
+        $this->assertFalse($reader->getRow());
+    }
+
     /**
      * @dataProvider getReaders
      */
@@ -64,40 +115,43 @@ public function testGetAll(Reader $reader)
      */
     public function testGetHeaders(Reader $reader)
     {
-        $this->assertEquals( $this->headerValues, $reader->getHeaders());
+        $this->assertEquals($this->headers, $reader->getHeaders());
     }
 
     /**
      * @dataProvider getReaders
      */
-    public function testAdvanceto(Reader $reader)
+    public function testAdvanceTo(Reader $reader)
     {
-        $reader->advanceTo( 3 );
+        $this->assertEquals(0, $reader->getLineNumber());
+
+        // getting a row before calling advanceTo() shouldn't affect it
+        $dataRow1 = $reader->getRow();
+        $this->assertEquals($this->dataRow1, $dataRow1);
+        $this->assertEquals(2, $reader->getLineNumber());
 
-        $this->assertEquals( 3, $reader->getLineNumber() );
+        $reader->advanceTo(3);
 
-        $reader->advanceTo( 0 );
+        $this->assertEquals(3, $reader->getLineNumber());
 
-        $row = array
-        (
-            'column1' => '1column2value',
-            'column2' => '1column3value',
-            'column3' => '1column4value',
-        );
+        $reader->advanceTo(1);
+
+        $row = $this->dataRow1;
 
         $actualRow = $reader->getRow();
-        $this->assertEquals( $row, $actualRow );
+        $this->assertEquals($row, $actualRow);
+        $this->assertEquals(2, $reader->getLineNumber());
 
-        $reader->advanceTo( 3 );
+        $reader->advanceTo(3);
 
-        $row = array
-        (
-            'column1' => '4column2value',
-            'column2' => '4column3value',
-            'column3' => '4column4value',
+        $row = array(
+            'column1' => '3column2value',
+            'column2' => '3column3value',
+            'column3' => '3column4value',
         );
 
-        $this->assertEquals( $row, $reader->getRow() );
+        $this->assertEquals($row, $reader->getRow());
+        $this->assertEquals(4, $reader->getLineNumber());
     }
 
     /**
@@ -105,21 +159,87 @@ public function testAdvanceto(Reader $reader)
      */
     public function testAdvanceToNoHeadersFirstRow(Reader $reader)
     {
-        $row = array (
+        $firstMetaRow = array(
             0 => 'Some Meta Data',
             1 => '',
             2 => '',
         );
+        $secondMetaRow = array(
+            0 => "Field: Value",
+            1 => '',
+            2 => '',
+        );
+
+        $actualRow = $reader->getRow();
+        $this->assertEquals($firstMetaRow, $actualRow);
+        $this->assertEquals(1, $reader->getLineNumber());
 
         $actualRow = $reader->getRow();
-        $this->assertEquals( $row, $actualRow );
+        $this->assertEquals($secondMetaRow, $actualRow);
+        $this->assertEquals(2, $reader->getLineNumber());
 
         // give it the ol' one-two-switcharoo
         $reader->advanceTo(3);
+        $advancedRow = $reader->getRow();
+
+        $this->assertEquals(
+            $this->headers,
+            $advancedRow
+        );
+
+        $reader->advanceTo(0);
+
+        $this->assertEquals($firstMetaRow, $reader->getRow());
+        $this->assertEquals(1, $reader->getLineNumber());
+
+        $this->assertEquals($secondMetaRow, $reader->getRow());
+        $this->assertEquals(2, $reader->getLineNumber());
+
+        $reader->advanceTo(1);
+
+        $row = $reader->getRow();
+        $this->assertEquals(2, $reader->getLineNumber());
+        $this->assertEquals(
+            $secondMetaRow,
+            $row
+        );
+    }
+
+    /**
+     * @dataProvider getReaders
+     */
+    public function testAdvanceToLastLine(Reader $reader)
+    {
+        $reader->advanceTo(5);
+    }
+
+    /**
+     * @dataProvider getReadersNoHeadersFirstRow
+     * @expectedException \LogicException
+     */
+    public function testAdvanceToBeforeHeaderLineNoHeadersFirstRow(Reader $reader)
+    {
+        $reader->setHeaderLine(3);
+        $reader->advanceTo(1);
+    }
+
+    /**
+     * @dataProvider getReaders
+     * @expectedException \LogicException
+     */
+    public function testAdvanceToHeaderLine(Reader $reader)
+    {
         $reader->getRow();
         $reader->advanceTo(0);
+    }
 
-        $this->assertEquals( $row, $reader->getRow() );
+    /**
+     * @dataProvider getReaders
+     * @expectedException \LogicException
+     */
+    public function testAdvanceToPastEof(Reader $reader)
+    {
+        $reader->advanceTo(999);
     }
 
     /**
@@ -127,13 +247,13 @@ public function testAdvanceToNoHeadersFirstRow(Reader $reader)
      */
     public function testSetHeaderLine(Reader $reader)
     {
-        $headers = $this->headerValues;
+        $headers = $this->headers;
 
-        $this->assertEquals( $headers, $reader->getHeaders() );
+        $this->assertEquals($headers, $reader->getHeaders());
 
         $reader->setHeaderLine(0);
 
-        $this->assertEquals( $headers, $reader->getHeaders() );
+        $this->assertEquals($headers, $reader->getHeaders());
     }
 
     /**
@@ -142,9 +262,9 @@ public function testSetHeaderLine(Reader $reader)
     public function testSetHeaderLineNoHeadersFirstRow(Reader $reader)
     {
         // set headers
-        $reader->setHeaderLine( 3 );
+        $reader->setHeaderLine(3);
 
-        $this->assertEquals( $this->headerValues, $reader->getHeaders() );
+        $this->assertEquals($this->headers, $reader->getHeaders());
 
         $rows = $reader->getAll();
 
@@ -157,7 +277,8 @@ public function testSetHeaderLineNoHeadersFirstRow(Reader $reader)
      */
     public function testGetLastLineNumber(Reader $reader)
     {
-        $this->assertEquals( 5, $reader->getLastLineNumber() );
+        $this->assertEquals(5, $reader->getLastLineNumber());
+        $this->assertEquals(5, $reader->getLastLineNumber());
     }
 
     /**
@@ -165,7 +286,7 @@ public function testGetLastLineNumber(Reader $reader)
      */
     public function testGetLastLineNumberNoHeadersFirstRow(Reader $reader)
     {
-        $this->assertEquals( 10, $reader->getLastLineNumber() );
+        $this->assertEquals(10, $reader->getLastLineNumber());
     }
 
     public function getReaders()
@@ -181,11 +302,11 @@ public function getReaders()
 
     public function getReadersNoHeadersFirstRow()
     {
-        $readerSemiColon = new Reader(__DIR__ . '/read_header_line_sc.csv', 'r+', false );
+        $readerSemiColon = new Reader(__DIR__ . '/read_header_line_sc.csv', 'r+', false);
         $readerSemiColon->setDelimiter(';');
 
         return array(
-            array(new Reader(__DIR__ . '/read_header_line.csv', 'r+', false )),
+            array(new Reader(__DIR__ . '/read_header_line.csv', 'r+', false)),
             array($readerSemiColon),
         );
     }
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
index 85526bc..75ade52 100644
--- a/tests/bootstrap.php
+++ b/tests/bootstrap.php
@@ -2,7 +2,7 @@
 
 require_once dirname(__DIR__) . '/vendor/autoload.php';
 
-call_user_func(function() {
+call_user_func(function () {
     $loader = new \Composer\Autoload\ClassLoader();
     $loader->add('EasyCSV\Test', __DIR__);
     $loader->register();