Introduce #[Override] attribute to mark overloaded methods

[jrf]

From https://core.trac.wordpress.org/ticket/59231:

​Marking overridden methods

This is a new feature which will be introduced in PHP 8.3, with limited validation functionality attached. The attribute basically allows to mark methods in a (child) class/interface which overload a method in a parent class or from an interface, as doing so intentionally.

Per the RFC:

... being able to express if a method is intended to override another method or implement an interface would make it easier to debug a mistake, to refactor and to clean up existing code. Another possible use case is to easily detect a possibly breaking change in a parent class that was provided by a library without needing to read the changelog in detail or missing some item in the list of changes

I'd like to advocate for adding these attributes to WP Core in all the relevant places as it:

• Increases awareness of the method overload for contributors.
• Can serve as a warning that the method signature should not be touched (unless the parent method signature changes).
• Has no downside as attributes are ignored in older PHP versions and in PHP versions where the attribute referenced does not exist.

In the rare case that the attribute, once added, would result in a fatal error, that would be fantastic, as that means we have actually found a bug in WP before it got into a stable release.

Trac ticket: https://core.trac.wordpress.org/ticket/59232

Awesome work team!

I haven't reviewed/verified the patch in detail, but just thought I'd leave a comment about something to think about.

The attribute as used in this patch - #[\Override] - is using a fully qualified class name.

In non-namespaced files (as all WP Core files are), it could be elected to not use the leading backslash.
Alternatively, a use Override import statement could be used.

I am in favour of using the fully qualified names, as is done in this patch, as it ensures that if, at some hypothetical time in the future, namespaces would be introduced, this code will continue to work without the need for further changes.

However, the other attributes currently in use in WP Core - #[AllowDynamicProperties] and #[ReturnTypeWillChange] - are used in an unqualified form.

I wonder if we should make all attributes consistently use fully qualified names ?

Note: this doesn't need to be addressed in this PR, but is a question which the changes in this PR triggered for me and something I believe we should have a think about.

[oglekler]

This ticket was discussed during bug scrub.

Hi @SergeyBiryukov this looks like a straight forward task, but we need another ticket to make things consistent :) Can you make this? Thank you 🙏

Add props to @mukesh27

[hellofromTonya]

The changes with this ticket will not impact production code. Rather, they are adding an annotation necessary for PHP 8.3 compatibility. Thus, I've changed the ticket to a task and it can go into the 6.4 beta cycle.

[jrf]

Replying to hellofromTonya:

The changes with this ticket will not impact production code. Rather, they are adding an annotation necessary for PHP 8.3 compatibility. Thus, I've changed the ticket to a task and it can go into the 6.4 beta cycle.

I'm happy for this to stay in the 6.4 beta cycle, but it is not a necessity. This is not needed for PHP 8.3 compatibility. It is a ticket to take advantage of a new PHP 8.3 feature which can help prevent future coding mistakes.

[hellofromTonya]

Thanks for clarifying @jrf :)

The PR is still in draft mode. With 6.4 RC1 in less than 15 hours, moving this to 6.5 and changing it back to an enhancement.

[oglekler]

@SergeyBiryukov what is the decision about consistency of attributes? It looks simple enough to move this forward without many complications.

[oglekler]

It looks like no one has time to tackle this. We have 2 days before Beta 1, so, I am moving this ticket to the next milestone.

[stoyangeorgiev]

Moving this one to a Future Release, since this one would need more time for testing.