Fix Collection throwing an exception when Traversable given.

src/Collection/CollectionInterface.php

@@ -1019,7 +1019,7 @@ interface CollectionInterface extends Iterator, JsonSerializable
      * losing any possible transformations. This is used mainly to remove empty
      * IteratorIterator wrappers that can only slowdown the iteration process.
      *
-     * @return \Iterator
+     * @return \Traversable
      */
     public function unwrap();
 

src/Collection/CollectionTrait.php

@@ -335,7 +335,7 @@ trait CollectionTrait
      */
     public function take($size = 1, $from = 0)
     {
-        return new Collection(new LimitIterator($this->unwrap(), $from, $size));
+        return new Collection(new LimitIterator($this, $from, $size));
     }
 
     /**
@@ -343,7 +343,7 @@ trait CollectionTrait
      */
     public function skip($howMany)
     {
-        return new Collection(new LimitIterator($this->unwrap(), $howMany));
+        return new Collection(new LimitIterator($this, $howMany));
     }
 
     /**
@@ -367,7 +367,8 @@ trait CollectionTrait
      */
     public function first()
     {
-        foreach ($this->take(1) as $result) {
+        $iterator = new LimitIterator($this, 0, 1);
+        foreach ($iterator as $result) {
             return $result;
         }
     }
@@ -377,18 +378,25 @@ trait CollectionTrait
      */
     public function last()
     {
-        $iterator = $this->unwrap();
-        $count = $iterator instanceof Countable ?
-            count($iterator) :
-            iterator_count($iterator);
+        $iterator = $this->optimizeUnwrap();
+        if (is_array($iterator)) {
+            return array_pop($iterator);
+        }
 
-        if ($count === 0) {
-            return null;
+        if ($iterator instanceof Countable) {
+            $count = count($iterator);
+            if ($count === 0) {
+                return null;
+            }
+            $iterator = new LimitIterator($iterator, $count - 1, 1);
         }
 
-        foreach ($this->take(1, $count - 1) as $last) {
-            return $last;
+        $result = null;
+        foreach ($iterator as $result) {
+            // No-op
         }
+
+        return $result;
     }
 
     /**
@@ -716,7 +724,7 @@ trait CollectionTrait
     /**
      * Backwards compatible wrapper for unwrap()
      *
-     * @return \Iterator
+     * @return \Traversable
      * @deprecated
      */
     // @codingStandardsIgnoreLine

tests/TestCase/Collection/CollectionTest.php

@@ -914,6 +914,29 @@ class CollectionTest extends TestCase
     }
 
     /**
+     * Tests the sample() method with a traversable non-iterator
+     *
+     * @return void
+     */
+    public function testSampleWithTraversableNonIterator()
+    {
+        $collection = new Collection($this->datePeriod('2017-01-01', '2017-01-07'));
+        $result = $collection->sample(3)->toList();
+        $list = [
+            '2017-01-01',
+            '2017-01-02',
+            '2017-01-03',
+            '2017-01-04',
+            '2017-01-05',
+            '2017-01-06',
+        ];
+        $this->assertCount(3, $result);
+        foreach ($result as $date) {
+            $this->assertContains($date->format('Y-m-d'), $list);
+        }
+    }
+
+    /**
      * Test toArray method
      *
      * @return void
@@ -1004,6 +1027,23 @@ class CollectionTest extends TestCase
     }
 
     /**
+     * Tests the take() method with a traversable non-iterator
+     *
+     * @return void
+     */
+    public function testTakeWithTraversableNonIterator()
+    {
+        $collection = new Collection($this->datePeriod('2017-01-01', '2017-01-07'));
+        $result = $collection->take(3, 1)->toList();
+        $expected = [
+            new \DateTime('2017-01-02'),
+            new \DateTime('2017-01-03'),
+            new \DateTime('2017-01-04'),
+        ];
+        $this->assertEquals($expected, $result);
+    }
+
+    /**
      * Tests match
      *
      * @return void
@@ -1913,6 +1953,36 @@ class CollectionTest extends TestCase
     }
 
     /**
+     * Tests the skip() method with a traversable non-iterator
+     *
+     * @return void
+     */
+    public function testSkipWithTraversableNonIterator()
+    {
+        $collection = new Collection($this->datePeriod('2017-01-01', '2017-01-07'));
+        $result = $collection->skip(3)->toList();
+        $expected = [
+            new \DateTime('2017-01-04'),
+            new \DateTime('2017-01-05'),
+            new \DateTime('2017-01-06'),
+        ];
+        $this->assertEquals($expected, $result);
+    }
+
+    /**
+     * Tests the first() method with a traversable non-iterator
+     *
+     * @return void
+     */
+    public function testFirstWithTraversableNonIterator()
+    {
+        $collection = new Collection($this->datePeriod('2017-01-01', '2017-01-07'));
+        $date = $collection->first();
+        $this->assertInstanceOf('DateTime', $date);
+        $this->assertEquals('2017-01-01', $date->format('Y-m-d'));
+    }
+
+    /**
      * Tests the last() method
      *
      * @return void
@@ -1933,13 +2003,60 @@ class CollectionTest extends TestCase
      *
      * @return void
      */
-    public function testLAstWithEmptyCollection()
+    public function testLastWithEmptyCollection()
     {
         $collection = new Collection([]);
         $this->assertNull($collection->last());
     }
 
     /**
+     * Tests the last() method with a countable object
+     *
+     * @return void
+     */
+    public function testLastWithCountable()
+    {
+        $collection = new Collection(new ArrayObject([1, 2, 3]));
+        $this->assertEquals(3, $collection->last());
+    }
+
+    /**
+     * Tests the last() method with an empty countable object
+     *
+     * @return void
+     */
+    public function testLastWithEmptyCountable()
+    {
+        $collection = new Collection(new ArrayObject([]));
+        $this->assertNull($collection->last());
+    }
+
+    /**
+     * Tests the last() method with a non-rewindable iterator
+     *
+     * @return void
+     */
+    public function testLastWithNonRewindableIterator()
+    {
+        $iterator = new NoRewindIterator(new ArrayIterator([1, 2, 3]));
+        $collection = new Collection($iterator);
+        $this->assertEquals(3, $collection->last());
+    }
+
+    /**
+     * Tests the last() method with a traversable non-iterator
+     *
+     * @return void
+     */
+    public function testLastWithTraversableNonIterator()
+    {
+        $collection = new Collection($this->datePeriod('2017-01-01', '2017-01-07'));
+        $date = $collection->last();
+        $this->assertInstanceOf('DateTime', $date);
+        $this->assertEquals('2017-01-06', $date->format('Y-m-d'));
+    }
+
+    /**
      * Tests sumOf with no parameters
      *
      * @return void
@@ -2383,4 +2500,16 @@ class CollectionTest extends TestCase
             yield $k => $v;
         }
     }
+
+    /**
+     * Create a DatePeriod object.
+     *
+     * @param string $start Start date
+     * @param string $end End date
+     * @return \DatePeriod
+     */
+    protected function datePeriod($start, $end)
+    {
+        return new \DatePeriod(new \DateTime($start), new \DateInterval('P1D'), new \DateTime($end));
+    }
 }