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/
Hi,
I filed a bug report with an attached patch that adds an
E_STRICTwarning 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/--
--
Etienne Kneuss
http://www.colder.ch
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
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
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/