On Wed, Oct 21, 2020 at 1:01 AM tyson andre <tysonandre...@hotmail.com>
wrote:
> Hi internals,
>
> Currently, if there is no constructor, php handles it just like
> `__construct(...$args) {}`,
> both for positional and named parameters. Is there any interest in
> changing that to be a deprecation warning if one or more parameters are
> passed?
> (or if 1 or more named parameters are passed)
>
> The default behavior of missing constructors would result in somewhat
> surprising behavior
> for `new $className(paramName: 123)` if this was used unexpectedly
> (silently ignoring it)
> (Desired behavior would be to deprecate/warn/throw)
>
> ```
> php > class A {}
> php > var_export(new A(paramName: 123));
> A::__set_state(array(
> ))
> php > var_export(new stdClass(paramName: 123));
> (object) array(
> )
> php > var_export(new stdClass(123));
> (object) array(
> )
> ```
>
> (I don't think https://wiki.php.net/rfc/named_params mentioned this
> (the current behavior follows from handling of extra positional
> parameters),
> and this mistake is probably unlikely in practice,
> but if it were to occur it would very likely be a bug)
>
> PHP already throws ArgumentCountError for passing too many parameters for
> internal methods
> so the default method not throwing is a surprise. I haven't yet looked
> into how much code would break if this were changed.
>
> Aside: If a static analyzer such as Phan, Psalm, or PHPStan are set up,
> those bugs could be detected easily,
> so the impact seems low, but I wonder if this type of bug could cause
> confusion (or feel unexpected)
> for those who are more familiar with other languages than they are with
> php.
>
> Thanks,
> - Tyson
>
Thanks for bringing this up Tyson!
There's two ways to interpret how this works:
1. We're calling an implicit constructor with signature __construct() {},
in which case unknown named args should not be accepted.
2. We're calling an implicit constructor with signature
__construct(...$args) {}, in which case unknown named args should be
accepted.
I believe we should be following 1. here and have put up
https://github.com/php/php-src/pull/6364 to fix this issue. (This actually
happens to match what we were doing prior to PHP 8, I changed the signature
of the pass function for unrelated reasons without considering this
interaction.)
As to the case of normal positional arguments, I agree that we should move
towards making that an error as well, but this will need a deprecation
phase, as it is long standing behavior. There's also the more general
question of passing additional arguments to userland functions, but that
one is a lot more controversial.
Regards,
Nikita
