Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120524
Message-ID: <91622930-384c-d72d-0e62-eb438266d588@bastelstu.be>
Date: Sun, 4 Jun 2023 16:12:29 +0200
MIME-Version: 1.0
Content-Language: en-US
To: internals@lists.php.net
References: <e58dde83-654d-b0ad-de77-d0450a56ef75@bastelstu.be>
 <CADyq6sJ3aaipfagKtoqksOm2JFp-UN5H0hkrOndgKad2W9x3fg@mail.gmail.com>
 <CA+p1FabKB1K2P9ZidyNgva+mfe1xzTtracyKkQGFKeWYcW1+_Q@mail.gmail.com>
 <CA+kxMuRse5=y_+XKk1LyQ3MpkLan75DzbE=sxtmeiaZf7W0yLQ@mail.gmail.com>
 <CA+p1FaZmagr9Js_a2wp2SOrbhb-uZAhcknXKRLQHonw45gBBOg@mail.gmail.com>
 <c2ea6b97-5ecd-ca12-c748-f0b5e37402c4@bastelstu.be>
 <CA+p1FaYpnRGwG35=a3X8eBGoONvJkyLiqFgcGr+BV0_XMM61pg@mail.gmail.com>
 <c8dcba49-2f76-1812-a9af-7930662e3b5b@bastelstu.be>
 <b2c0a7ab-71bd-a799-495e-9a6e1e84c25f@gmail.com>
In-Reply-To: <b2c0a7ab-71bd-a799-495e-9a6e1e84c25f@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] RFC [Discussion]: Marking overridden methods
 (#[\Override])
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

Hi

On 5/30/23 14:17, Rowan Tommins wrote:
> On 29/05/2023 19:29, Tim Düsterhus wrote:
>> I think this is a flawed premise: Any sort of analysis that PHP itself
>> performs can also be performed in userland.
> 
> 
> This isn't actually true. There is a lot of dynamic functionality in PHP
> where correctness can't be proven ahead of time, and run-time checks are
> the only way. That's why Hack evolved away from being a superset of PHP
> to being a related but incompatible language, and why languages like
> Dart continue to include run-time type checks despite mandatory
> pre-compilation analysis.
> 

Fair enough. Main point still stands, though. LSP checks or other type 
checks wouldn't technically be necessary in PHP itself. As with "duck 
typing" the runtime check at the last possible moment (e.g. the actual 
method call) would catch the mistake, however it would catch it much 
much later, once the broken data made it halfway through the application.

These type checks are effectively (re)implemented in userland and are 
even more powerful, due to the support for generics / typed arrays. 
Nonetheless having them in the language provides a clear benefit, 
because they are always available and thus compatible, no matter which 
choices the other developer of a library made with regard to static 
analysis.

>> It's part of the language and thus will *always* be available. If it's
>> not always available, then folks who switch jobs or contribute to
>> various open source projects that made a different choice with regard
>> to analyzers will possibly:
>>
>> - Be unable to rely on a specific check that they found useful in the
>> past, because the new SA tool doesn't implement it.
>> - Be unable to rely on a specific check working as they learned it,
>> because the new SA tool implements it differently.
> 
> 
> This is a reasonable argument, but it basically comes down to PHP
> Internals becoming the controller of a standardised set of static
> analysis attributes, because a lot of the functionality will still only
> exist in each of those tools.

In this specific case of #[\Override] the main functionality is fully 
available with "just PHP itself".

The reverse check that the attribute is used wherever possible is pushed 
to the userland analyzers / IDEs for now. It would be possible to 
include it in PHP itself, if we can agree on a reasonable solution to 
"configure it" (see my email in response to Claude).

I'm not sure if I would agree on a standardized set of attributes if PHP 
itself would not be able to make use of it, because the value of 
documenting it in the PHP manual would not exist. Developers would need 
to download one of many third party tools to make use of it.

>> As the proposal is a compile time check, the mistake would also be
>> immediately apparent just by loading the class (e.g. as part of *any*
>> kind of test), it is not necessary to actually execute the method in
>> question. So it should be sufficiently visible and usable even in
>> codebases that are in a less-than-stellar state.
> 
> 
> The reason I'm so skeptical of this feature is that the category of
> mistake the engine would be able to catch seems like it would be rather
> rare. And I really do think it makes a difference that the author of the
> *inheriting* class is the only one that can trigger the check.

It's anecdata, but the attribute would have saved me debugging effort 
several times in the past year - that's the whole reason I'm proposing 
it. It also saved me time in other language that have the feature 
already built-in.

> Users of Guzzle won't see any impact from the Guzzle code base adding
> those attributes; they would have to add them to their own code
> separately. If they are not using any kind of static analysis tools,
> they will not be reminded to add them. They will then only see a benefit
> from remembering to add them in the rare case that a parent class
> removes a method they were previously overriding.

As mentioned before with my own Java experience, I believe that having 
this kind of feature in the language itself will naturally help its 
adoption. It will start to appear in codebases, developers will learn 
about its existence by looking at the code. It will likely also be 
included in PHP tutorials targeted at developers that learn PHP.

Speaking from my own experience with languages that have this kind of 
feature, it is not something that I "need to remember to add". It 
becomes a natural part of creating a new method, just like adding 
parameter types and return types became a natural part of it.

> It all seems rather a small niche for the engine to care about, unless
> it's the beginning of a larger project to standardise and encourage such
> attributes.

Best regards
Tim Düsterhus