On Tue, May 23, 2023, at 10:47 AM, Sara Golemon wrote:
> On Thu, May 11, 2023 at 11:37 AM Tim Düsterhus <t...@bastelstu.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 specious
> point.  The second argument about FC is more defensible, though I
> personally think it's not merited in this particular case.  I'd personally
> 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 code,
> but sometimes they get in the way of mocking or some other non-production
> 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, but I 
can also see the argument for "meh, the SA tools can already do this, and those 
who would bother using it are probably already using SA tools."

Sara's suggestion of using it for other operations, such as "yes, I'm violating 
LSP, trust me, I know what I'm doing" has me very interested.  That 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 going 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 PHPStan happy, but 
having a more directly-supported in the language option would be nice.

Making an attribute that can *control* inheritance rather than just *flag* 
inheritance seems like it would be much more compelling.

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to