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 Add Comment Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
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

Add a Patch

Pull Requests

Add a Pull Request

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: Fri Apr 19 20:01:29 2024 UTC