Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120333
Message-ID: <8c6039e1-8b9d-e86b-a278-90cad5e41a75@bastelstu.be>
Date: Thu, 18 May 2023 00:36:08 +0200
MIME-Version: 1.0
Content-Language: en-US
To: Dan Ackroyd <Danack@basereality.com>
Cc: PHP internals <internals@lists.php.net>
References: <e58dde83-654d-b0ad-de77-d0450a56ef75@bastelstu.be>
 <CA+kxMuT6mL7fhyaA+cMfHjHcS+5uA52ELUKz0c_8dRv-8+ZV1g@mail.gmail.com>
In-Reply-To: <CA+kxMuT6mL7fhyaA+cMfHjHcS+5uA52ELUKz0c_8dRv-8+ZV1g@mail.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/16/23 18:17, Dan Ackroyd wrote:
>> I'm now opening discussion for the RFC "Marking overridden methods (#[\Override])":
> 
> This RFC is probably a good idea, even if the number of people who
> will benefit from the behaviour in the engine might be quite low. My
> guess is the number of people who will bother to write those
> attributes, and who also won't bother to setup CI, is quite small.

I would expect IDEs to provide "Quick Fix" actions to add the attribute 
where possible if the RFC passes. From what I'm seeing as 
not-a-Java-programmer, the @Override annotation in Java appears to be 
very commonly used. I certainly came across it when forced to use Java 
at university.

With regard to usefulness for less-well tested and old codebases, I can 
only talk from my personal experience with such a codebase.

Having the proposed attribute there would certainly have prevented some 
regressions. One specific example that comes to my mind is a class that 
wasn't final but also wasn't meant to be extended. During refactoring I 
inlined one simple method to improve readability. However that broke a 
child class that relied on that method being called to adjust the 
behavior. If the child class would have had the #[\Override] attribute 
on the method in question, the regression would have been immediately 
apparent, even without having automated tests covering the functionality.

> But, this RFC another step forward from where we are, to having 'too
> many' attributes/annotations, so I have a question.
> 
> How do we stop prevent ourselves from copying the problems that Java
> has made for itself?
> 
> Or putting it another way, is it possible for us to set any guidelines
> on what annotations are 'good' or 'bad' ?

I believe I touched on that in the "Why an attribute and not a keyword?" 
section, because I assumed a question like this would come up.

The attribute as-proposed would not have any runtime effect on the code 
and neither would it have an effect on the public API of the code that 
uses the attribute. It is purely an assistance to the reader of the code 
and could be stripped without changing anything. If having the attribute 
on a method would result in an error being emitted after the initial 
code-and-debug cycle then the attribute is merely pointing out a 
breaking change in a *different class*. This breaking change would also 
exist without the attribute. It would just be less obvious.

As such I believe that on a scale from 'good' to 'bad' the attribute is 
very far in the direction of the 'good' end. It is certainly more 'good' 
than both AllowDynamicProperties (affects the public API) and 
SensitiveParameter (affects runtime behavior). The ReturnTypeWillChange 
attribute is just a hack that will go away in the future based on my 
understanding, so I won't put it on the scale.

> I hope I am at least consistent with my concerns about annotations.
> 
> Danack wrote in https://externals.io/message/114116#114196
>>
>> I think I disagree with this very strongly, and plan* to vote against
>> any RFC that embeds another language in annotations.**
> 
>> https://blog.softwaremill.com/the-case-against-annotations-4b2fb170ed67

Thank you for the link to that article, that was an insightful read as 
someone who, as said above, was forced to Java, but didn't deep-dive it 
due to dislike.

I'd like to point out that the article specifically calls out the 
@Override annotation as one of the "good" annotations in Java:

> That’s not to say that annotations are not useful at all. There are some great use-cases. For example, the @Override and @FunctionalInterface annotations in Java or @tailrec in Scala. These are checked at compile-time and provide actionable value to the programmer. They don’t modify how the program behaves at run-time, and they don’t required any kind of container at run-time to fully utilise the annotated class or method.

This assessment by the article's author appears to match my assessment 
from above.

I also believe that #[\Override] falls into a similar category as a 
#[\Memoize] you mentioned in your "one asterisk" footnote of the linked 
email.

Best regards
Tim Düsterhus