Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114172
Date: Sun, 25 Apr 2021 23:32:57 +0100
To: "Chase Peeler" <chasepeeler@gmail.com>
Cc: "Mike Schinkel" <mike@newclarity.net>,
	"David Gebler" <davidgebler@gmail.com>,
	"PHP Internals" <internals@lists.php.net>
Message-ID: <1790b2a13b0.1195cec82263227.7824457681413099592@void.tn>
In-Reply-To: <CAPKYkKx6wVccCqWJ-Cpk0MtMPR1KHDGQN8AB7GOCTL3exQ9CdA@mail.gmail.com>
References: <V7C5-r6tzKmxjhPnvy9gtfdMtxkj0z_PJphaehaqcPsOLX0pEKtGU4A8EuSKyS-cbZl5HPVA2ejXCuXXOfOksH9VACGOKT3awRsr9aSNIg8=@protonmail.com>
 <5b9f1500-615a-48f1-815f-1d48b327ef90@processus.org> <CAJpLVh2UybGgAqhfAGQ=kKQ-qvmG68HWKaUJXOwPv7DgEbAdKw@mail.gmail.com>
 <179049b1475.11134368b213512.254739612773841999@void.tn> <CAJpLVh0pz_WopKza=XXqPrvS4bQZhWom1FQ6QLW-fbC-8JHkrA@mail.gmail.com>
 <CADyq6sLtgNkyM-qQcwRY8mMOxnkrz_9qeG6XK2uDBqsh=smMtg@mail.gmail.com>
 <A631B43E-22B4-490A-8C6F-8BFA0FB30B23@cschneid.com> <CA+p1FaY7Je3wfMCwCHCKAZEwPQzcRXwdunC=1df98898xqKV-g@mail.gmail.com>
 <0BF84585-DDC3-4B25-BFD2-D8B916D135EE@newclarity.net> <CAPKYkKx6wVccCqWJ-Cpk0MtMPR1KHDGQN8AB7GOCTL3exQ9CdA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Importance: Medium
User-Agent: Zoho Mail
Subject: Re: [PHP-DEV] [RFC][Draft] Sealed Classes
From: azjezz@void.tn (Saif Eddin Gmati)

---- On Sun, 25 Apr 2021 22:56:26 +0100 Chase Peeler <chasepeeler@gmail.com=
> wrote ----

 > On Sun, Apr 25, 2021 at 11:36 AM Mike Schinkel <mike@newclarity.net> wro=
te:=20
 > =20
 > >=20
 > >=20
 > > > On Apr 24, 2021, at 7:39 PM, David Gebler <davidgebler@gmail.com> wr=
ote:=20
 > > >=20
 > > > I don't love this idea, I'm not very fond of the final keyword, eith=
er;=20
 > >=20
 > > I'll start by saying the final keyword caused me a tremendous amount o=
f=20
 > > heartache because it was used on a class in a framework that I badly, =
badly=20
 > > needed to extend.=20
 > >=20
 > > But even so, I recognize why they used it, and I still don't have a gr=
eat=20
 > > argument for how they could address the reasons they used it some othe=
r way.=20
 > >=20
 > > > I've always believed annotations (or attributes in PHP these days) a=
re a=20
 > > > better of way of indicating you, as an author of a class, did not wr=
ite=20
 > > it=20
 > > > with inheritability in mind or intended than restricting language=20
 > > features=20
 > > > through syntactic constructs.=20
 > > >=20
 > > > The RFC says "when you have a class in your code base that shares so=
me=20
 > > > implementation detail between 2 or more other objects, your only=20
 > > protection=20
 > > > against others making use of this class is to add `@internal` annota=
tion,=20
 > > > which doesn't offer any runtime guarantee that no one is extending t=
his=20
 > > > object", to which I ask - why do you need this guarantee? What does =
it=20
 > > > qualitatively add? If I make a judgement that I want to extend your =
