Make ResultSet iterator current() not return a mixed value (#1506)

Returning potentially false from the iterator current() means each result set item could be `false` which makes it impractical to safely use it.
When using phpstan, you will get errors like `Cannot call method getId() on bool|Elastica\Result.` when you just iterate over the result set.

Also `current()` should not need to check `valid()` again because that is done automatically by php when using foreach. When valid returns false, the loop is ended. So that is part of the Iterator spec. So using current on a non-existing item should not happen unless you misuse the Iterator.

Author: Tobion

CHANGELOG.md

@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file based on the
 
 ### Backward Compatibility Breaks
 
+* Made result sets adhere to `\Iterator` interface definition that they implement. Specifically, you need to call `valid()` on the result set before calling `current()`. When using `foreach` this is done by PHP automatically. When `valid` returns false, the return value of `current` is undefined instead of false. 
+  * `\Elastica\ResultSet::next` returns `void` instead of `\Elastica\Result|false`
+  * `\Elastica\Bulk\ResponseSet::current` returns `\Elastica\Bulk\Response` instead of `\Elastica\Bulk\Response|false`
+  * `\Elastica\Multi\ResultSet::current` returns `\Elastica\ResultSet` instead of `\Elastica\ResultSet|false`
+
 ### Bugfixes
 
 ### Added

lib/Elastica/Bulk/ResponseSet.php

@@ -95,13 +95,11 @@ public function hasError()
     }
 
     /**
-     * @return bool|\Elastica\Bulk\Response
+     * @return \Elastica\Bulk\Response
      */
     public function current()
     {
-        return $this->valid()
-            ? $this->_bulkResponses[$this->key()]
-            : false;
+        return $this->_bulkResponses[$this->key()];
     }
 
     /**

lib/Elastica/Multi/ResultSet.php

@@ -2,7 +2,6 @@
 namespace Elastica\Multi;
 
 use Elastica\Response;
-use Elastica\ResultSet as BaseResultSet;
 
 /**
  * Elastica multi search result set
@@ -36,8 +35,8 @@ class ResultSet implements \Iterator, \ArrayAccess, \Countable
     /**
      * Constructs ResultSet object.
      *
-     * @param \Elastica\Response $response
-     * @param BaseResultSet[]
+     * @param \Elastica\Response    $response
+     * @param \Elastica\ResultSet[] $resultSets
      */
     public function __construct(Response $response, $resultSets)
     {
@@ -80,13 +79,11 @@ public function hasError()
     }
 
     /**
-     * @return bool|\Elastica\ResultSet
+     * @return \Elastica\ResultSet
      */
     public function current()
     {
-        return $this->valid()
-            ? $this->_resultSets[$this->key()]
-            : false;
+        return $this->_resultSets[$this->key()];
     }
 
     /**

lib/Elastica/ResultSet.php

@@ -236,15 +236,11 @@ public function countSuggests()
     /**
      * Returns the current object of the set.
      *
-     * @return \Elastica\Result|bool Set object or false if not valid (no more entries)
+     * @return \Elastica\Result Set object
      */
     public function current()
     {
-        if ($this->valid()) {
-            return $this->_results[$this->key()];
-        }
-
-        return false;
+        return $this->_results[$this->key()];
     }
 
     /**
@@ -253,8 +249,6 @@ public function current()
     public function next()
     {
         ++$this->_position;
-
-        return $this->current();
     }
 
     /**
@@ -308,7 +302,7 @@ public function offsetExists($offset)
      *
      * @throws Exception\InvalidException If offset doesn't exist
      *
-     * @return Result|null
+     * @return Result
      */
     public function offsetGet($offset)
     {

test/Elastica/Bulk/ResponseSetTest.php

@@ -106,14 +106,6 @@ public function testIterator()
         }
 
         $this->assertFalse($responseSet->valid());
-        $this->assertNotInstanceOf(Bulk\Response::class, $responseSet->current());
-        $this->assertFalse($responseSet->current());
-
-        $responseSet->next();
-
-        $this->assertFalse($responseSet->valid());
-        $this->assertNotInstanceOf(Bulk\Response::class, $responseSet->current());
-        $this->assertFalse($responseSet->current());
 
         $responseSet->rewind();
 
@@ -126,21 +118,12 @@ public function isOkDataProvider()
     {
         list($responseData, $actions) = $this->_getFixture();
 
-        $return = [];
-        $return[] = [$responseData, $actions, true];
+        yield [$responseData, $actions, true];
         $responseData['items'][2]['index']['ok'] = false;
-        $return[] = [$responseData, $actions, false];
-
-        return $return;
+        yield [$responseData, $actions, false];
     }
 
-    /**
-     * @param array $responseData
-     * @param array $actions
-     *
-     * @return ResponseSet
-     */
-    protected function _createResponseSet(array $responseData, array $actions)
+    protected function _createResponseSet(array $responseData, array $actions): ResponseSet
     {
         $client = $this->createMock(Client::class);
 
@@ -155,10 +138,7 @@ protected function _createResponseSet(array $responseData, array $actions)
         return $bulk->send();
     }
 
-    /**
-     * @return array
-     */
-    protected function _getFixture()
+    protected function _getFixture(): array
     {
         $responseData = [
             'took' => 5,

test/Elastica/QueryTest.php

@@ -119,7 +119,8 @@ public function testSetSort()
         $this->assertEquals(2, $resultSet->count());
 
         $first = $resultSet->current()->getData();
-        $second = $resultSet->next()->getData();
+        $resultSet->next();
+        $second = $resultSet->current()->getData();
 
         $this->assertEquals('guschti', $first['firstname']);
         $this->assertEquals('nicolas', $second['firstname']);
@@ -130,7 +131,8 @@ public function testSetSort()
         $this->assertEquals(2, $resultSet->count());
 
         $first = $resultSet->current()->getData();
-        $second = $resultSet->next()->getData();
+        $resultSet->next();
+        $second = $resultSet->current()->getData();
 
         $this->assertEquals('nicolas', $first['firstname']);
         $this->assertEquals('guschti', $second['firstname']);