[RFC] Change Directory class to behave like an opaque object

Christoph Becker

1 year ago

On 14.09.2024 at 17:33, Gina P. Banyard wrote:

> I came across the Directory class while doing some code exploration of ext/standard.
> This class is effectively an opaque object for Directory resources, however it doesn't behave like one, as it has existed since PHP 4.
>
> As such, I am proposing an RFC to turn this class into an opaque object.
>
> RFC: https://wiki.php.net/rfc/directory-opaque-object

As I see it, an opaque object is an object which doesn't have any public
members.  Since Directory has five public members, I wouldn't call it
opaque.

Other than that wording issue, I welcome this RFC.  Thank you!

Christoph

Gina P. Banyard

1 year ago

On Saturday, 14 September 2024 at 18:48, Christoph M. Becker <cmbecker69@gmx.de> wrote:

> On 14.09.2024 at 17:33, Gina P. Banyard wrote:
>
> > I came across the Directory class while doing some code exploration of ext/standard.
> > This class is effectively an opaque object for Directory resources, however it doesn't behave like one, as it has existed since PHP 4.
> >
> > As such, I am proposing an RFC to turn this class into an opaque object.
> >
> > RFC: https://wiki.php.net/rfc/directory-opaque-object
>
>
> As I see it, an opaque object is an object which doesn't have any public
> members. Since Directory has five public members, I wouldn't call it
> opaque.

Eh, GMP is also "kinda" an opaque object, and I don't really have any better naming idea, so I went with what is the closest common concept we have.
But if you have any idea for better terminology, I would gladly use it.

Best regards,

Gina P. Banyard

Christoph Becker

1 year ago

On 17.09.2024 at 18:03, Gina P. Banyard wrote:

> On Saturday, 14 September 2024 at 18:48, Christoph M. Becker <cmbecker69@gmx.de> wrote:
>
>> On 14.09.2024 at 17:33, Gina P. Banyard wrote:
>>
>>> RFC: https://wiki.php.net/rfc/directory-opaque-object
>>
>> As I see it, an opaque object is an object which doesn't have any public
>> members. Since Directory has five public members, I wouldn't call it
>> opaque.
>
> Eh, GMP is also "kinda" an opaque object,

Yeah, but this only has magic methods, while Directory already has
"proper" methods.

> and I don't really have any better naming idea, so I went with what is the closest common concept we have.
> But if you have any idea for better terminology, I would gladly use it.

No, I don't have a better idea, and I'm not too much concerned about
that now.  Only my first reaction was: "What?  Why remove the existing
methods?"

Christoph

David CARLIER

1 year ago

Seems right thing to do.

I saw some usage like

final class Dir extends Directory
{
}

also did not see much in *new* codes Directory being extended so 8.5 sure.

Cheers.

On Sat, 14 Sept 2024 at 16:36, Gina P. Banyard <internals@gpb.moe> wrote:

Derick Rethans

1 year ago

On Sat, 14 Sep 2024, Gina P. Banyard wrote:

> I came across the Directory class while doing some code exploration of
> ext/standard. This class is effectively an opaque object for Directory
> resources, however it doesn't behave like one, as it has existed since
> PHP 4.
>
> As such, I am proposing an RFC to turn this class into an opaque
> object.
>
> RFC: https://wiki.php.net/rfc/directory-opaque-object

I see you want to make "new Directory" not work - but wouldn't it make
more sense if that was the *prefered* way instead of using dir() ?

cheers,
Derick

Christoph Becker

1 year ago

On 16.09.2024 at 18:31, Derick Rethans wrote:

> On Sat, 14 Sep 2024, Gina P. Banyard wrote:
>
>> I came across the Directory class while doing some code exploration of
>> ext/standard. This class is effectively an opaque object for Directory
>> resources, however it doesn't behave like one, as it has existed since
>> PHP 4.
>>
>> As such, I am proposing an RFC to turn this class into an opaque
>> object.
>>
>> RFC: https://wiki.php.net/rfc/directory-opaque-object
>
> I see you want to make "new Directory" not work - but wouldn't it make
> more sense if that was the *prefered* way instead of using dir() ?

I also thought this at first, but if Directory will replace Dir handle
resources in PHP next, it might not make too much sense, since I presume
that dir() is rarely used compared to opendir(), and I  think we need to
keep opendir() and friends for a long time, anyway.

Christoph

Gina P. Banyard

1 year ago

On Monday, 16 September 2024 at 18:31, Derick Rethans <derick@php.net> wrote:

