FAQ
Hi,

I filed a bug report with an attached patch that adds an E_STRICT warning when defining a function with a required parameter after an optional function parameter, for example:

function foo($optional = 1, $required) {}

Although doing this works, code written like that is probably making a faulty assumption somewhere, and emitting this error would help raise the quality of php code.

The bug and patch are here: http://bugs.php.net/bug.php?id=53399

The patch applies against both the PHP 5.3 branch, and trunk. I'm not sure I'd advocate including it in PHP 5.3, but I'd definitely like to see it in 5.4. The patch also includes two tests, and fixes this problem in the Zend/tests/call_user_func_005.phpt test, which is the only test I found that fails as a result.

At some point in the future, I would like to make this a more severe error than an E_STRICT, but I'd rather not immediately break code that (until now) worked without warning.

Thoughts/comments?

-John

--
John Bafford
http://bafford.com/

Search Discussions

  • Etienne Kneuss at Nov 24, 2010 at 5:46 pm

    On Nov 24 12:28:38, John Bafford wrote:
    Hi,

    I filed a bug report with an attached patch that adds an E_STRICT warning when defining a function with a required parameter after an optional function parameter, for example:

    function foo($optional = 1, $required) {}

    Although doing this works, code written like that is probably making a faulty assumption somewhere, and emitting this error would help raise the quality of php code.

    The bug and patch are here: http://bugs.php.net/bug.php?id=53399

    The patch applies against both the PHP 5.3 branch, and trunk. I'm not sure I'd advocate including it in PHP 5.3, but I'd definitely like to see it in 5.4. The patch also includes two tests, and fixes this problem in the Zend/tests/call_user_func_005.phpt test, which is the only test I found that fails as a result.

    At some point in the future, I would like to make this a more severe error than an E_STRICT, but I'd rather not immediately break code that (until now) worked without warning.

    Thoughts/comments?

    Given the semantics of PHP arguments, there is "nothing wrong" with
    defining a required argument after an optional one, and in some cases
    it is required. Consider:

    function foo(Plop $a, $b) {}

    if you want to allow 'null' for $a as well, you have to write:

    function foo(Plop $a = null, $b) {}

    Best,

    -John

    --
    John Bafford
    http://bafford.com/


    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
    --
    Etienne Kneuss
    http://www.colder.ch
  • Stas Malyshev at Nov 24, 2010 at 7:02 pm
    Hi!
    Given the semantics of PHP arguments, there is "nothing wrong" with
    defining a required argument after an optional one, and in some cases
    it is required. Consider:
    I think there's something wrong with it, primarily - the fact that it
    doesn't really make any sense. The object/null thing is a kludge.
    Unfortunately, I don't see a proper way to handle this without named
    arguments.

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • John Bafford at Nov 25, 2010 at 2:06 am

    On Nov 24, 2010, at 14:02, Stas Malyshev wrote:
    Given the semantics of PHP arguments, there is "nothing wrong" with
    defining a required argument after an optional one, and in some cases
    it is required. Consider:
    I think there's something wrong with it, primarily - the fact that it doesn't really make any sense. The object/null thing is a kludge. Unfortunately, I don't see a proper way to handle this without named arguments.

    Ok, I re-wrote the patch and test:
    http://bugs.php.net/patch-display.php?bug_id=53399&patch=strict-required-opt2.patch&revision=latest

    Now, the test happens in zend_do_end_function_declaration(). It loops through each of the function's parameters and emits the E_STRICT if it detects a required parameter after an optional parameter. The tests are loosened so that Class $param = null will not by itself trigger the warning. So it's not a 100% solution since we have to allow for the object/null kludge, but it's still an improvement from not having anything at all.

    To make this work, I added a field to zend_arg_info, 'optional', which is populated in zend_do_receive_arg(). Also, we might want to modify php_reflection.c:_parameter_string() to use zend_arg_info.optional, which would allow indicating whether the parameter was declared optional vs. whether it actually is being treated as optional.

    I think I can move the implementation back into zend_do_receive_arg(), but before I go ahead and do that, I'd like to make sure everyone's happier with this solution than my first. It might also be possible to get rid of zend_arg_info.optional, but I'd need to know how/if it's possible to access the relevant oplines for the rest of the function's parameters.

    -John

    --
    John Bafford
    http://bafford.com/
  • Etienne Kneuss at Nov 25, 2010 at 2:09 am

    On Nov 24 11:02:45, Stas Malyshev wrote:
    Hi!
    Given the semantics of PHP arguments, there is "nothing wrong" with
    defining a required argument after an optional one, and in some cases
    it is required. Consider:
    I think there's something wrong with it, primarily - the fact that
    it doesn't really make any sense. The object/null thing is a kludge.
    Unfortunately, I don't see a proper way to handle this without named
    arguments.
    Sure, it's hackish as hell, but that's PHP's fault, not the user using
    it.

    My point is that we shouldn't add new E_STRICTs for something with no
    possible alternatives.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
    --
    Etienne Kneuss
    http://www.colder.ch

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedNov 24, '10 at 5:28p
activeNov 25, '10 at 2:09a
posts5
users4
websitephp.net

People

Translate

site design / logo © 2022 Grokbase