# Add E_STRICT when defining a required function parameter after an optional parameter

- Thread: https://externals.io/message/50474
- Original archive: https://news-web.php.net/php.internals/50474
- Started by John Bafford on 2010-11-24T17:28:38+00:00
- Messages: 5
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #50474 — John Bafford — 2010-11-24T17:28:38+00:00

URL: https://externals.io/message/50474#50474

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/

---

## #50475 — Etienne Kneuss — 2010-11-24T17:35:01+00:00

URL: https://externals.io/message/50474#50475 · In reply to: #50474

> 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/
> 
> 
> --
> 
> 
> 

-- 
Etienne Kneuss
http://www.colder.ch

---

## #50478 — Stas Malyshev — 2010-11-24T19:02:45+00:00

URL: https://externals.io/message/50474#50478 · In reply to: #50475

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

---

## #50488 — Etienne Kneuss — 2010-11-25T02:03:33+00:00

URL: https://externals.io/message/50474#50488 · In reply to: #50478

> Hi!
> 
> > […]
> 
> 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

---

## #50487 — John Bafford — 2010-11-25T02:05:57+00:00

URL: https://externals.io/message/50474#50487 · In reply to: #50478

> > […]
> 
> 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/