> On Sat, 14 Sep 2024, Gina P. Banyard wrote:
>
> > I came across the Directory class while doing some code exploration of
> > ext/standard. This class is effectively an opaque object for Directory
> > resources, however it doesn't behave like one, as it has existed since
> > PHP 4.
> >
> > As such, I am proposing an RFC to turn this class into an opaque
> > object.
> >
> > RFC: https://wiki.php.net/rfc/directory-opaque-object
>
>
> I see you want to make "new Directory" not work - but wouldn't it make
> more sense if that was the prefered way instead of using dir() ?

I don't have any strong preferences, but this could always be amended as a follow-up.
I don't expect anyone to instantiate Directory with new anyway as it results in a broken object.

Best regards,

Gina P. Banyard

Gina P. Banyard

1 year ago

On Saturday, 14 September 2024 at 16:33, Gina P. Banyard <internals@gpb.moe> wrote:

> Hello internals,
>
> I came across the Directory class while doing some code exploration of ext/standard.
> This class is effectively an opaque object for Directory resources, however it doesn't behave like one, as it has existed since PHP 4.
>
> As such, I am proposing an RFC to turn this class into an opaque object.
>
> RFC: https://wiki.php.net/rfc/directory-opaque-object

Unless there are further comments I will be opening the vote for the RFC on Saturday.

Best regards,

Gina P. Banyard

Gina P. Banyard

1 year ago

On Tuesday, 24 September 2024 at 13:25, Gina P. Banyard <internals@gpb.moe> wrote:

> On Saturday, 14 September 2024 at 16:33, Gina P. Banyard internals@gpb.moe wrote:
>
> > Hello internals,
> >
> > I came across the Directory class while doing some code exploration of ext/standard.
> > This class is effectively an opaque object for Directory resources, however it doesn't behave like one, as it has existed since PHP 4.
> >
> > As such, I am proposing an RFC to turn this class into an opaque object.
> >
> > RFC: https://wiki.php.net/rfc/directory-opaque-object
>
>
> Unless there are further comments I will be opening the vote for the RFC on Saturday.

I have changed some of the terminology and added a rationale section to the RFC:
https://wiki.php.net/rfc/directory-opaque-object

Unless there are again some comments about it, I will open the RFC on Thursday the 3rd of October.

Best regards,

Gina P. Banyard

Christian Schneider

1 year ago

Am 14.09.2024 um 17:33 schrieb Gina P. Banyard <internals@gpb.moe>:

> I came across the Directory class while doing some code exploration of ext/standard.
> This class is effectively an opaque object for Directory resources, however it doesn't behave like one, as it has existed since PHP 4.
>
> As such, I am proposing an RFC to turn this class into an opaque object.
>
> RFC: https://wiki.php.net/rfc/directory-opaque-object

While I understand the other proposed changes (although I'm always a bit wary about the wish to make more and more things final) I was wondering why you would prevent cloning?
What are the possible downsides in allowing people to get a clone of a Directory object?

Regards,
- Chris

Gina P. Banyard

1 year ago

On Tuesday, 24 September 2024 at 14:34, Christian Schneider <cschneid@cschneid.com> wrote:

> Am 14.09.2024 um 17:33 schrieb Gina P. Banyard internals@gpb.moe:
>
> > I came across the Directory class while doing some code exploration of ext/standard.
> > This class is effectively an opaque object for Directory resources, however it doesn't behave like one, as it has existed since PHP 4.
> >
> > As such, I am proposing an RFC to turn this class into an opaque object.
> >
> > RFC: https://wiki.php.net/rfc/directory-opaque-object
>
>
> While I understand the other proposed changes (although I'm always a bit wary about the wish to make more and more things final) I was wondering why you would prevent cloning?
> What are the possible downsides in allowing people to get a clone of a Directory object?

Mainly because the current behaviour doesn't do a proper clone.
It still shares a common directory resource with the original object meaning that there is spooky action at a distance even thought cloning is meant to prevent this.

Best regards,

Gina P. Banyard

Nick Lockheart

1 year ago

On Sat, 2024-09-14 at 15:33 +0000, Gina P. Banyard wrote:

> Hello internals,
>
> I came across the Directory class while doing some code exploration
> of ext/standard.
> This class is effectively an opaque object for Directory resources,
> however it doesn't behave like one, as it has existed since PHP 4.
>
> As such, I am proposing an RFC to turn this class into an opaque
> object.
>
> RFC: https://wiki.php.net/rfc/directory-opaque-object
>
>
> Best regards,
>
> Gina P. Banyard

I don't suppose we could call the Directory class something else, like
"SPLDirectory"?

You can't make a Directory with `new` (or at least you aren't supposed
to be able to), so any existing code would already get a Directory
object with `dir()` not `new Directory()`. There would be no BC break
with a rename.

Why?

It's very easy for someone that doesn't know about the Directory class
to create their own class called `Directory`, instantiate it with `new`
expecting their autoloader to pick it up, and then getting very strange
results.

The error will usually be something like, "call to undefined method
Directory::MyMethod", which makes developers go nuts, looking at their
Directory class, seeing "MyMethod" clearly defined right there, and yet
PHP claims it can't find the method. But PHP is finding the **class**
because surely, the autoloader loaded it!

What actually happened is the autoloader didn't run because the name
"Directory" is already registered. But there is no warning or error
when someone writes `new Directory();`.

Since the built-in Directory class is never instantiated by name,
nothing would break by renaming it, since it only gets created by
`dir()`.

A rename would free up the name 'Directory' so people wouldn't get
frustrated with bizarre behavior when they make their own Directory
class (which I imagine is very common).

Gina P. Banyard

1 year ago

On Wednesday, 25 September 2024 at 10:12, Nick Lockheart <lists@ageofdream.com> wrote:

> On Sat, 2024-09-14 at 15:33 +0000, Gina P. Banyard wrote:
> I don't suppose we could call the Directory class something else, like
> "SPLDirectory"?
>
> You can't make a Directory with `new` (or at least you aren't supposed
> to be able to), so any existing code would already get a Directory
> object with `dir()` not `new Directory()`. There would be no BC break
> with a rename.

