[8.x] Fixes for PHP 8.1

[jrmajor]

Fixes passing null to non-nullable parameters.

[jrmajor]

This is the default in the constructor, which is bypassed by mocks. The same goes for $prefix.

[taylorotwell]

Why?

[jrmajor]

markTestSkipped() has @psalm-return never-return annotation. There are 48 occurrences of this method in Laravel tests and only those two are followed by a return.

[driesvints]

@jrmajor thanks a bunch for these prs 👍

[selcukcukur]

@jrmajor If there is an update with PHP 8.1 that will require this pull request, I don't fully understand what this update is about?

[jrmajor]

@selcukcukur I'm not sure I've understood your question. PHP 8.1 will be released on November 25, but early dev builds are available for testing, and we want Laravel to be ready for it when it lands. This PR fixes passing null to non-nullable arguments of internal functions, which was deprecated in this RFC. You can see a list of other implemented proposals there.

[selcukcukur]

@selcukcukur I'm not sure I've understood your question. PHP 8.1 will be released on November 25, but early dev builds are available for testing, and we want Laravel to be ready for it when it lands. This PR fixes passing null to non-nullable arguments of internal functions, which was deprecated in this RFC. You can see a list of other implemented proposals there.

I got it, I looked at this document now. So why do we translate an empty string? Wouldn't it make more sense to have these parameters do the string conversion?

Sample :

protected $foo;

public function bar(string $foo = null) 
{
     return (string) $foo;
}

or

protected $foo;

public function bar($foo = null) 
{
    return $foo ? (string) $foo : $foo;
}

or

protected $foo;

public function bar($foo = null) 
{
   if(is_null($foo) {
     return (string) $foo;
   }
}

I still don't understand why we had to make the parameter an empty string. The transformations I have shown above can be easily applied where necessary. Isn't this a better solution than the examples I've shown, am I wrong?

[jrmajor]

The first example makes no sense, no need to cast $foo to string, as it's type hinted as a string and no other value can be passed there. The second one would have a return type of ?string, which I don't think was your intention. Did you mean $foo ?: (string) $foo? The third one makes sense, though it would be easier to write as return $foo ?? ''.

Also, what's the aim of having protected $foo in your examples? It's not used anywhere.

I still don't understand why we had to make the parameter an empty string.

What parameter?

[selcukcukur]

If it will throw a type exception when it takes a "null" value, we can fix it by converting it to a string. I am saying this from the following point of view. It doesn't seem nice to me to define default values for parameters in the class.

[selcukcukur]

Briefly, isn't it a good solution to convert a "null" value to a string instead of defining it?

[jrmajor]

It doesn't seem nice to me to define default values for parameters in the class.

It has already had the default value and it was defined in the constructor. Using mocks, however, bypasses the constructor and this property isn't set to its default. I've written that already in this comment.

Seems pretty nice to me, that's how default properties are meant to be used. It just has been unnecessary, as it was set in the constructor anyway (except for when this class was mocked, which nobody noticed).