I'm just beginning to hopefully become a regular contributor for Laravel, and I noticed there was a lot of PHP parameter types that are missing in some files (I assume for backwards compatibility reasons when we supported PHP 5). As PHP 7.3 is the current minimum version allowed, I figured this would be an easy first task. Is this an improvement I can start working on?
Will this break code?
Consider the following function in Container:
/**
* Register a binding with the container.
*
* @param string $abstract
* @param \Closure|string|null $concrete
* @param bool $shared
* @return void
*/
public function bind($abstract, $concrete = null, $shared = false)
Category 1: Non-breaking change
public function bind($abstract, mixed $concrete = null, $shared = false): void
Non breaking changes usually include:
Changing return types
mixed parameter type (once min version is PHP 8)
Types to internal closures
Types to private functions
Types to protected functions (unless the class is extended by the developer, but I guess that is the case with almost any change. Perhaps it should only be considered if the class is unlikely to be extended)
Category 2: Unlikely breaking change
A category 2 change would look like:
public function bind(string $abstract, mixed $concrete = null, bool $shared = false): void
There is very low likelihood that this will cause problems, unless the user is calling the function wrong (in which case they should know).
Category 3: Breaking change
This is where introducing types could break a function that is called in a valid way. This can happen in cases which can handle variable number of parameters. Care needs to be taken that changes like this is not being done. Obviously!
What about functions that is coincidentally working now, but will break later on if we introduce types?
Here is an example of a function call that would break in the next major version of Laravel for Category 2 type changes, that is just coincidentally working:
$container->bind(5, Greeting::class);
Here we are sending an integer for the $abstract parameter. This coincidentally works due to the way PHP arrays works. But lets say later down the line we want to change the internal data structure to say a trie tree, or a new faster PHP hash map data structure that relies on string keys. This wouldn't normally be a breaking change, but would break the above code. It is better the developer knows now about the mistake than later on.
What's the benefit?
Parameter types have a lot of serious benefits that aid development.
For example, someone could mistakenly think the 3rd parameter is how the object is initialised (like I did when I was new to Laravel) and do this:
$container->bind('greeting', Greeting::class, function () {
return new Greeting("Hello");
}); // INCORRECT USAGE
This currently runs without error. Debugging this can also be quite a pain. With type hinting it will raise an error, and show the exact line where the mistake is.
Argument 3 passed to Illuminate\Container\Container::bind() must be of the type bool, object given, called in /.../app/Console/Commands/TestCommand.php on line 57
Other benefits include better IDE auto-complete and error checking (see images below).
PHPStorm without Type Hints
PHPStorm with Type Hints
Just to be clear. This would have to go in a major version (e.g. Laravel 9).
So what do you think? Shall I start working on it, starting with Container.
Hi,
I'm just beginning to hopefully become a regular contributor for Laravel, and I noticed there was a lot of PHP parameter types that are missing in some files (I assume for backwards compatibility reasons when we supported PHP 5). As PHP 7.3 is the current minimum version allowed, I figured this would be an easy first task. Is this an improvement I can start working on?
Will this break code?
Consider the following function in Container:
Category 1: Non-breaking change
Non breaking changes usually include:
mixedparameter type (once min version is PHP 8)Category 2: Unlikely breaking change A category 2 change would look like:
There is very low likelihood that this will cause problems, unless the user is calling the function wrong (in which case they should know).
Category 3: Breaking change This is where introducing types could break a function that is called in a valid way. This can happen in cases which can handle variable number of parameters. Care needs to be taken that changes like this is not being done. Obviously!
What about functions that is coincidentally working now, but will break later on if we introduce types?
Here is an example of a function call that would break in the next major version of Laravel for Category 2 type changes, that is just coincidentally working:
Here we are sending an integer for the
$abstractparameter. This coincidentally works due to the way PHP arrays works. But lets say later down the line we want to change the internal data structure to say a trie tree, or a new faster PHP hash map data structure that relies on string keys. This wouldn't normally be a breaking change, but would break the above code. It is better the developer knows now about the mistake than later on.What's the benefit?
Parameter types have a lot of serious benefits that aid development. For example, someone could mistakenly think the 3rd parameter is how the object is initialised (like I did when I was new to Laravel) and do this:
This currently runs without error. Debugging this can also be quite a pain. With type hinting it will raise an error, and show the exact line where the mistake is.
Other benefits include better IDE auto-complete and error checking (see images below).
PHPStorm without Type Hints
PHPStorm with Type Hints
Just to be clear. This would have to go in a major version (e.g. Laravel 9).
So what do you think? Shall I start working on it, starting with
Container.