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

go to bug id or search bugs for

Request #69196 PharData should extends Phar
Submitted: 2015-03-06 11:00 UTC Modified: 2021-01-19 15:47 UTC
Votes:2
Avg. Score:3.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: hywan@php.net Assigned:
Status: Open Package: PHAR related
PHP Version: 5.6Git-2015-03-06 (Git) OS:
Private report: No CVE-ID: None
View Developer Edit
 [2015-03-06 11:00 UTC] hywan@php.net 
Description:
------------
According to the documentation, the PharData class (http://php.net/class.phardata) extends the Phar class. But when using the reflection, we see that the PharData class extends the RecursiveDirectoryIterator class.

It must extend the Phar class since it overrides some of their methods.

Test script:
---------------
$r = new ReflectionClass('PharData');
var_dump($r->getParentClass()->getName());

Expected result:
----------------
string(4) "Phar"

Actual result:
--------------
string(26) "RecursiveDirectoryIterator"

Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-03-07 03:06 UTC] reeze@php.net
-Summary: PharData extends Phar or not? +Summary: PharData should extends Phar
 [2015-03-07 03:06 UTC] reeze@php.net 
I believe so.
 [2015-03-08 11:58 UTC] reeze@php.net
-Assigned To: +Assigned To: reeze
 [2015-03-29 07:12 UTC] mike@php.net 
Related To: Bug #52909
 [2015-03-29 07:13 UTC] mike@php.net 
Not sure it should really be this way (Phar extends PharData or PharData extends Phar).

Not directly related: See also bug #52909
 [2015-03-30 02:23 UTC] reeze@php.net 
Related To: Bug #52909
 [2015-03-30 03:26 UTC] reeze@php.net 
-- Repost.

Hi Mike,

The bug #69196 could be fixed by specify different arg info IMO.

class Phar and PharData shared the most of the implementation, we could either fix doc or fix implementation, but as the documentation exist so long, I do prefer fix the implementation.
 [2015-03-30 03:26 UTC] reeze@php.net 
Related To: Bug #69196
 [2015-03-30 09:50 UTC] mike@php.net 
Sure, I'm with you, but I wouldn't fix implementation of "1+2 == 3" if documentation says "1+2 == 4". I just mean we should think about what is th right thing to do.
 [2017-10-24 06:19 UTC] kalle@php.net
-Status: Assigned +Status: Open -Assigned To: reeze +Assigned To:
 [2018-08-05 01:49 UTC] carusogabriel@php.net 
Related To: Bug #52909
 [2019-06-14 15:27 UTC] bishop@php.net
-Type: Bug +Type: Feature/Change Request
 [2019-06-14 15:27 UTC] bishop@php.net 
Arguably, this is a documentation bug, not a Phar bug. However, it's quite surprising and, so, I'm converting it to a feature request. A recent conversation on internals suggested making Phar and PharData implement an interface (rather than inheriting).

See https://marc.info/?l=php-internals&m=156051452912774&w=2:

On Wed, 12 Jun 2019 at 18:16, Bishop Bettini <bishop@***> wrote:

> On Wed, Jun 12, 2019 at 11:35 AM G. P. B. <george.banyard@***>
> wrote:
>
>>    - PharData::setAlias, PharData::setDefaultStub and  PharData::setStub
>>    always throw PharException
>>    <https://www.php.net/manual/en/class.pharexception.php> [11] [12] [13]
>> [11] https://www.php.net/manual/en/phardata.setalias.php
>> [12] https://www.php.net/manual/en/phardata.setdefaultstub.php
>> [13] https://www.php.net/manual/en/phardata.setstub.php
>
>
> I don't know how much this is used in the wild, but these methods exist so
> that a user may treat a Phar and a PharData as interface-equivalent objects
> independent of the phar.readonly INI setting. I lean toward leaving these
> no-op methods as is, but I am happy to further discuss their merit.
>

This does make sense, liek said I was just going thought the doc and didn't
try to see the bigger picture especially as I don't use Phar at all.
Would it make sense to create an interface PharStream (or something else)
on which both these object inherit? If this doesn't make sense please
ignore me.
 [2021-01-19 15:47 UTC] cmb@php.net 
Just for the record, the PharData documentation has been
fixed in the meantime.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Tue Dec 03 17:01:29 2024 UTC