Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120394
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-441-ga3ab13cd6d-fm-20230517.001-ga3ab13cd
Mime-Version: 1.0
Message-ID: <1fd2566e-ce23-4ab5-af5e-d3e57389c327@app.fastmail.com>
In-Reply-To: 
 <CAESVnVrs0hV4C_NqZvkJ+V_M7UOQDPSpqkaBvuxhx6=7L0ViVg@mail.gmail.com>
References: <e58dde83-654d-b0ad-de77-d0450a56ef75@bastelstu.be>
 <CAESVnVrs0hV4C_NqZvkJ+V_M7UOQDPSpqkaBvuxhx6=7L0ViVg@mail.gmail.com>
Date: Tue, 23 May 2023 17:44:23 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] RFC [Discussion]: Marking overridden methods (#[\Override])
From: larry@garfieldtech.com ("Larry Garfield")

On Tue, May 23, 2023, at 10:47 AM, Sara Golemon wrote:
> On Thu, May 11, 2023 at 11:37=E2=80=AFAM Tim D=C3=BCsterhus <tim@baste=
lstu.be> wrote:
>>
>> I'm now opening discussion for the RFC "Marking overridden methods
>> (#[\Override])":
>>
>
> I 100% get the intent behind this RFC, and as someone who's used this =
in
> multiple other languages the benefit to defensive coding is obvious.
>
> Thoughts:
>
> I think targeting 8.3 is aggressive as we're less than a month from FF
> (accounting for discussion and voting period).
>
>
> The first argument (about not impacting callers) for "why an attribute=
 and
> not a keyword" feels like it's tying itself into a knot to make a spec=
ious
> point.  The second argument about FC is more defensible, though I
> personally think it's not merited in this particular case.  I'd person=
ally
> rather have a keyword here.
>
>
> If you do go with an Attribute, then I'd go ahead and go further by
> allowing to specify the name of the class being overridden and possibly
> flags about intentional LSP violations.  For examples:
>
>
> // Intentional LSP violations
> class A {
>   public function foo(): \SimpleXMLElement { return
> simplexml_load_file("/tmp/file.xml"); }
> }
> class TestA extends A {
>   #[\Override(Override::IGNORE_RETURN_TYPE_VIOLATION)]
>   public function foo(): TestProxyClass { return TestProxy(parent::foo=
()); }
> }
>
> LSP checks are super valuable for writing clean and well debuggable co=
de,
> but sometimes they get in the way of mocking or some other non-product=
ion
> activity.  This could provide a "get out of jail free card" for those
> circumstances where you just want to tell the engine, "Shut up, I know=
 what
> I'm doing".

I've been luke-warm on this RFC so far.  I can sorta see the use case, b=
ut I can also see the argument for "meh, the SA tools can already do thi=
s, and those who would bother using it are probably already using SA too=
ls."

Sara's suggestion of using it for other operations, such as "yes, I'm vi=
olating LSP, trust me, I know what I'm doing" has me very interested.  T=
hat has a number of potential uses that SA tools would not be sufficient=
 for.

The first is testing, which is the example Sara gave.

The second is when you have a wide type in an interface method parameter=
, but in a given child class you know with certainty that the type is go=
ing to be some subset of that.  It's not typical, but I have a few cases=
 where I've needed that.  Narrower docblock types seem to work to keep P=
HPStan happy, but having a more directly-supported in the language optio=
n would be nice.

Making an attribute that can *control* inheritance rather than just *fla=
g* inheritance seems like it would be much more compelling.

--Larry Garfield