class=20
 > > > or implement your interface, I can just delete the sealed keyword fr=
om=20
 > > your=20
 > > > code and carry on. So it doesn't actually offer any guarantee at all=
 that=20
 > > > I'm not extending the type.=20
 > >=20
 > > Actually, it does offer such a guarantee.  It guarantees if you are us=
ing=20
 > > a non-forked version of the original developer's (OD's) library or=20
 > > framework then that class won't be extended. When someone pulls the=20
 > > original non-forked version from its source repository =E2=80=94 such =
as when using=20
 > > Composer =E2=80=94 then that code will be (effectively) guaranteed not=
 to be=20
 > > extended.=20
 > >=20
 > > OTOH, if you do delete the sealed (or final) keyword you have then for=
ked=20
 > > the code, in a defacto manner if not a literal one. If you use a forke=
d=20
 > > version of the code, you now own the maintenance of that code and any =
bugs=20
 > > that are generated by your forked changes in using code. The original=
=20
 > > developer has no moral, ethical or even contractual obligation to care=
=20
 > > about the breakage you cause.=20
 > >=20
 > =20
 > I'd argue that if the original developer made clear that you shouldn't=
=20
 > extend a class, then they still have no moral, ethical, or even contract=
ual=20
 > obligation to care about the fact you've used the object in a way they w=
ere=20
 > clear was not supported.=20
 > =20
 > I'm with David on this one. I can understand the need to enforce a=20
 > final/sealed concept for core functionality implemented in C which might=
 do=20
 > some funny things under the hood. I don't think that should be extended =
to=20
 > userland. If you want to warn someone, that's fine. But don't totally=20
 > prohibit them.=20
 > =20
 > Given the ability for composer to pull from forked repos and the easy of=
=20
 > keeping a forked repo in sync with it's upstream version, creating a for=
k=20
 > just to remove a sealed/final designation isn't that difficult to do.=20
 > =20
 > =20
 > >=20
 > > Hypothetical example:  You fork the code, remove sealed/final, then=20
 > > subclass the code and add a method, let's call it ToString(). And you =
write=20
 > > your application to use ToString(). Now the OD releases a new minor ve=
rsion=20
 > > and they also add a ToString() method. Applications using your fork=20
 > > probably cannot use the new version of the OD's library because when t=
he=20
 > > library calls ToString() your version is called. So you have to update=
 your=20
 > > application to use the new version of the library and once again remov=
e=20
 > > sealed/final.=20
 > >=20
 > > AND, if your code is instead another add-on library, now users of your=
=20
 > > add-on library will also have to fix their code too.  Which could=20
 > > potentially be a large number of users if your add-on is successful.=
=20
 > >=20
 > > So not using final or sealed can result in some really hairy and possi=
bly=20
 > > impossible to fully resolve backward compatibility concerns for develo=
pers=20
 > > who publish libraries and/or frameworks.=20
 > >=20
 > > > The best it can achieve is to indicate your=20
 > > > intentions, which I believe can be adequately done today through an=
=20
 > > > attribute, no addition to the language needed.=20
 > >=20
 > > Still, I concur with your concerns.  Developers too often implement fi=
nal=20
 > > classes in libraries and frameworks without fully addressing all the=
=20
 > > use-cases and/or adding enough extensibility points because it makes t=
heir=20
 > > lives easier.  Because of that final =E2=80=94 and sealed, if added =
=E2=80=94 can make the=20
 > > life of an application developer a living hell.=20
 > >=20
 > > So what's the answer?  I don't know that I have the ultimate answer, b=
ut I=20
 > > would be a lot more comfortable with adding features to PHP such as on=
es=20
 > > like sealed that restrict the "O" in S.O.L.I.D.[0] if PHP were to offe=
r the=20
 > > following three (3) things, all of which can be found in Go, and I am =
sure=20
 > > other languages:=20
 > >=20
 > > 1. Class embedding[1] =E2=80=94 Allows one class to embed another and =
