DI injecting new object instances for nullable parameters

[lupinitylabs]

Hi!

I'm not sure whether this is a bug or a feature, so I'm starting a discussion on this first.

Say you create a fresh component using artisan make:component Test and edit the constructor as follows:

    public function __construct(string $title = null, Collection $tags = null, string $body = null)
    {
        dd($title, $tags, $body);
    }

And subsequently add <x-test /> to your template.

My expection would be to see a dump saying null, null, null in this case.
But in reality, the DI creates a new Collection instance for $tags and thus the output is null, Illuminate\Support\Collection {#260 #items: []}, null.

My question is: is this a bug or a feature? Why does the DI even care for nullable parameters? Why should it even care?

(Just for completeness: using ?Collection does not change the behavior either.)

[Alanaktion]

I've run into this myself too and find it very annoying. This really seems like the wrong behavior to me.

It completely defeats the purpose of making the argument nullable, since it will literally never be null unless you're manually instantiating the class. Taylor's comment on #33955 makes sense in that it would be a major breaking change, but this seems like a major design flaw in the DI to me, since a large feature of the PHP language is just unusable on anything instantiated by the framework.

The "solution" is very dumb because you have to just not use PHP anymore to work around it, but you can use PHPDoc comments instead of native types:

/**
 * @param null|Collection $tags
 */
public function __construct(string $title = null, $tags = null, string $body = null)
{
    dd($title, $tags, $body);
}

I would still personally love to see an actual framework change for this, since the DI behavior here makes no sense to me. If an argument is nullable, presumably the author wants a null value if nothing is explicitly passed, since that's the entire point of making an argument nullable.

[kmuenkel]

It's also making a false assumption that the content of a newly-instantiated dependency will be neutral. I just ran across a bug in which a class was expecting a nullable instance of Carbon. If no date was available to the caller, it leaves that argument empty. Sadly when the object is instantiated, a Carbon instance is injected in with the default false presumption that the current date should be set on it. Now I need to write some kind of special-case handler or an override for the app() helper with Reflection magic of my own to avoid this. Otherwise, I'll just have to forego the ability to swap concrete implementations in my Service Provider and just new it up the old fashioned way.