php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #72119 Interface declaration compatibility regression with default values
Submitted: 2016-04-28 11:33 UTC Modified: 2016-04-29 09:28 UTC
From: ben dot davies at gmail dot com Assigned: dmitry (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: 7.0.6 OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: ben dot davies at gmail dot com
New email:
PHP Version: OS:

 

 [2016-04-28 11:33 UTC] ben dot davies at gmail dot com 
Description:
------------
I wanted to raise this against 7.0.6, but it was not available in the list above, despite being released?

There seems to be a regression in interface method declaration compatibility checking.
The below test script only fails on 7.0.6.

It also only fails if the $baz is type hinted as array.

Test script:
---------------
<?php

interface Foo {
    public function bar(array $baz = null);
}

class Hello implements Foo {
    public function bar(array $baz = [])
    {

    }
}

Expected result:
----------------
N/A

Actual result:
--------------
Fatal error: Declaration of Hello::bar(array $baz = Array) must be compatible with Foo::bar(array $baz = NULL) in /in/VhEtd on line 7

Process exited with code 255.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-04-28 15:18 UTC] levim@php.net
-PHP Version: Next Minor Version +PHP Version: 7.0.6 -Assigned To: +Assigned To: levim
 [2016-04-28 15:18 UTC] levim@php.net 
This was changed in commit e9d65160 by Dmitry. It appears to be a fix for a different bug (71978).
 [2016-04-28 15:22 UTC] bwoebi@php.net
-Status: Assigned +Status: Not a bug -Assigned To: levim +Assigned To:
 [2016-04-28 15:22 UTC] bwoebi@php.net 
This is not a bug; your code is a LSP violation which had been fixed in bug #71428.

In the concrete example, the Foo::bar() function can be passed null, but Hello::bar() cannot.

[This is something we try to fix with nullable parameters/null unions in 7.1.]
 [2016-04-28 15:23 UTC] levim@php.net
-Summary: Interface declaration compatibility regression +Summary: Interface declaration compatibility regression with default values -Assigned To: +Assigned To: levim
 [2016-04-28 15:23 UTC] levim@php.net 
I have confirmed that this is not a bug. The reason is that in the interface you are permitted to pass null:

<?php
$Foo->bar(null);
?>

However, in the implementing class Hello it would not be permitted. Everything permitted in the parent must also be permitted in the child, thus it is not a bug.
 [2016-04-28 15:26 UTC] ben dot davies at gmail dot com 
Hi both,

No doubt you are both correct, but this appears to be a BC break.
This works in every version of php up to 7.0.6.

https://3v4l.org/VhEtd

Thanks
 [2016-04-28 16:34 UTC] dmitry@php.net 
The original problem was introduced by attempt to fix #71428 (that expects exactly the opposite behavior) in commit ee9a78a033696ff9546fb1dbfecd28f20477b511
 [2016-04-28 16:42 UTC] bwoebi@php.net 
Correct; it should never have worked in the first place though;
While it technically is a BC break, I personally think this was in the first place an important (long-term) bugfix. [which thus shouldn't be reverted in 7.0.7]
 [2016-04-28 16:50 UTC] rowan dot collins at gmail dot com 
Given that it has the potential to break existing code, I think it would make more sense to fix it in 7.1, when people will be expecting to check changelogs and update code. Breaking compatibility in a patch release will just lead to people not trusting the official releases, and thus not getting important security fixes, which is in nobody's interest.
 [2016-04-28 16:52 UTC] nikic@php.net
-Package: PHP Language Specification +Package: Scripting Engine problem
 [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:07 UTC] ab@php.net 
We should not introduce such BC breaches in a minor release. This is really one of the basic OOP usage cases, the impact is unknown. I'd say keeping BC in this case is more important.

Thanks.
 [2016-04-28 17:14 UTC] dmitry@php.net
-Status: Not a bug +Status: Closed -Assigned To: levim +Assigned To: dmitry
 [2016-04-28 17:14 UTC] dmitry@php.net 
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.
 [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-29 09:28 UTC] ben dot davies at gmail dot com 
Hey Anatol, Dmitry,

Looks like the release notes for 7.0.6 are wrong, because they include:
"Fixed bug #71428 (inheritance and allow_null)."

Which this issue reverted, so shouldn't be in the release notes?

Thanks.
 [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.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Wed Nov 27 14:01:29 2024 UTC