php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
Have you experienced this issue?
Rate the importance of this bug to you:

 [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: Fri Jun 14 12:01:29 2024 UTC