Changed CollectionTrait::shuffle() to return all elements in collection.

[othercorey]

Fixes #14757

[othercorey]

gethostbynamel() is returning 2 addresses for 'localhost' now:

1) Cake\Test\TestCase\Network\SocketTest::testAddresses
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
     0 => '127.0.0.1'
+    1 => '127.0.0.1'
 )

[ADmad]

While this is for a bug fix, it's a change in behavior that could surprise people, so safer to target 4.1 and document in migration guide.

[othercorey]

I added it to 4.0.x in case we wanted to backport to 3.9.x. Up to you though.

[markstory]

Dropping the keys is both good and bad. It will fix one but and potentially create another for existing users. One benefit of this change is it makes the behavior consistent with PHP's shuffle() method which also drops keys.

[othercorey]

All I can say is this is the current documentation for shuffle(). To me, that suggests that you want all the elements. If users need to have unique keys only, then they can re-create the collection first.

     * Returns a new collection with the elements placed in a random order,
     * this function does not preserve the original keys in the collection.

[ADmad]

Here's an alternate fix which doesn't unnecessarily reset keys for all collections:

diff --git a/src/Collection/CollectionTrait.php b/src/Collection/CollectionTrait.php
index 1a7690c652..12e113da83 100644
--- a/src/Collection/CollectionTrait.php
+++ b/src/Collection/CollectionTrait.php
@@ -330,7 +330,12 @@ trait CollectionTrait
      */
     public function shuffle(): CollectionInterface
     {
-        $elements = $this->toArray();
+        $preserveKeys = true;
+        $iterator = $this->unwrap();
+        if (get_class($iterator) === AppendIterator::class) {
+            $preserveKeys = false;
+        }
+        $elements = $this->toArray($preserveKeys);
         shuffle($elements);
 
         return $this->newCollection($elements);

[othercorey]

The shuffle will reset keys regardless.

[markstory]

Dropping the keys is both good and bad. It will fix one but and potentially create another for existing users. One benefit of this change is it makes the behavior consistent with PHP's shuffle() method which also drops keys.

[markstory]

It might also be good to assert that there is no a key anymore.

[ADmad]

The shuffle will reset keys regardless.

In that case I rescind my alternative fix :)