immediately=20
 > > have access to all its properties and methods, and also to be able to=
=20
 > > extract an instance of that embedded class.  It is called "Type embedd=
ing"=20
 > > in Go.=20
 > >=20
 > > 2.Type definitions[2] =E2=80=94 A typedef would allow developers to de=
fine=20
 > > constrained versions of existing types, such as `FiveStarRating` which=
=20
 > > could only contain 1, 2, 3, 4 or 5, or types that identify a signature=
, for=20
 > > example as `ConvertToString` which could require a closure that implem=
ents=20
 > > `func(mixed):string`. In Go you can compose other types to create new=
=20
 > > types, but I'm not sure if those other type of types could apply to PH=
P, at=20
 > > least as it currently exists, and especially because it is not a compi=
led=20
 > > language.=20
 > >=20
 > > 3. Structural typing[3] =E2=80=94 Basically interfaces that can be imp=
lemented=20
 > > implicitly rather than explicitly.  For example, if I wanted to implem=
ent a=20
 > > Stringable interface that requires a ToString():string method then=20
 > > structural typing would allow me to implement that interface simply by=
=20
 > > adding a ToString() method instead of requiring me to also add "implem=
ents=20
 > > Stringable" to the class definition.=20
 > >=20
 > > Those three features are all killer language features and would make g=
reat=20
 > > additions to PHP.  IMO, of course.=20
 > >=20
 > > #fwiw=20
 > >=20
 > > -Mike=20
 > >=20
 > > [0] https://stackify.com/solid-design-open-closed-principle/=20
 > > [1] https://travix.io/type-embedding-in-go-ba40dd4264df=20
 > > [2] https://go101.org/article/type-system-overview.html=20
 > > [3]=20
 > > https://blog.carbonfive.com/structural-typing-compile-time-duck-typing=
/ <=20
 > > https://blog.carbonfive.com/structural-typing-compile-time-duck-typing=
/>=20
 > =20
 > =20
 > =20
 > --=20
 > Chase Peeler=20
 > chasepeeler@gmail.com=20
 >=20

> I can understand the need to enforce a final/sealed concept for core func=
tionality implemented in C which might do some funny things under the hood.=
 I don't think that should be extended to userland. If you want to warn som=
eone, that's fine. But don't totally prohibit them.

This is not about doing "funny things under the hood", this is about declar=
e a possible sub-types for a specific type.

Taking the `ResultInterface` example from the RFC, there can only be 2 sub =
types for it, `Success` or `Failure`, so when a function declares it's retu=
rn type as `ResultInterface`, you know you are getting either one of these =
two data types, any nothing else.

If the interface would be unsealed, that guarantee doesn't exist anymore, a=
nd the library which might contain functions that operate on the Result typ=
e would become broken as now they have another type that is not being handl=
ed when unwrapping ( hence why Psl doesn't actually contain the `unwrap` fu=
nction mentioned in the RFC ).

> given the ability for composer to pull from forked repos and the easy of =
keeping a forked repo in sync with it's upstream version, creating a fork j=
ust to remove a sealed/final designation isn't that difficult to do.

Yes, you can, but i don't think any maintainer is willing to waste their ti=
me looking into a weird bug resulted from new types that you created by uns=
ealing a sealed class/interface.

If you ever had to do this, then the library you are using is flawed, as it=
 should offer you a way to extend things when inheritance makes sense, but =
sometimes it doesn't ( e.g there's no reason why someone would want a new `=
Result` type that is not `Success` or `Failure`, or would want to implement=
 the `ExceptionInterface` that is used within a library )

for services, I prefer to have implementations declared final, but also off=
er an unsealed interface to make it possible for people to decorate the imp=
lementation or rewrite their own.

If you find yourself in need to extend a class that is declared final, deco=
rate it instead, and if there's no interface for it, maybe send a PR, forki=
ng to remove final is not the solution.