[8.x] Fixes for PHP 8.1 in Illuminate\Support

[jrmajor]

Fixes passing null to non-nullable parameters.

Would you accept a PR to 9.x changing the following default parameters?

- Collection::implode($value, $glue = null)
+ Collection::implode($value, $glue = '')

- Stringable::substrCount($needle, $offset = null, $length = null)
+ Stringable::substrCount($needle, $offset = 0, $length = null)

Note that Str::substrCount() already has default $offset = 0.

[driesvints]

@jrmajor yes those changes for 9.x seem okay to me. @taylorotwell for you as well?

[GrahamCampbell]

Would you accept a PR to 9.x changing the following default parameters?

I don't like those. I always prefer defaults as null where possible, so people can skip over passing a param by passing null, knowing the correct default will be used.

[jrmajor]

@GrahamCampbell @driesvints

I don't like those. I always prefer defaults as null where possible, so people can skip over passing a param by passing null, knowing the correct default will be used.

Since PHP 8.0 has been released, named arguments can be used for skipping parameters, so as more and more people upgrade, this stops being an important problem.

Correct defaults help you understand what the method does. For example, if you're not sure whether Collection::implode($value, $glue = null) uses an empty string or a space as the default $glue, you will need to look into its source code to figure it out. With $glue = '' its behavior is obvious by just looking at the method signature, which is also shown by IDE without even jumping to another file.

[GrahamCampbell]

Since PHP 8.0 has been released, named arguments can be used for skipping parameters, so as more and more people upgrade, this stops being an important problem.

Laravel does not support or recommend named parameters, in the sense that they are not included in the BC promise, and could be broken in patch relases.

[jrmajor]

@GrahamCampbell Even taking this into account, it's still easier to see what's the default value (you'd need to do this anyway to see if it's a null) and pass this default, than to navigate to another file and read the entire method to find out what default will be used.

By the way, it's a shame that Laravel does not support named arguments. Do you plan to support it in future releases? With little or no type hints, and now no support for named parameters, it seems like Laravel is falling behind a bit.

[selcukcukur]

@GrahamCampbell Even taking this into account, it's still easier to see what's the default value (you'd need to do this anyway to see if it's a null) and pass this default, than to navigate to another file and read the entire method to find out what default will be used.

By the way, it's a shame that Laravel does not support named arguments. Do you plan to support it in future releases? With little or no type hints, and now no support for named parameters, it seems like Laravel is falling behind a bit.

This has nothing to do with being left behind.

@jrmajor Laravel is a framework that aims to provide the best performance and perfect flexibility with the least code possible, so they add by calculating even a single letter they add. The things they add are also very careful.

[jrmajor]

@selcukcukur How does not supporting named arguments provide more flexibility? It's preventing users from using new language features, that's why I've written about “falling behind”. Hope that they will be supported in the future releases though. Also, the ability to pass objects to functions that expect a string is a little bit more of flexibility than I would usually need.

[driesvints]

@jrmajor we decided against it because it would mean we sometimes would need to wait a year for a new major release to rename parameters. We don't want to be bound by that.