[9.x] Fix LazyCollection::makeIterator() to accept non Generator Function

[fuwasegu]

Introduction

LazyCollection accepts a generator function instead of a Generator in its constructor so that it can be called many times.
However, there is no logic implemented to check at runtime whether a received closure is really a generator function.
So, for example, if you pass the following Closure to LazyCollection::make()

$lazy = LazyCollection::make(fn (): string => "foo");

Calling LazyCollection::getIterator() will result in the following error

Illuminate\Support\LazyCollection::getIterator(): Return value must be of type Traversable, string returned

This is because LazyCollection::makeIterator() called in LazyCollection::getIterator() returns the result of executing the function even if it is not a generator function.

What changed?

The original code looked like this

protected function makeIterator($source)
{
    if ($source instanceof IteratorAggregate) {
        return $source->getIterator();
    }

    if (is_array($source)) {
        return new ArrayIterator($source);
    }

    return $source();
}

It is not known if $source is callable in the first place, and even if it is, it is not known if it returns Traversable or not.
Therefore, the following modifications have been made

protected function makeIterator($source)
{
    if ($source instanceof IteratorAggregate) {
        return $source->getIterator();
    }

    if (is_array($source)) {
        return new ArrayIterator($source);
    }

    if (is_callable($source)) {
        $maybeTraversable = $source();
        return $maybeTraversable instanceof Traversable
            ? $maybeTraversable
            : new ArrayIterator((array)$maybeTraversable);
    }

    return new ArrayIterator((array)$source);
}

I considered throwing an exception if $source was not a generator function, but seeing that LazyCollection::getArrayableItems() converts $item to an array of any value, I decided that makeIterator should do the same I decided that makeIterator should do the same.

Why wasn't this done in the constructor?

If you want to achieve the above in the constructor, use

• Use ReflectionFunction::isGenerator(), etc.
• Actually execute the function and check the return type.

However, I wanted to avoid the overhead of using Reflection as much as possible, and I also wanted to avoid wasting data in memory when a closure is executed in the constructor and an Iterator is returned.

thank you.