php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71428 Validation type inheritance with = NULL
Submitted: 2016-01-21 18:52 UTC Modified: 2016-04-28 17:16 UTC
From: dpa-bugs at aegee dot org Assigned:
Status: Closed Package: Class/Object related
PHP Version: 7.0.2 OS:
Private report: No CVE-ID: None
 [2016-01-21 18:52 UTC] dpa-bugs at aegee dot org
Description:
------------
<?php
class A { }
class B           {  public function m(A $a = NULL, $n) { echo "B.m";} };
class C extends B {  public function m(A $a       , $n) { echo "C.m";} };
$b = new B();
$b->m(new A(), $b);
$c = new C();
$c->m(new A(), $b);
?>

reports nothing, but shall report:

PHP Warning:  Declaration of C::m(A $a, $n) should be compatible with B::m(A $a = NULL, $n) on line 4


Removing on both places $n reports correctly:

PHP Warning:  Declaration of C::m(A $a) should be compatible with B::m(A $a = NULL) on line 4

Same with php 5.6.17.


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-01-21 21:23 UTC] nikic@php.net
Another example that does not rely on optional-before-required arguments:

class A {
    public function m(array $a = null) {}
}
class B extends A {
    public function m(array $a = []) {}
}
 [2016-01-21 22:10 UTC] dpa-bugs at aegee dot org
By the way, I do not write = NULL, because I want the argument to be optional.  I write it, in order to be able to pass NULL as parameter.
 [2016-03-23 17:05 UTC] danielgarthsims at gmail dot com
Is this a bug? I don't see why it's a problem. Looking at the nikic's example, the method signature doesn't change, just the default value for the method.

In the original example, you're still requiring two variables, but the second one can no longer be null. I'm not sure what the expected behavior should be.
 [2016-03-23 21:20 UTC] dpa-bugs at aegee dot org
In all examples, you cannot pass NULL as first parameter to the derived m(), but you can bass NULL as first parameter to parent::m().  So the method signature changes, in terms of possible values that can be supplied.
 [2016-03-28 07:14 UTC] krakjoe@php.net
-Status: Open +Status: Verified
 [2016-03-28 07:14 UTC] krakjoe@php.net
There are bugs in 

 * zend_do_perform_type_hint_check (never verifies compatibility of zend_arg_info.allow_null) 
 * zend_get_function_declaration

The bugs in zend_get_function_declaration are because we make this assumption: 

    if (i >= required && !arg_info->is_variadic)

So do not generate the correct error message when you fix the type hint check.

It's 8am on Monday morning ... someone else should have a go ...
 [2016-03-28 10:14 UTC] inefedor at gmail dot com
I wonder if we should go through RFC stage for that supposedly BC-breaking fix?
 [2016-03-28 10:53 UTC] bwoebi@php.net
It's technically a BC break, but at the same time a big LSP violation bug.

If you relied on that, your code is technically anyway broken. (as passing null to a typehint of superclass should break,)
 [2016-03-29 08:58 UTC] krakjoe@php.net
Automatic comment on behalf of krakjoe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dd70c39556fe6efe2e1f28cecf8fc73d9588d04a
Log: fix bug #71428: Validation type inheritance with = NULL
 [2016-03-29 08:58 UTC] krakjoe@php.net
-Status: Verified +Status: Closed
 [2016-03-29 08:58 UTC] krakjoe@php.net
Automatic comment on behalf of krakjoe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dd70c39556fe6efe2e1f28cecf8fc73d9588d04a
Log: fix bug #71428: Validation type inheritance with = NULL
 [2016-04-28 16:58 UTC] dmitry@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9e45ac53ce64461e56f82f5847526a675c769f88
Log: Fixed BC break described by bug #72119. It was introduced after 7.0.5 release by attempt to fix bug #71428.
 [2016-04-28 17:16 UTC] dmitry@php.net
-Status: Closed +Status: Re-Opened
 [2016-04-28 17:16 UTC] dmitry@php.net
The fix was reverted
 [2016-04-28 17:43 UTC] ab@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=75fa8842f4b64f60db65474cb5d3ca9f40875725
Log: Fixed BC break described by bug #72119. It was introduced after 7.0.5 release by attempt to fix bug #71428.
 [2016-04-28 17:43 UTC] ab@php.net
-Status: Re-Opened +Status: Closed
 [2016-05-23 07:14 UTC] dmitry@php.net
Automatic comment on behalf of levim
Revision: http://git.php.net/?p=php-src.git;a=commit;h=56c3d75780b7c2faf290722a615fd2d797d2f041
Log: Fix bug #71428
 [2016-07-20 11:31 UTC] davey@php.net
Automatic comment on behalf of levim
Revision: http://git.php.net/?p=php-src.git;a=commit;h=56c3d75780b7c2faf290722a615fd2d797d2f041
Log: Fix bug #71428
 [2016-07-20 11:31 UTC] davey@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9e45ac53ce64461e56f82f5847526a675c769f88
Log: Fixed BC break described by bug #72119. It was introduced after 7.0.5 release by attempt to fix bug #71428.
 [2016-07-20 11:32 UTC] davey@php.net
Automatic comment on behalf of krakjoe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dd70c39556fe6efe2e1f28cecf8fc73d9588d04a
Log: fix bug #71428: Validation type inheritance with = NULL
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Mon Oct 07 12:01:27 2024 UTC