Idea: Warning about named parameters with a missing constructor? View as Markdown

On Wed, Oct 21, 2020 at 1:01 AM tyson andre tysonandre775@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