This is factually incorrect.
This would be a large BC break, moreover PHP "owns" the global namespace.

If you are in a namespace and attempting to instantiate a class
named Directory you will either get the namespace version of the class,
or an Error as there is no fallback to the global namespace for classes.

Otherwise, if you are in the global namespace and try to create a class
which is called Directory this would result in an Error being thrown.

Finally, people may have typed properties, or parameters with the type
Directory, renaming this would break all that code.

Best regards,

Gina P. Banyard

Nick Lockheart

1 year ago

On Tue, 2024-10-01 at 16:36 +0000, Gina P. Banyard wrote:

> On Wednesday, 25 September 2024 at 10:12, Nick Lockheart
> <lists@ageofdream.com> wrote:
>
> > On Sat, 2024-09-14 at 15:33 +0000, Gina P. Banyard wrote:
> > I don't suppose we could call the Directory class something else,
> > like
> > "SPLDirectory"?
> >
> > You can't make a Directory with `new` (or at least you aren't
> > supposed
> > to be able to), so any existing code would already get a Directory
> > object with `dir()` not `new Directory()`. There would be no BC
> > break
> > with a rename.
>
> This is factually incorrect.
> This would be a large BC break, moreover PHP "owns" the global
> namespace.
>

Is every single word really a reserved word in the global namespace?

> Otherwise, if you are in the global namespace and try to create a
> class which is called Directory this would result in an Error being
> thrown.
>

Actually, it will not throw an error because classes are auto loaded.

The auto loader will not run because Directory is already registered.
It will instantiate a built-in Directory object.

An error would only be thrown if the file with the other Directory
class is manually included, which most people do not do anymore for
class definitions.

Rob Landers

1 year ago

On Tue, Oct 1, 2024, at 22:21, Nick Lockheart wrote:

> On Tue, 2024-10-01 at 16:36 +0000, Gina P. Banyard wrote:
> > On Wednesday, 25 September 2024 at 10:12, Nick Lockheart
> > <lists@ageofdream.com> wrote:
> >
> > > On Sat, 2024-09-14 at 15:33 +0000, Gina P. Banyard wrote:
> > > I don't suppose we could call the Directory class something else,
> > > like
> > > "SPLDirectory"?
> > >
> > > You can't make a Directory with `new` (or at least you aren't
> > > supposed
> > > to be able to), so any existing code would already get a Directory
> > > object with `dir()` not `new Directory()`. There would be no BC
> > > break
> > > with a rename.
> >
> > This is factually incorrect.
> > This would be a large BC break, moreover PHP "owns" the global
> > namespace.
> >
>
> Is every single word really a reserved word in the global namespace?
>
>
> > Otherwise, if you are in the global namespace and try to create a
> > class which is called Directory this would result in an Error being
> > thrown.
> >
>
> Actually, it will not throw an error because classes are auto loaded.
>
> The auto loader will not run because Directory is already registered.
> It will instantiate a built-in Directory object.
>
> An error would only be thrown if the file with the other Directory
> class is manually included, which most people do not do anymore for
> class definitions.
>

Hey Nick,

Is this actually an issue though? \Directory would be a weird thing to autoload. Most people tend to autoload specific namespaces. I think it would be weird to autoload into the global namespace.

Maybe I am wrong, and that is why I ask.

— Rob

Nick Lockheart

1 year ago

