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

go to bug id or search bugs for

Bug #70606 The documented signature for SplObjectStorage methods is wrong
Submitted: 2015-09-30 10:16 UTC Modified: 2015-09-30 15:57 UTC
From: stof at notk dot org Assigned:
Status: Wont fix Package: Documentation problem
PHP Version: 5.6.13 OS: n/a
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: stof at notk dot org
New email:
PHP Version: OS:

 

 [2015-09-30 10:16 UTC] stof at notk dot org 
Description:
------------
The documentation for the addAll, removeAll and removeAllExcept methods of the SplObjectStorage shows that the argument is typehinted as SplObjectStorage: http://php.net/manual/en/splobjectstorage.addall.php

While this is true that they accept only SplObjectStorage instances, the methods are NOT typehinted, which cause issues when trying to overwrite them. We receive an error saying that the signature does not match, while we follow the documentation.

I had to go read the C source code to understand the actual issue (and I think many people would not do it).
https://github.com/php/php-src/blob/PHP-5.6/ext/spl/spl_observer.c#L972
https://github.com/php/php-src/blob/PHP-5.6/ext/spl/spl_observer.c#L940

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

class MyObjectStorage extends SplObjectStorage
{
    public function addAll(SplObjectStorage $object) {}
}

Actual result:
--------------
T

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-09-30 11:51 UTC] salathe@php.net 
This is more a failing of the source code, which doesn't declare the argument type.  I'd rather see this fixed there, than change the documentation.

Would you be okay with changing this to a bug report for the SPL?
 [2015-09-30 14:21 UTC] stof at notk dot org 
Well, changing the source code would be a BC break, so it cannot be done in 5.x (and it would make it painful to write such code compatible with both 5.x and 7.x if it gets changed in 7.x btw).
 [2015-09-30 15:52 UTC] salathe@php.net
-Status: Open +Status: Wont fix
 [2015-09-30 15:52 UTC] salathe@php.net 
Okay, if you want to keep it as a documentation bug then I'll close as "won't fix".  

We tell white lies throughout the documentation where it makes more sense than the fine details of the source, and in this case I think it adds more value to the documentation to have the type declaration in the prototype than not.
 [2015-09-30 15:57 UTC] stof at notk dot org 
Then why not at least adding a note saying that it is not a true typehint, to avoid WTF for people actually overwriting the method ?

For 5.x at least, it cannot be anything else than a documentation bug (unless you want to drop all PHP policies about backward compatibility, which would be a very bad news for the community).
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Apr 25 23:01:29 2024 UTC