Proposal: deprecate undefined passed arguments

2015-02-23 16:32 GMT-03:00 Marc Bennewitz dev@mabe.berlin:

Hi all,

Because the feature freeze for PHP 7 is near I like to know your thoughts
about one more change in passing arguments. Sure this one would introduce
one more deprecated message / BC break but this one s for reducing BC break
in the future!

Currently a caller can pass undefined arguments as much he like. Such
passed arguments are only visible by the callee using
func_get_args/func_num_args but the need for such function definitions has
been gone with argument unpacking feature. The other rare possibility for
ignoring passed arguments by callables from functions like array_walk has
been gone, too, with the introduction of clusures.

By the way we already have internal functions triggering warnings on
unknown arguments. This is also an unneeded inconsistency and more it's
invisible by the user without testing each function for it.

At least simply ignoring passed arguments is a violation as described in
the following examples (http://3v4l.org/U2lnf):

class Calculator {
public function calc($v) {
return $v + 1;
}
}

class MyCalculator extends Calculator {
public function calc($v1, $v2 = 0) {
return parent::calc($v1) + $v2;
}
}

function calcArray(array $values, Calculator $calculator) {
foreach ($values as &$v) {
$v = $calculator->calc($v, $v); // Second argument is wrong !!
}
return $values;
}

$ar = [1,2,3];
var_dump(calcArray($ar, new Calculator)); // ignores the second argument
var_dump(calcArray($ar, new MyCalculator)); // UNEXPECTED: the second
argument will be used

Both calculators should be 100% compatible but they aren't as the function
"calcArray" shows.

So because of the described issues with the existing code base I like to
propose the change to no longer ignore undefined arguments and introduce
E_DEPRECATED messages if a function get's an undefined argument. So the
user get enough time to fix their code before throwing errors in this case
in PHP 8.

As this should be consistent over the engine I would propose this change
for user defined functions and for internal functions as well.

Again, yes this one would introduces one more BC break but first this
would be deprecated before disabled and next it's for reducing BC in the
future.

Marc

--

Have you checked https://wiki.php.net/rfc/strict_argcount? Only difference
is that I'm proposing a warning instead of E_DEPRECATE, but it's still a
draft and open to discussion. Maybe you would like to contribute to the RFC
before it reaches discussion this week?

Hi!

So because of the described issues with the existing code base I like to
propose the change to no longer ignore undefined arguments and introduce
E_DEPRECATED messages if a function get's an undefined argument. So the
user get enough time to fix their code before throwing errors in this
case in PHP 8.

I don't think it is a good idea to break all functions that use
func_get_args(). To have a BC break, there should be a larger
improvement in the functionality, compensating for it, but here no
functionality is added, just removed, and removed for apparently no
reason besides trying to be "more strict". I know that's some kind of
fashion lately to introduce limitations in the language that add nothing
but "strictness" but frankly I see no point in this one. If there is
some perceivable improvement in easiness of development, or some added
capabilities which are enabled by this, or some problem that was hard to
solve before but with this change is easy - please describe it, so far I
didn't see it.

As this should be consistent over the engine I would propose this change
for user defined functions and for internal functions as well.

If you are worried about consistency that much, I would propose removing
warning from internal functions, since it is not very useful for any
real purpose anyway - extra argument does not hurt anything, just
produces annoying messages.

Again, yes this one would introduces one more BC break but first this
would be deprecated before disabled and next it's for reducing BC in the
future.

Deprecating is not a magic trick that makes BC breaks OK. Deprecating is
a way to handle BC breaks, but it doesn't make it any better so it can
not be used as an argument for BC break being more acceptable. If BC
break is warranted then we can talk about how to do it, i.e. with
deprecation, etc. but for that we need to first decide that the break is
OK, without relying on specific process for achieving it as an argument.

Also, I don't exactly see how it reduces BC breaks in the future. BC
promise does not include a promise not to add function arguments.

Stas Malyshev
smalyshev@gmail.com