Re: Proposal: deprecate undefined passed arguments

Hi Marcio, Am 23.02.2015 um 21:15 schrieb Marcio Almada:

2015-02-23 16:32 GMT-03:00 Marc Bennewitz <dev@mabe.berlin <mailto: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

    --     PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php

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? Wow, nice to see this - didn't noted it before.

But I have to questions/notes: 1. Why you throw only a warning and not an error (deprecating this feature first). I don't see the benefit. 2. As I understand it correctly passing unknown arguments will be allowed if the function uses one of the|func_[get|num]_arg[s]()| functions. This looks wired to me as the caller should not know the body to know how a function can be called and because of the dynamic nature of PHP this check isn't possible in all cases as you already noted. Additionally such strict check is for detecting wrong calls but this detection doesn't work with code like this: https://github.com/zendframework/zf2/blob/master/library/Zend/Cache/Storage/Adapter/Memcached.php#L213 In my opinion functions using |func_[get|num]_arg[s]() |to implement variadic arguments are totally outdated and should be refactored using new semantic. I also think such code isn't used very often and deprecating it + removing later on (PHP 8) will give more than enough time to update it as it's simple to find and simple to fix. Also to previous example given for ZF2 shows that the function |func_num_args()| have it's rights to exit but i'm unsure about |func_get_arg[s]()|. Marc