>
> Hey Nick,
>
> Is this actually an issue though? \Directory would be a weird thing
> to autoload. Most people tend to autoload specific namespaces. I
> think it would be weird to autoload into the global namespace.
>
> Maybe I am wrong, and that is why I ask.
>
> — Rob

In a situation where people aren't using namespaces at all, it would be
normal to create your own class with a conflicting name, not knowing
there was a built-in.

Larry Garfield

1 year ago

On Tue, Oct 1, 2024, at 4:02 PM, Nick Lockheart wrote:

>>
>> Hey Nick,
>>
>> Is this actually an issue though? \Directory would be a weird thing
>> to autoload. Most people tend to autoload specific namespaces. I
>> think it would be weird to autoload into the global namespace.
>>
>> Maybe I am wrong, and that is why I ask.
>>
>> — Rob
>
>
> In a situation where people aren't using namespaces at all, it would be
> normal to create your own class with a conflicting name, not knowing
> there was a built-in.

Which is why anyone creating a global namespace class, especially with a generic name like that, in 2024 is Doing PHP Wrong(tm).  The top-level namespace has been considered "reserved" (at least de facto, if perhaps not sung from the rooftops) for... 15 years?  Maybe not as long as there have been namespaces, but close to.

I have no strong opinions on this RFC as I've not paid close attention to it, but "we can't introduce an unnamespaced class, that's a BC break" is simply not true, and not consistent with how Internals and the stdlib are and have been developed.

--Larry Garfield

Nick Lockheart

1 year ago

On Tue, 2024-10-01 at 18:32 -0500, Larry Garfield wrote:

> On Tue, Oct 1, 2024, at 4:02 PM, Nick Lockheart wrote:
> > >
>
> Which is why anyone creating a global namespace class, especially
> with a generic name like that, in 2024 is Doing PHP Wrong(tm).

OK. I'm going to go back to writing fast, efficient, and Wrong(tm) PHP,
like I have been doing for the past twenty years.

Thanks,
Nick

Gina P. Banyard

1 year ago

On Wednesday, 2 October 2024 at 00:32, Larry Garfield <larry@garfieldtech.com> wrote:

>
> I have no strong opinions on this RFC as I've not paid close attention to it, but "we can't introduce an unnamespaced class, that's a BC break" is simply not true, and not consistent with how Internals and the stdlib are and have been developed.
>

This is not a new class, it is a class that has existed since the PHP 4 days.
Arguably, if this would be a new class, that argument would have more merit.
But changing the name of class that is 20+ years old and could have been used in type declarations since possibly PHP 5.0 is a non-starter IMHO.


Best regards,

Gina P. Banyard

Christoph Becker

1 year ago

On 02.10.2024 at 12:55, Gina P. Banyard wrote:

> On Wednesday, 2 October 2024 at 00:32, Larry Garfield <larry@garfieldtech.com> wrote:
>
>> I have no strong opinions on this RFC as I've not paid close attention to it, but "we can't introduce an unnamespaced class, that's a BC break" is simply not true, and not consistent with how Internals and the stdlib are and have been developed.
>
> This is not a new class, it is a class that has existed since the PHP 4 days.

Plus it is not part of some rarely used extension, but rather of
ext/standard, so it is always available, and nobody had been able to
declare a userland class Directory (https://3v4l.org/dUOq0).

> Arguably, if this would be a new class, that argument would have more merit.

Right.

> But changing the name of class that is 20+ years old and could have been used in type declarations since possibly PHP 5.0 is a non-starter IMHO.

I agree, although one could pursue the RFC process.  This would be a
completely different RFC, though.

Christoph

Rob Landers

1 year ago

On Tue, Oct 1, 2024, at 23:02, Nick Lockheart wrote:

> >
> > Hey Nick,
> >
> > Is this actually an issue though? \Directory would be a weird thing
> > to autoload. Most people tend to autoload specific namespaces. I
> > think it would be weird to autoload into the global namespace.
> >
> > Maybe I am wrong, and that is why I ask.
> >
> > — Rob
>
>
> In a situation where people aren't using namespaces at all, it would be
> normal to create your own class with a conflicting name, not knowing
> there was a built-in.
>

Hey Nick,

Your reply just gave me flashbacks of the days before ctrl-click and games of “Find the Buried Implementation.” Heh.

I’m sure code like you describe exists, but I haven’t personally run into it (outside of WordPress) since 2011? In either case, auto loading was not in use. I’m not even sure how it would work (maybe a custom/manual class-map type thing)?

Sleeping on it, I do agree that Directory is a poor name for a final class. You would expect something called Directory to be abstract and be implemented in concrete objects like FileDirectory, FTPDirectory, WebDavDirectory, S3Directory, etc. For the base abstraction of a directory to be final is a bit weird, IMHO.

— Rob